fix(ext/node): fix assertion error message of assert.ok (denoland#29803)

Author: kt3k

ext/node/polyfills/assert.ts

@@ -65,9 +65,10 @@ function toNode(
     expected: unknown;
     message?: string | Error;
     operator?: string;
+    stackStartFn?: Function;
   },
 ) {
-  const { operator, message, actual, expected } = opts || {};
+  const { operator, message, actual, expected, stackStartFn } = opts || {};
   try {
     fn();
   } catch (e) {
@@ -78,6 +79,7 @@ function toNode(
           message,
           actual,
           expected,
+          stackStartFn,
         });
       } else if (message instanceof Error) {
         throw message;
@@ -87,6 +89,7 @@ function toNode(
           message: e.message,
           actual,
           expected,
+          stackStartFn,
         });
       }
     }
@@ -100,10 +103,11 @@ function assert(actual: unknown, message?: string | Error): asserts actual {
       message: "No value argument passed to `assert.ok()`",
     });
   }
-  toNode(
-    () => asserts.assert(actual),
-    { message, actual, expected: true },
-  );
+  if (actual) {
+    return;
+  }
+
+  equal(actual, true, message);
 }
 const ok = assert;
 
@@ -272,6 +276,7 @@ function equal(
       operator: "==",
       actual,
       expected,
+      stackStartFn: equal,
     },
   );
 }

tests/node_compat/config.jsonc

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

tests/node_compat/runner/TODO.md

@@ -220,7 +220,6 @@ NOTE: This file should not be manually edited. Please edit `tests/node_compat/co
 - [parallel/test-aborted-util.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-aborted-util.js)
 - [parallel/test-abortsignal-cloneable.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-abortsignal-cloneable.js)
 - [parallel/test-accessor-properties.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-accessor-properties.js)
-- [parallel/test-assert-builtins-not-read-from-filesystem.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-assert-builtins-not-read-from-filesystem.js)
 - [parallel/test-assert-calltracker-calls.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-assert-calltracker-calls.js)
 - [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)

tests/node_compat/test/parallel/test-assert-builtins-not-read-from-filesystem.js

@@ -0,0 +1,55 @@
+// 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';
+
+// Do not read filesystem when creating AssertionError messages for code in
+// builtin modules.
+
+require('../common');
+const assert = require('assert');
+const EventEmitter = require('events');
+const e = new EventEmitter();
+e.on('hello', assert);
+
+if (process.argv[2] !== 'child') {
+  const tmpdir = require('../common/tmpdir');
+  tmpdir.refresh();
+  const { spawnSync } = require('child_process');
+
+  let threw = false;
+  try {
+    e.emit('hello', false);
+  } catch (err) {
+    const frames = err.stack.split('\n');
+    const [, filename, line, column] = frames[1].match(/\((.+):(\d+):(\d+)\)/);
+    // Spawn a child process to avoid the error having been cached in the assert
+    // module's `errorCache` Map.
+
+    const { output, status, error } =
+      spawnSync(process.execPath,
+                [process.argv[1], 'child', filename, line, column],
+                { cwd: tmpdir.path, env: process.env });
+    assert.ifError(error);
+    assert.strictEqual(status, 0, `Exit code: ${status}\n${output}`);
+    threw = true;
+  }
+  assert.ok(threw);
+} else {
+  const { writeFileSync } = require('fs');
+  const [, , , filename, line, column] = process.argv;
+  const data = `${'\n'.repeat(line - 1)}${' '.repeat(column - 1)}` +
+               'ok(failed(badly));';
+
+  writeFileSync(filename, data);
+  assert.throws(
+    () => e.emit('hello', false),
+    {
+      message: 'false == true'
+    }
+  );
+}

tests/node_compat/test/parallel/test-assert.js

@@ -457,7 +457,7 @@ assert.throws(
   testBlockTypeError(assert.doesNotThrow, undefined);
 }
 
-// https://github.com/nodejs/node/issues/3275
+// https://github.com/nodejs/node/issues/3277765
 assert.throws(() => { throw 'error'; }, (err) => err === 'error');
 assert.throws(() => { throw new Error(); }, (err) => err instanceof Error);
 
@@ -778,8 +778,8 @@ assert.throws(
   {
     code: 'ERR_ASSERTION',
     constructor: assert.AssertionError,
-    generatedMessage: true,
     /* TODO(kt3k): Enable this assertion
+    generatedMessage: true,
     message: 'The expression evaluated to a falsy value:\n\n  ' +
              'assert.ok(null)\n'
     */
@@ -790,8 +790,8 @@ assert.throws(
   {
     code: 'ERR_ASSERTION',
     constructor: assert.AssertionError,
-    generatedMessage: true,
     /* TODO(kt3k): Enable this assertion
+    generatedMessage: true,
     message: 'The expression evaluated to a falsy value:\n\n  ' +
              "assert(typeof 123n === 'string')\n"
     */