Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

assert.any for assert.deepEqual to match on types instead of values #55319

Open
aldipower opened this issue Oct 8, 2024 · 12 comments
Open

assert.any for assert.deepEqual to match on types instead of values #55319

aldipower opened this issue Oct 8, 2024 · 12 comments
Labels
assert Issues and PRs related to the assert subsystem. feature request Issues that request new features to be added to Node.js.

Comments

@aldipower
Copy link

aldipower commented Oct 8, 2024

What is the problem this feature will solve?

Given for example a Date.now in a nested object I would like be able to run deepEqual on the object.
Such situation often happens if you are testing responses from APIs or something similar.

This obviously will throw an AssertionError:

        assert.deepEqual({
            a: 1,
            b: Date.now()
        }, {
            a: 1,
            b: 1728380577053,
        });

What is the feature you are proposing to solve the problem?

One option would be to introduce an asymmetric matcher assert.any that could dynamically check on the given type in the expected object.
Checking the type is often enough in such situation.

This would pass:

        assert.deepEqual({
            a: 1,
            b: Date.now()
        }, {
            a: 1,
            b: assert.any(Number),
        });

Another option would be to let you define custom asymmetric matchers:

        assert.deepEqual({
            a: 1,
            b: Date.now()
        }, {
            a: 1,
            b: assert.asymetricMatcher((v) => typeof v === "Number"),
        });

What alternatives have you considered?

I tried to use another assertion library which are capable of doing this: Vitest, unexpected.js, Jest
But this gives mangled output and I am not satisfied to bring in such big libraries for the sake of only doing asymmetric matching. Also I explain my tests in a documentation for other developers and I would like to use the Node built-in tools for simplicity.

@aldipower aldipower added the feature request Issues that request new features to be added to Node.js. label Oct 8, 2024
@BridgeAR BridgeAR added the assert Issues and PRs related to the assert subsystem. label Oct 8, 2024
@RedYetiDev
Copy link
Member

RedYetiDev commented Oct 8, 2024

IIRC custom assertions have been discussed before, and people are, for the most part, +1 on them, however the specific details of how they'll work haven't yet been discussed.

For your specific case, comparing the typeof using strictEqual would work.

@aldipower
Copy link
Author

For your specific case, comparing the typeof using strictEqual would work.

Do not get this fully. How is this meant?
It is already possible to compare on the type in an object? How?
Or it would work to add this as a feature?

@RedYetiDev
Copy link
Member

RedYetiDev commented Oct 8, 2024

I meant that for your specific case,

strictEqual(typeof something, 'number') could be used, however that doesn't make the request any less valid. (Also this solution isn't ideal for a number of reasons)

@BridgeAR
Copy link
Member

BridgeAR commented Oct 8, 2024

I think it's a good idea to add such functionality. The details would have to be clarified and they might be a bit more tricky but it's something I also wanted for a while already.

@RedYetiDev I don't think this is a good alternative as it would require multiple different assertions instead of one.

@RedYetiDev
Copy link
Member

The details would have to be clarified and they might be a bit more tricky but it's something I also wanted for a while already.

@nodejs/assert

@l0gicpath
Copy link

👍 on this. I agree, typeof with strictEqual would work but as @RedYetiDev said it's not ideal, you'll also need to handle cases where instanceof would be the correct check, I guess there are other corner cases. You can put this behind a function which will take you far enough, I have one in every project. But would be great to see this as part of the assert lib.

@RedYetiDev
Copy link
Member

RedYetiDev commented Oct 8, 2024

I'm definitely in favor of this, and I'm excited to see where it could go. Hypothetically, the following could be a possible (though very rough) starting point:

  1. assert.define: Define a general custom assertion.

    const assertGte = assert.define((a, b) => a >= b);
    assertGte(10, 5); // Passes
    assertGte(3, 5);  // Fails with a generic assertion error

    Alternatively:

    const assertGte = assert.define((a, b) => {
      if (a < b) throw new AssertionError(...);
      return true;
    });
    assertGte(10, 5); // Passes
    assertGte(3, 5);  // Fails with thrown AssertionError
  2. assert.when: Apply custom conditions in deep assertions (replaces .of).

    assert.deepStrictEqual({ idx: 0 }, { idx: assert.when((a) => a > 0) }); // Fails

(With the same alternative syntax)

@aldipower
Copy link
Author

@RedYetiDev Yeah, sounds great. But definitely make sure this can be used in an (nested) object, like in your second example. This is the whole point about it. To make tests on dynamically changing objects easier.

@RedYetiDev
Copy link
Member

RedYetiDev commented Oct 8, 2024

assert.define should be relatively simple to implement, and I've whipped up a userland prototype for how I believe it could work:

import { strict as assert, AssertionError } from 'node:assert';

assert.define = (cb) => {
    const customAssertion = (...args) => {
        try {
            if (!cb(...args)) {
                // Some logic would go here to show *where* the assertion failed,
                // similarly to assert(false) showing the failed assertion
                throw new AssertionError({
                    message: `${args[0]} failed a custom assertion with ${args[1]}`,
                    actual: args[0],
                    expected: args[1],
                    operator: cb,
                    stackStartFn: customAssertion,
                });
            }
        } catch (e) {
            if (e instanceof AssertionError) {
                // Some logic would go here to show *where* the assertion failed,
                // similarly to assert(false) showing the failed assertion
                throw e;
            }
            throw e;
        }
    };
    return customAssertion;
};

const assertGte = assert.define((a, b) => {
    if (a < b) {
        throw new AssertionError({
            message: `${a} is not greater than or equal to ${b}`,
            actual: a,
            expected: b,
            operator: '>=',
        });
    }
    return true;
});

assertGte(5, 10);

For assert.when, I would assume it would be a function with a symbol (Let's say kIsCustomAssertion), and the deepStrictEqual (or deepEqual, etc), would check whether that symbol exists. If it does, it'll run the custom assertion, otherwise, it'll compare the values normally.

@BridgeAR
Copy link
Member

BridgeAR commented Oct 8, 2024

The problematic part is how to implement this in the existing code. It is used in util.isDeepStrictEqual() as well and it is tuned for performance. So to use it, we would have to detect it being a magical assert object that we special handle and only for assert.

The visualized error is also problematic. Throwing an error inside of the assertion method would be fine. Otherwise it's tricky.
We currently use a diff. But this would not be valuable with a custom assertion method, as it's unclear what caused the difference. An assertion could actually trigger on having an identical value. And the algorithm is not suitable to let users know what part failed. That's done during the inspection comparison.

@BridgeAR
Copy link
Member

BridgeAR commented Oct 8, 2024

A possibility would be to use a WeakSet for detecting the methods being created by assert. The existence check would also not be that costly.

@ljharb
Copy link
Member

ljharb commented Oct 9, 2024

The challenge with any special sentinel values that can be stored in a nested object is that those very values might be present in the actual and not just the expected. With that WeakSet approach, I suppose you could skip invoking the custom assertion in "expected" when that property is also a custom assertion in "actual", but I haven't thought that through sufficiently yet to be sure that would solve it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assert Issues and PRs related to the assert subsystem. feature request Issues that request new features to be added to Node.js.
Projects
Development

No branches or pull requests

5 participants