Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ jobs:
- 22.x
- 24.x
- 25.x
- 26.x

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that this PR came because of behavioral changes in Node 26, but would it be better to separate this PR into two, for separation / tracking: one for the assert changes, and one for the GitHub workflow change? 🤔 Or is that too nit-picky @legendecas thoughts?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to split it, and there is an ordering that makes the split better than keeping it together.

The two are coupled in one direction only. The new harness test can only fail on 26.x, so if the assert fix lands without 26.x in the matrix, CI runs a test that passes on all four existing versions without ever exercising the thing it is checking. The other direction is free: adding 26.x on its own is safe today, because nothing in the suite fails until an assertion actually fails, so the matrix line cannot turn main red by itself.

So if you would rather have them separate:

  1. A matrix-only PR adding 26.x. Green on current main, no behavior change.
  2. This PR rebased on it, carrying only implementors/node/assert.js and tests/harness/assert.js.

Each is then independently reviewable, and the fix arrives with a job that actually runs it. Say the word and I will open the matrix PR and strip the workflow line out of this one.

One thing that belongs in the matrix PR rather than here: it takes the matrix to five Node.js versions while #37 is still open about dropping 20.x. 20.x and 25.x are both end of life now (v20.20.2 and v25.9.0, both March 2026) and 26.x is Current and untested, so trading 20.x for 26.x would hold the count at four. I left that out of this PR because it is a separate decision.

runs-on: ${{ matrix.runner }}
steps:
- name: Harden Runner
Expand Down
21 changes: 11 additions & 10 deletions implementors/node/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,17 @@ import {
match,
} from 'node:assert/strict';

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

Object.assign(globalThis, { assert });
46 changes: 46 additions & 0 deletions tests/harness/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,3 +105,49 @@ if (!threw) throw new Error('assert.match must throw when input is not a string'
threw = false;
try { assert.match('hello', 'hello'); } catch { threw = true; }
if (!threw) throw new Error('assert.match must throw when pattern is not a RegExp');

// The message is optional on every method. A failure without one must still
// report the comparison, and a failure with one must report that message.
function failureOf(label, fn) {
try {
fn();
} catch (error) {
return error;
}
throw new Error(`${label} was expected to throw`);
}

const optionalMessage = [
['assert', () => assert(false), (m) => assert(false, m)],
['assert.ok', () => assert.ok(false), (m) => assert.ok(false, m)],
['assert.strictEqual', () => assert.strictEqual(1, 2), (m) => assert.strictEqual(1, 2, m)],
['assert.notStrictEqual', () => assert.notStrictEqual(1, 1), (m) => assert.notStrictEqual(1, 1, m)],
[
'assert.deepStrictEqual',
() => assert.deepStrictEqual({ a: 1 }, { a: 2 }),
(m) => assert.deepStrictEqual({ a: 1 }, { a: 2 }, m),
],
['assert.match', () => assert.match('hello', /world/), (m) => assert.match('hello', /world/, m)],
['assert.throws', () => assert.throws(() => {}, /oops/), (m) => assert.throws(() => {}, /oops/, m)],
];

const customMessage = 'a message the caller chose';

for (const [label, withoutMessage, withMessage] of optionalMessage) {
// Only a substring check: the strict assert methods append their own value
// comparison to a caller supplied message.
const described = failureOf(label, () => withMessage(customMessage));
if (!described.message.includes(customMessage)) {
throw new Error(
`${label} must fail with the message it was given but failed with "${described.message}"`,
);
}

const bare = failureOf(label, withoutMessage);
if (bare.name !== described.name) {
throw new Error(
`${label} must fail the same way with and without a message, but without one it failed ` +
`with "${bare.name}: ${bare.message}" instead of ${described.name}`,
);
}
}
Loading