FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: don't turn an omitted assert message into undefined · nodejs/node-api-cts@590e050 · GitHub

/ node-api-cts Public

Commit 590e050

Browse files
hexbinoct
committed
fix: don't turn an omitted assert message into undefined
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>
1 parent 9aa4c88 commit 590e050

3 files changed

Lines changed: 58 additions & 10 deletions

File tree

‎.github/workflows/test.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ jobs:
1717
- 22.x
1818
- 24.x
1919
- 25.x
20+
- 26.x
2021
runs-on: ${{ matrix.runner }}
2122
steps:
2223
- name: Harden Runner

‎implementors/node/assert.js‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,17 @@ import {
77
match,
88
} from 'node:assert/strict';
99

10-
const assert = Object.assign((value, message) => ok(value, message), {
11-
ok: (value, message) => ok(value, message),
12-
strictEqual: (actual, expected, message) =>
13-
strictEqual(actual, expected, message),
14-
notStrictEqual: (actual, expected, message) =>
15-
notStrictEqual(actual, expected, message),
16-
deepStrictEqual: (actual, expected, message) =>
17-
deepStrictEqual(actual, expected, message),
18-
throws: (fn, error, message) => throws(fn, error, message),
19-
match: (string, regex, message) => match(string, regex, message),
10+
// Forward with rest arguments rather than named parameters: an omitted trailing
11+
// message has to stay omitted. Node.js 26 reads the message as a variadic tuple,
12+
// so an explicitly passed `undefined` is a message of the wrong type there and
13+
// the assertion fails with ERR_INVALID_ARG_TYPE instead of the value comparison.
14+
const assert = Object.assign((...args) => ok(...args), {
15+
ok: (...args) => ok(...args),
16+
strictEqual: (...args) => strictEqual(...args),
17+
notStrictEqual: (...args) => notStrictEqual(...args),
18+
deepStrictEqual: (...args) => deepStrictEqual(...args),
19+
throws: (...args) => throws(...args),
20+
match: (...args) => match(...args),
2021
});
2122

2223
Object.assign(globalThis, { assert });

‎tests/harness/assert.js‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,3 +105,49 @@ if (!threw) throw new Error('assert.match must throw when input is not a string'
105105
threw = false;
106106
try { assert.match('hello', 'hello'); } catch { threw = true; }
107107
if (!threw) throw new Error('assert.match must throw when pattern is not a RegExp');
108+
109+
// The message is optional on every method. A failure without one must still
110+
// report the comparison, and a failure with one must report that message.
111+
function failureOf(label, fn) {
112+
try {
113+
fn();
114+
} catch (error) {
115+
return error;
116+
}
117+
throw new Error(`${label} was expected to throw`);
118+
}
119+
120+
const optionalMessage = [
121+
['assert', () => assert(false), (m) => assert(false, m)],
122+
['assert.ok', () => assert.ok(false), (m) => assert.ok(false, m)],
123+
['assert.strictEqual', () => assert.strictEqual(1, 2), (m) => assert.strictEqual(1, 2, m)],
124+
['assert.notStrictEqual', () => assert.notStrictEqual(1, 1), (m) => assert.notStrictEqual(1, 1, m)],
125+
[
126+
'assert.deepStrictEqual',
127+
() => assert.deepStrictEqual({ a: 1 }, { a: 2 }),
128+
(m) => assert.deepStrictEqual({ a: 1 }, { a: 2 }, m),
129+
],
130+
['assert.match', () => assert.match('hello', /world/), (m) => assert.match('hello', /world/, m)],
131+
['assert.throws', () => assert.throws(() => {}, /oops/), (m) => assert.throws(() => {}, /oops/, m)],
132+
];
133+
134+
const customMessage = 'a message the caller chose';
135+
136+
for (const [label, withoutMessage, withMessage] of optionalMessage) {
137+
// Only a substring check: the strict assert methods append their own value
138+
// comparison to a caller supplied message.
139+
const described = failureOf(label, () => withMessage(customMessage));
140+
if (!described.message.includes(customMessage)) {
141+
throw new Error(
142+
`${label} must fail with the message it was given but failed with "${described.message}"`,
143+
);
144+
}
145+
146+
const bare = failureOf(label, withoutMessage);
147+
if (bare.name !== described.name) {
148+
throw new Error(
149+
`${label} must fail the same way with and without a message, but without one it failed ` +
150+
`with "${bare.name}: ${bare.message}" instead of ${described.name}`,
151+
);
152+
}
153+
}

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL