fix: don't turn an omitted assert message into undefined - #70
Open
hexbinoct wants to merge 1 commit into
Open
Conversation
implementors/node/assert.js wraps each assertion in an arrow function with named parameters and forwards them positionally, so a call that leaves the message out passes an explicit undefined instead. Node.js 26 reads the message as a variadic tuple (nodejs/node#58849, first released in v26.0.0), where [undefined] is a message of the wrong type rather than an absent one. assert.strictEqual(1, 2) on v26 fails with TypeError [ERR_INVALID_ARG_TYPE]: The "message" argument must be one of type string or function. Received undefined instead of an AssertionError carrying the actual and expected values. strictEqual, notStrictEqual and deepStrictEqual take that path; ok, match and throws still report normally. Verified broken on v26.0.0 and v26.5.1, fine on v25.9.0 and earlier. It only shows up once an assertion fails, which is the moment a conformance run is worth reading, so a runtime with a real Node-API gap gets a message about the CTS harness rather than about its own behavior. Forwarding with rest arguments leaves an omitted argument omitted. The harness test now checks that every method fails the same way with and without a message, and the CI matrix gains 26.x, the only entry that exercises it. Signed-off-by: hexbinoct <abubakarm@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Node.js implementor's
assertshim swallows the actual/expected diff on Node.js 26, so afailing test reports a harness error instead of the conformance gap that caused it.
What happens
implementors/node/assert.jsforwards each method through an arrow function with namedparameters:
A caller that omits the message therefore passes an explicit
undefined. Node.js 26 changed theinternal message parameter into a variadic tuple (nodejs/node#58849,
first released in v26.0.0), and there
[undefined]is a message of the wrong type rather than anabsent one:
So on v26
assert.strictEqual(1, 2)reports that instead of anAssertionErrorcarrying1and2.strictEqual,notStrictEqualanddeepStrictEqualtake that path;ok,matchandthrowsstill report normally.assert.strictEqual(1, 2)AssertionErrorAssertionErrorTypeError [ERR_INVALID_ARG_TYPE]TypeError [ERR_INVALID_ARG_TYPE]The suite stays green either way, because nothing is wrong until an assertion actually fails. That
is also why it matters: the failure message is what a conformance run is read for, and a runtime
with a genuine Node-API gap would be handed a message about the CTS harness rather than about its
own behavior.
The change
implementors/node/assert.js: forward with rest arguments, so an omitted argument stays omitted.tests/harness/assert.js: for all seven entry points, check that a failure with a messagecarries that message, and that a failure without one fails the same way. The existing checks only
asserted that something was thrown, which a
TypeErrorsatisfies..github/workflows/test.yml: add26.x.The matrix entry is in the same commit on purpose. 26.x is the only version where the new check has
any teeth, so without it the test is inert. Happy to split it out if you would rather keep matrix
changes separate, and note that it puts the count at five Node.js versions while
#37 is still open about dropping 20.x. For what
it is worth, 20.x and 25.x are both past end of life now (last releases v20.20.2 and v25.9.0, both
March 2026) while 26.x is Current and was untested.
Verification
Test suite plus lint, all green:
node:<major>-bookworm, GCC 12.2: 20 (throughts-strip.js, as CI does), 22, 25, 26Teeth check on the new test, running
tests/harness/assert.jsdirectly rather than through therunner: with the fix reverted it fails on v26.5.1 with
and passes on v24.18.1, which is exactly why the matrix entry is needed.
Claude Opus 5 found this, wrote the fix and the test, and drafted this text; I reviewed both.