fix(ext/node): improve comparison of faked objects in deepStrictEqual (#29819)

This commit improves the handling of objects with faked prototypes in
`deepStrictEqual`. This enables `parallel/test-assert-checktag.js`

- `Date` objects are checked by `core.isDate`
- In object comparison, now it checks `obj.toString()` output (The
difference of `Symbol.toStringTag` is now detected).
- Stoped using std's assertion error message for `deepStrictEqual`, but
started using Node.js version of diff string. Now the diff expression is
more compatible with Node.
This commit is contained in:
Yoshiya Hinosawa 2025-06-20 23:33:18 +09:00 committed by GitHub
parent 1e9415cda8
commit c538f44fa0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 109 additions and 19 deletions

View file

@ -5,7 +5,7 @@ import { stringify } from "jsr:@std/yaml@^0.221/stringify";
// Bump this number when you want to purge the cache.
// Note: the tools/release/01_bump_crate_versions.ts script will update this version
// automatically via regex, so ensure that this line maintains this format.
const cacheVersion = 61;
const cacheVersion = 62;
const ubuntuX86Runner = "ubuntu-24.04";
const ubuntuX86XlRunner = "ubuntu-24.04-xl";

View file

@ -188,8 +188,8 @@ jobs:
~/.cargo/registry/index
~/.cargo/registry/cache
~/.cargo/git/db
key: '61-cargo-home-${{ matrix.os }}-${{ matrix.arch }}-${{ hashFiles(''Cargo.lock'') }}'
restore-keys: '61-cargo-home-${{ matrix.os }}-${{ matrix.arch }}-'
key: '62-cargo-home-${{ matrix.os }}-${{ matrix.arch }}-${{ hashFiles(''Cargo.lock'') }}'
restore-keys: '62-cargo-home-${{ matrix.os }}-${{ matrix.arch }}-'
if: '!(matrix.skip)'
- uses: dsherret/rust-toolchain-file@v1
if: '!(matrix.skip)'
@ -391,7 +391,7 @@ jobs:
!./target/*/*.zip
!./target/*/*.tar.gz
key: never_saved
restore-keys: '61-cargo-target-${{ matrix.os }}-${{ matrix.arch }}-${{ matrix.profile }}-${{ matrix.job }}-'
restore-keys: '62-cargo-target-${{ matrix.os }}-${{ matrix.arch }}-${{ matrix.profile }}-${{ matrix.job }}-'
- name: Apply and update mtime cache
if: '!(matrix.skip) && (!startsWith(github.ref, ''refs/tags/''))'
uses: ./.github/mtime_cache
@ -780,7 +780,7 @@ jobs:
!./target/*/gn_root
!./target/*/*.zip
!./target/*/*.tar.gz
key: '61-cargo-target-${{ matrix.os }}-${{ matrix.arch }}-${{ matrix.profile }}-${{ matrix.job }}-${{ github.sha }}'
key: '62-cargo-target-${{ matrix.os }}-${{ matrix.arch }}-${{ matrix.profile }}-${{ matrix.job }}-${{ github.sha }}'
libs:
name: build libs
needs:

View file

@ -1,9 +1,8 @@
// Copyright 2018-2025 the Deno authors. MIT license.
// vendored from std/assert/mod.ts
import { primordials } from "ext:core/mod.js";
import { core, primordials } from "ext:core/mod.js";
const {
DatePrototype,
ArrayPrototypeJoin,
ArrayPrototypeMap,
DatePrototypeGetTime,
@ -13,6 +12,7 @@ const {
ObjectIs,
ObjectKeys,
ObjectPrototypeIsPrototypeOf,
ObjectPrototypeToString,
ReflectHas,
ReflectOwnKeys,
RegExpPrototype,
@ -89,10 +89,10 @@ export function equal(c: unknown, d: unknown): boolean {
) {
return String(a) === String(b);
}
if (
ObjectPrototypeIsPrototypeOf(DatePrototype, a) &&
ObjectPrototypeIsPrototypeOf(DatePrototype, b)
) {
if (core.isDate(a) || core.isDate(b)) {
if (!core.isDate(a) || !core.isDate(b)) {
return false;
}
const aTime = DatePrototypeGetTime(a);
const bTime = DatePrototypeGetTime(b);
// Check for NaN equality manually since NaN is not
@ -112,6 +112,9 @@ export function equal(c: unknown, d: unknown): boolean {
if (a && b && !constructorsEqual(a, b)) {
return false;
}
if (ObjectPrototypeToString(a) !== ObjectPrototypeToString(b)) {
return false;
}
if (
ObjectPrototypeIsPrototypeOf(WeakMapPrototype, a) ||
ObjectPrototypeIsPrototypeOf(WeakMapPrototype, b)

View file

@ -383,11 +383,16 @@ function deepStrictEqual(
throw new ERR_MISSING_ARGS("actual", "expected");
}
toNode(
() => asserts.assertEquals(actual, expected),
{ message, actual, expected, operator: "deepStrictEqual" },
);
if (!asserts.equal(actual, expected)) {
throw new AssertionError({
message,
actual,
expected,
operator: "deepStrictEqual",
});
}
}
function notDeepStrictEqual(
actual: unknown,
expected: unknown,

View file

@ -348,8 +348,9 @@ export function createErrDiff(
`${blue}...${defaultColor}`;
}
}
return `${msg}${skipped ? skippedMsg : ""}\n${res}${other}${end}${indicator}`;
return `${msg}${
skipped ? skippedMsg : ""
}\n${res}${other}${end}${indicator}\n`;
}
export interface AssertionErrorDetailsDescriptor {

View file

@ -245,6 +245,7 @@
"test-arm-math-illegal-instruction.js",
"test-assert-async.js",
"test-assert-builtins-not-read-from-filesystem.js",
"test-assert-checktag.js",
"test-assert-fail.js",
"test-assert.js",
"test-async-hooks-run-in-async-scope-caught-exception.js",

View file

@ -1,7 +1,7 @@
<!-- deno-fmt-ignore-file -->
# Remaining Node Tests
1208 tests out of 3993 have been ported from Node 23.9.0 (30.25% ported, 70.37% remaining).
1211 tests out of 3993 have been ported from Node 23.9.0 (30.33% ported, 70.30% remaining).
NOTE: This file should not be manually edited. Please edit `tests/node_compat/config.json` and run `deno task setup` in `tests/node_compat/runner` dir instead.
@ -224,7 +224,6 @@ NOTE: This file should not be manually edited. Please edit `tests/node_compat/co
- [parallel/test-assert-calltracker-getCalls.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-assert-calltracker-getCalls.js)
- [parallel/test-assert-calltracker-report.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-assert-calltracker-report.js)
- [parallel/test-assert-calltracker-verify.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-assert-calltracker-verify.js)
- [parallel/test-assert-checktag.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-assert-checktag.js)
- [parallel/test-assert-deep-with-error.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-assert-deep-with-error.js)
- [parallel/test-assert-deep.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-assert-deep.js)
- [parallel/test-assert-esm-cjs-message-verify.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-assert-esm-cjs-message-verify.js)

View file

@ -0,0 +1,81 @@
// deno-fmt-ignore-file
// deno-lint-ignore-file
// Copyright Joyent and Node contributors. All rights reserved. MIT license.
// Taken from Node 23.9.0
// This file is automatically generated by `tests/node_compat/runner/setup.ts`. Do not modify this file manually.
'use strict';
const { hasCrypto } = require('../common');
const { test } = require('node:test');
const assert = require('assert');
// Turn off no-restricted-properties because we are testing deepEqual!
/* eslint-disable no-restricted-properties */
// Disable colored output to prevent color codes from breaking assertion
// message comparisons. This should only be an issue when process.stdout
// is a TTY.
if (process.stdout.isTTY)
process.env.NODE_DISABLE_COLORS = '1';
test('', { skip: !hasCrypto }, () => {
// See https://github.com/nodejs/node/issues/10258
{
const date = new Date('2016');
function FakeDate() {}
FakeDate.prototype = Date.prototype;
const fake = new FakeDate();
assert.notDeepEqual(date, fake);
assert.notDeepEqual(fake, date);
// For deepStrictEqual we check the runtime type,
// then reveal the fakeness of the fake date
assert.throws(
() => assert.deepStrictEqual(date, fake),
{
message: 'Expected values to be strictly deep-equal:\n' +
'+ actual - expected\n' +
'\n' +
'+ 2016-01-01T00:00:00.000Z\n' +
'- Date {}\n'
}
);
assert.throws(
() => assert.deepStrictEqual(fake, date),
{
message: 'Expected values to be strictly deep-equal:\n' +
'+ actual - expected\n' +
'\n' +
'+ Date {}\n' +
'- 2016-01-01T00:00:00.000Z\n'
}
);
}
{ // At the moment global has its own type tag
const fakeGlobal = {};
Object.setPrototypeOf(fakeGlobal, Object.getPrototypeOf(globalThis));
for (const prop of Object.keys(globalThis)) {
fakeGlobal[prop] = global[prop];
}
assert.notDeepEqual(fakeGlobal, globalThis);
// Message will be truncated anyway, don't validate
assert.throws(() => assert.deepStrictEqual(fakeGlobal, globalThis),
assert.AssertionError);
}
{ // At the moment process has its own type tag
const fakeProcess = {};
Object.setPrototypeOf(fakeProcess, Object.getPrototypeOf(process));
for (const prop of Object.keys(process)) {
fakeProcess[prop] = process[prop];
}
assert.notDeepEqual(fakeProcess, process);
// Message will be truncated anyway, don't validate
assert.throws(() => assert.deepStrictEqual(fakeProcess, process),
assert.AssertionError);
}
});
/* eslint-enable */