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 an assertion for same members where order is important #717

Closed
dominicbarnes opened this issue Jun 4, 2016 · 10 comments
Closed

Add an assertion for same members where order is important #717

dominicbarnes opened this issue Jun 4, 2016 · 10 comments

Comments

@dominicbarnes
Copy link

I've come to discover that it's not straightforward to test that an array has the same members, and that those members are in the expected order. Consider this base example:

var a = {};
var b = {};
var list = [ a, b ];

All of the assertions around "members" all explicitly do not consider order, but it would be really convenient to be able to assert that too:

// no idea what to call this assertion
assert.sameExactMembers(list, [ a, b ]);

My use-case is that I have an internal list that I can retrieve. In one case I return the list unsorted. (so sameMembers works great) In the other case, I am sorting it in a specific way, so I want to assert that my sort worked as expected.

I am using deepEqual now, but the member objects can be fairly big/deep and it's very inefficient to traverse these objects, especially since I have the direct object refs in my tests.

// in my case, a & b could be huge, so would prefer not traverse them
assert.deepEqual(list, [ a, b ]);

Another option is that I can just use strictEqual on each item in the array, but that gets unwieldy quickly imo:

// only 2 in this case... but it's copy-pasta and ugly beyond a few members
assert.strictEqual(list[0], a);
assert.strictEqual(list[1], b);
@meeber
Copy link
Contributor

meeber commented Jun 4, 2016

@dominicbarnes Thanks for the issue! I agree. Testing each element individually, either manually or via iteration, is awkward and doesn't produce useful diffs. And using deepEqual isn't efficient for hugely nested objects. Plus sometimes there might be a specific need to test elements in order via reference.

@lucasfcosta @keithamus Is there a graceful way to handle this situation that I'm missing?

@lucasfcosta
Copy link
Member

lucasfcosta commented Jun 4, 2016

Thanks for the issue @dominicbarnes.
@meeber I have faced the same problem described here. I would also find useful if we could have an assertion that compares arrays in order without going through every key of the objects inside it. However, we've got this plugin which checks sorting.

What about adding an assertion which compares arrays items one by one taking order into account but without going through every one of the objects' keys? IMO this is a common enough use case.

Also, you can use chai-things for other assertions related to Arrays.

@meeber
Copy link
Contributor

meeber commented Jun 4, 2016

Hmm... I think there are two main approaches:

  1. New flag that tells the members assertion to require same-order.
  2. New assertion that is very similar to members but requires same-order.

Either way, it should be usable with either deep or strict comparison.

Brainstorming syntax:

expect([1, 2, 3]).to.have.ordered.members([1, 2, 3]); // strict
expect([1, 2, 3]).to.have.deep.ordered.members([1, 2, 3]); // deep

expect([1, 2, 3]).to.have.orderedMembers([1, 2, 3]); // strict
expect([1, 2, 3]).to.have.deep.orderedMembers([1, 2, 3]); // deep

@lucasfcosta
Copy link
Member

Considering our current assertions I'd go with the first one. It seems more fluid and more similar to the english lanaguage. Adding a flag with the chainable word ordered would be great IMO.

@dominicbarnes
Copy link
Author

At first glance, I really like the idea of adding an ordered flag to sameMembers. How does that work in the assert API?

I'd also be content with adding orderedMembers as it's own assertion.

@meeber
Copy link
Contributor

meeber commented Jun 4, 2016

For the assert interface, it would be something like you've written: assert.orderedMembers

The only thing I'm a little uneasy about with the word "ordered" is the ease with which it could be misinterpreted as "sorted". But that might not be an issue.

I'm still kinda expecting @keithamus to pop in here with some fancy way to use the existing API to fulfill this use case :D

@keithamus
Copy link
Member

Hey y'all. Good discussion going on.

Looks like we don't have any existing assertion to cover this. I'm happy with an ordered or exact flag. Seems like you've all settled on ordered, so I say we go for it! I'll change the labels to pr-wanted and hard-fix, a short to do list of things the PR should include:

@meeber
Copy link
Contributor

meeber commented Jun 12, 2016

Starting this now!

@meeber
Copy link
Contributor

meeber commented Jun 12, 2016

What should be the behavior if both the contains and the ordered flags are set? Specifically, should both of these tests pass or only the first?

expect([1, 2, 3]).to.include.ordered.members([1, 2]);
expect([1, 2, 3]).to.include.ordered.members([2, 3]);

I'm leaning toward only the first passing, as I suspect it's a more common use case to test that an array begins with an ordered subset.

@lucasfcosta
Copy link
Member

@meeber I agree with you. I think that most people would expect the second assertion to fail, it's not common to expect that an array has ordered elements in the middle or in the end of it, IMO it goes against the main goal of the flag for this issue.

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

No branches or pull requests

4 participants