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

Add t.unorderedEqual() assertion #3234

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

tommy-mitchell
Copy link
Contributor

Closes #3020

Adds a new assertion for testing equality in an unordered manner between two Maps or two Sets/Arrays:

test('unorderedEqual', t => {
	t.unorderedEqual([1, 2, 3], new Set([2, 3, 1]));

	t.unorderedEqual(
		new Map([['a', 1], ['b', 2], ['c', 3]]),
		new Map([['b', 2], ['c', 3], ['a', 1]]),
		'Map keys should be equal!',
	);
});

Todos:

  • Improve error messages, format values with concordance
  • Improve types (e.g. Actual extends Map | Set | Array)
    • Add type tests
  • Fix failing tests

Copy link
Member

@novemberborn novemberborn left a comment

Choose a reason for hiding this comment

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

I think we could make this more generic, require both values to have an entries() method that returns an Iterator. Each result should then be an array with a key and a value.

You can make a map of the results for expected and then iterate through actual.

If we want we could then still require both values to have the same constructor but it may be interesting not to.

@@ -137,6 +137,10 @@ Assert that `actual` is deeply equal to `expected`. See [Concordance](https://gi

Assert that `actual` is not deeply equal to `expected`. The inverse of `.deepEqual()`. Returns a boolean indicating whether the assertion passed.

### `.unorderedEqual(actual, expected, message?)`

Assert that all values in `actual` are in `expected`, returning a boolean indicating whether the assertion passed.
Copy link
Member

Choose a reason for hiding this comment

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

Could you clarify the differences with deepEqual?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thoughts?

Assert that all values in actual are in expected. This is a variant of .deepEqual() that does not depend
on the order of actual/expected and only compares instances of Maps or Sets/arrays.

The size of actual and expected must be equal. For Maps, each key-value pair in actual must be in
expected, and vice-versa. For Sets/arrays, each value in actual must be in expected.

Returns true if the assertion passed and throws otherwise.

@@ -123,6 +123,9 @@ export type Assertions = {
* indicating whether the assertion passed.
*/
truthy: TruthyAssertion;

/** Assert that all values in `actual` are in `expected`, returning a boolean indicating whether the assertion passed. */
Copy link
Member

Choose a reason for hiding this comment

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

Here too let's clarify the differences with deepEqual.

lib/unordered-equal.js Outdated Show resolved Hide resolved
lib/assert.js Outdated Show resolved Hide resolved
lib/assert.js Outdated Show resolved Hide resolved
lib/assert.js Outdated
if (actualInfo.size !== expectedInfo.size) {
fail(new AssertionError({
assertion: 'unorderedEqual',
message: 'size must be equal',
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be the message argument. You can then do something like this:

ava/lib/assert.js

Lines 332 to 333 in a39dab0

raw: {actual, expected},
values: [formatDescriptorWithLabel('Values are deeply equal to each other, but they are not the same:', actualDescriptor)],

lib/assert.js Outdated
return false;
}

const comparedKeysResult = concordance.compare(actual.keys, expected.keys, concordanceOptions);
Copy link
Member

Choose a reason for hiding this comment

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

Did you mean:

Suggested change
const comparedKeysResult = concordance.compare(actual.keys, expected.keys, concordanceOptions);
const comparedKeysResult = concordance.compare(actual.keys(), expected.keys(), concordanceOptions);

Of course that returns an iterator, which Concordance may walk over (can't recall), but the keys won't be in the same order.

If both values have the same size, then you probably don't need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agh - the joys of plain JavaScript.

If both values have the same size, then you probably don't need this?

Good catch, I think you're right as well.

@novemberborn
Copy link
Member

Please re-open if you have a chance to return to this.

@tommy-mitchell
Copy link
Contributor Author

@novemberborn I don't think I can reopen by myself.

@novemberborn novemberborn reopened this Feb 27, 2024
@novemberborn
Copy link
Member

I don't think I can reopen by myself.

@tommy-mitchell done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Alternative assertion for unordered deep comparisons
2 participants