[node] Harden TestContext's assert against missing methods, add missing v22 method
#71902
Conversation
Renegade334
force-pushed
the
node-test-assert-imports
branch
from
d8cedfe to
cc1d13c
Compare
Renegade334
force-pushed
the
node-test-assert-imports
branch
from
c86cdc6 to
4f50be0
Compare
Renegade334
requested review from
eps1lon and
peterblazejewicz
as code owners
|
@Renegade334 Thank you for submitting this PR! This is a live comment that I will keep updated. 1 package in this PRCode ReviewsBecause this is a widely-used package, a DT maintainer will need to review it before it can be merged. You can test the changes of this PR in the Playground. Status
Once every item on this list is checked, I'll ask you for permission to merge and publish the changes. Diagnostic Information: What the bot saw about this PR{
"type": "info",
"now": "-",
"pr_number": 71902,
"author": "Renegade334",
"headCommitOid": "a40c72611ecaec578a42a4bda52f925951f333e7",
"mergeBaseOid": "0f3e3a0e1f0ffc33250f9ce9526533e60678bd3d",
"lastPushDate": "2025-02-08T23:16:00.000Z",
"lastActivityDate": "2025-02-09T01:49:43.000Z",
"hasMergeConflict": false,
"isFirstContribution": false,
"tooManyFiles": false,
"hugeChange": false,
"popularityLevel": "Critical",
"pkgInfo": [
{
"name": "node",
"kind": "edit",
"files": [
{
"path": "types/node/test.d.ts",
"kind": "definition"
},
{
"path": "types/node/test/test.ts",
"kind": "test"
},
{
"path": "types/node/v20/test.d.ts",
"kind": "definition"
},
{
"path": "types/node/v20/test/test.ts",
"kind": "test"
}
],
"owners": [
"Microsoft",
"jkomyno",
"alvis",
"r3nya",
"btoueg",
"smac89",
"touffy",
"DeividasBakanas",
"eyqs",
"Hannes-Magnusson-CK",
"hoo29",
"kjin",
"ajafff",
"islishude",
"mwiktorczyk",
"mohsen1",
"galkin",
"parambirs",
"eps1lon",
"ThomasdenH",
"WilcoBakker",
"wwwy3y3",
"samuela",
"kuehlein",
"bhongy",
"chyzwar",
"trivikr",
"yoursunny",
"qwelias",
"ExE-Boss",
"peterblazejewicz",
"addaleax",
"victorperin",
"NodeJS",
"LinusU",
"wafuwafu13",
"mcollina",
"Semigradsky"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Critical"
}
],
"reviews": [],
"mainBotCommentID": 2646095748,
"ciResult": "pass"
} |
|
🔔 @microsoft @jkomyno @alvis @r3nya @btoueg @smac89 @Touffy @DeividasBakanas @eyqs @Hannes-Magnusson-CK @hoo29 @kjin @ajafff @islishude @mwiktorczyk @mohsen1 @galkin @parambirs @eps1lon @ThomasdenH @WilcoBakker @wwwy3y3 @samuela @kuehlein @bhongy @chyzwar @trivikr @yoursunny @qwelias @ExE-Boss @peterblazejewicz @addaleax @victorperin @nodejs @LinusU @wafuwafu13 @mcollina @Semigradsky — please review this PR in the next few days. Be sure to explicitly select |
Previously, the test module had no CI checks for whether or not the correct functions from the assert module were re-exported as methods in TestContext's
assert. Changes that add top-level functions in the assert module are easily missed from the test module – for example when #71663 addedpartialDeepStrictEqual(), which didn't make it into TestContext.This edit simplifies the re-export behaviour, and adds tests that will flag if top-level assertion functions get added from the assert module and aren't mirrored in the test module, using similar checks to those used upstream. CI results are available for both commits for demonstration purposes, both before and after adding the function that was missing in the v22 branch.
The docblocks for the
assertproperties in test.d.ts weren't exposed by tsserver when annotating method calls anyway, so removing these isn't really an issue.