assert.deepEqual() report difference between promises during test

[regseb]

Version

v22.9.0

Platform

Linux regseblaptop 6.8.0-45-generic #45-Ubuntu SMP PREEMPT_DYNAMIC Fri Aug 30 12:02:04 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

Ubuntu 24.04.1 LTS

What steps will reproduce the bug?

1. Create test.mjs

import assert from "node:assert/strict";
import { mock, test } from "node:test";

test("promise", () => {
    assert.deepEqual(Promise.resolve("foo"), Promise.resolve("foo"));
});

test("mock, promise and await", async () => {
    const fn = mock.fn(() => "foo");
    await fn(Promise.resolve("bar"));
    assert.deepEqual(fn.mock.calls[0].arguments[0], Promise.resolve("bar"));
});

2. node --test test.mjs

How often does it reproduce? Is there a required condition?

Always.

What is the expected behavior? Why is that the expected behavior?

The assert.deepEqual() doesn't report difference between the two identical promises.

✔ promise (1.830551ms)
✔ mock, promise and await (0.533274ms)
ℹ tests 2
ℹ suites 0
ℹ pass 2
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 75.905452

What do you see instead?

The assert.deepEqual() find difference between the two identical promises.

✖ promise (5.761508ms)
  AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
  + actual - expected
  
    Promise {
      'foo',
  +   [Symbol(async_id_symbol)]: 39,
  -   [Symbol(async_id_symbol)]: 40,
      [Symbol(trigger_async_id_symbol)]: 17
    }
      at TestContext.<anonymous> (file:///home/regseb/dev/tc/mock/test.mjs:5:12)
      at Test.runInAsyncScope (node:async_hooks:211:14)
      at Test.run (node:internal/test_runner/test:930:25)
      at Test.start (node:internal/test_runner/test:829:17)
      at startSubtestAfterBootstrap (node:internal/test_runner/harness:289:17) {
    generatedMessage: true,
    code: 'ERR_ASSERTION',
    actual: [Promise],
    expected: [Promise],
    operator: 'deepStrictEqual'
  }

✖ mock, promise and await (0.791194ms)
  AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
  + actual - expected
  
    Promise {
      'bar',
  +   [Symbol(async_id_symbol)]: 52,
  +   [Symbol(trigger_async_id_symbol)]: 20
  -   [Symbol(async_id_symbol)]: 66,
  -   [Symbol(trigger_async_id_symbol)]: 54
    }
      at TestContext.<anonymous> (file:///home/regseb/dev/tc/mock/test.mjs:11:12)
      at async Test.run (node:internal/test_runner/test:931:9)
      at async Test.processPendingSubtests (node:internal/test_runner/test:629:7) {
    generatedMessage: true,
    code: 'ERR_ASSERTION',
    actual: [Promise],
    expected: [Promise],
    operator: 'deepStrictEqual'
  }

ℹ tests 2
ℹ suites 0
ℹ pass 0
ℹ fail 2
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 82.541815

✖ failing tests:

test at test.mjs:4:1
✖ promise (5.761508ms)
  AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
  + actual - expected
  
    Promise {
      'foo',
  +   [Symbol(async_id_symbol)]: 39,
  -   [Symbol(async_id_symbol)]: 40,
      [Symbol(trigger_async_id_symbol)]: 17
    }
      at TestContext.<anonymous> (file:///home/regseb/dev/tc/mock/test.mjs:5:12)
      at Test.runInAsyncScope (node:async_hooks:211:14)
      at Test.run (node:internal/test_runner/test:930:25)
      at Test.start (node:internal/test_runner/test:829:17)
      at startSubtestAfterBootstrap (node:internal/test_runner/harness:289:17) {
    generatedMessage: true,
    code: 'ERR_ASSERTION',
    actual: [Promise],
    expected: [Promise],
    operator: 'deepStrictEqual'
  }

test at test.mjs:8:1
✖ mock, promise and await (0.791194ms)
  AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
  + actual - expected
  
    Promise {
      'bar',
  +   [Symbol(async_id_symbol)]: 52,
  +   [Symbol(trigger_async_id_symbol)]: 20
  -   [Symbol(async_id_symbol)]: 66,
  -   [Symbol(trigger_async_id_symbol)]: 54
    }
      at TestContext.<anonymous> (file:///home/regseb/dev/tc/mock/test.mjs:11:12)
      at async Test.run (node:internal/test_runner/test:931:9)
      at async Test.processPendingSubtests (node:internal/test_runner/test:629:7) {
    generatedMessage: true,
    code: 'ERR_ASSERTION',
    actual: [Promise],
    expected: [Promise],
    operator: 'deepStrictEqual'
  }

Additional information

The problem only occurs in test. #55198 (comment)

1. Create index.mjs

import assert from "node:assert/strict";
import { mock } from "node:test";

assert.deepEqual(Promise.resolve("foo"), Promise.resolve("foo"));

const fn = mock.fn(() => "foo");
await fn(Promise.resolve("bar"));
assert.deepEqual(fn.mock.calls[0].arguments[0], Promise.resolve("bar"));

2. node index.mjs
3. The assert.deepEqual() doesn't report difference between the two identical promises. 👍

[RedYetiDev]

I've preemptively added the known limitation Issues that are identified as known limitations. label, as this seems to be WAI. The test runner uses promises behind the scene, so they are probably interfering. I doubt there's anything that will fix that?

[RedYetiDev]

CC @nodejs/test_runner

[RedYetiDev]

FWIW

import assert from 'node:assert';

const a = Promise.resolve('foo');
const b = Promise.resolve('bar');

assert.deepStrictEqual(a, b);

Also doesn't report anything so maybe that's a seperate bug 🤔

[cjihrig]

Fixing this will require refactoring https://github.com/nodejs/node/blob/main/lib/internal/test_runner/harness.js to remove async_hooks (which should probably be done anyway). Commenting out this line fixes the issue.

[targos]

This isn't specific to the test runner, though:

import assert from 'node:assert';
import asyncHooks from 'async_hooks';

const hook = asyncHooks.createHook({ promiseResolve() {} });
hook.enable();

const a = Promise.resolve('foo');
const b = Promise.resolve('bar');

assert.deepStrictEqual(a, b);

$ node test.mjs
node:internal/modules/run_main:123
    triggerUncaughtException(
    ^

AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
+ actual - expected

  Promise {
+   'foo',
+   [Symbol(async_id_symbol)]: 9,
-   'bar',
-   [Symbol(async_id_symbol)]: 10,
    [Symbol(trigger_async_id_symbol)]: 0
  }

[BridgeAR]

We use symbols, when we want to have something internal. Enumerable symbol properties are however always compared by assert.deepStrictEqual. I personally would like to mark them as non-enumerable or use actual private properties. That way the internals would not be compared anymore.

[cjihrig]

I was hoping we could migrate to AsyncLocalStorage, but it (I guess unsurprisingly) has the same issue:

'use strict';
const { AsyncLocalStorage } = require('node:async_hooks');
const { deepEqual } = require('node:assert/strict');
const asyncLocalStorage = new AsyncLocalStorage();

asyncLocalStorage.run({}, () => {
  deepEqual(Promise.resolve('foo'), Promise.resolve('foo'));
});

[targos]

The issue goes away with AsyncLocalStorage if used with --experimental-async-context-frame

[geeksilva97]

Folks. Quick question. Is there anything to assert promises? Something like this maybe?

---

This comment shows an example that reports nothing because it is unable to compare the values held in Promises i.e. foo and bar. In this example, if we console.log({a, b}) the result will be { a: Promise { 'foo' }, b: Promise { 'bar' } }. So, no symbols.

When inside of test the promises will have those symbols then the assertion fails - because symbols are being compared. Setting them to non-enumerable will get into the same case reported by @RedYetiDev .

Ref: https://github.com/nodejs/node/blob/main/lib/internal/util/comparisons.js#L361-L366

'use strict';
import assert from 'node:assert';
import { test } from 'node:test';

test(() => {
    const a = Promise.resolve('foo');
    const b = Promise.resolve('bar');

    const symbols = Object.getOwnPropertySymbols(a);

    symbols.forEach((s) => {
        Object.defineProperty(a, s, {
            enumerable: false
        });

        Object.defineProperty(b, s, {
            enumerable: false
        });
    });

    assert.deepStrictEqual(a, b); // reports nothing
});

I hope it makes sense.

[BridgeAR]

It is actually not possible to look into a Promise value with JavaScript alone without using then / await. To achieve that during inspection, we use a V8 API.
The reason it originally failed with the different values was only because of the enumerable symbol property, not because of the promise value. That was ignored.

For me, it is questionable, if we would want to compare promises. It could end up with flaky tests in case a promise is sometimes already settled and sometimes still pending. That can only be resolved by awaiting the promise result and if that is already the case, the value itself can easily be compared instead of the promise.

Thus, two things come to my mind:

1. Do we want to just always fail promise comparisons with something like an "ERR_WRONG_INPUT" error?
2. I believe it would be good to mark symbols as non-enumerable to not be compared and printed by default or switch them to private properties.

[regseb]

I prefer assert to support promises. In my unit tests, I check the arguments of a function. So I compare a promise in an array:

assert.deepEqual(listener.mock.calls[0].arguments, [
    Promise.resolve("two_four_five"),
    {
        prop: "quux",
        args: ["four", "five"],
        metadata: {},
    },
]);

If the promises are removed, I'd have to make an assert for the promise and asserts for every other element in the array:

assert.equal(listener.mock.calls[0].arguments.length, 2);
assert.equal(await listener.mock.calls[0].arguments[0], "two_four_five");
assert.deepEqual(listener.mock.calls[0].arguments[1], {
    prop: "quux",
    args: ["four", "five"],
    metadata: {},
});

[BridgeAR]

@regseb your "work around" is indeed the way to go. Everything else would also potentially cause race conditions in your test, as I tried to outline above.

[geeksilva97]

Thus, two things come to my mind:

1. Do we want to just always fail promise comparisons with something like an "ERR_WRONG_INPUT" error?
2. I believe it would be good to mark symbols as non-enumerable to not be compared and printed by default or switch them to private properties.

I think (1) would be good. As a dev I'd like to know this "limitation" (having warning message at least).

For (2), do you mean during the assertion?

[mcollina]

I would consider two promises to be deep equal only if they are equal.

[BridgeAR]

@mcollina are two pending promises equal? I would not consider them equal. It is natively also not possible to compare a settled promise value in an synchronous way without internal V8 methods.

The only real solution for this is to wait for the promise result. assert.rejects() is such an API. To receive the values, it would however be better for tests to just await the value. If that rejects, it's an error. If that passes, it is possible to compare the values again with deepStrictEqual().

[mcollina]

I meant that two promises are only equal if they are the same promise.