search

chaijs / chai

BDD / TDD assertion framework for node.js and the browser that can be paired with any testing framework.
https://chaijs.github.io
MIT License
8.15k stars 698 forks source link

Set support in same members #1583

Closed koddsson closed 9 months ago

koddsson commented 10 months ago

Fixes https://github.com/chaijs/chai/issues/1468

Add Set support to members/sameMembers. While doing that I want to introduce a iterable assertion that I think could be useful.

43081j commented 10 months ago

should we try support all iterables? may be possible by looking for Symbol.iterator? 🤔

just wondering if we want it to support something which isn't a set or array but is iterable

keithamus commented 10 months ago

I think supporting iterators generally is a good idea. Otherwise though this is a good step forward.

koddsson commented 10 months ago

should we try support all iterables? may be possible by looking for Symbol.iterator? 🤔

just wondering if we want it to support something which isn't a set or array but is iterable

I think supporting iterators generally is a good idea. Otherwise though this is a good step forward.

Yes! I wanted to change this:

https://github.com/chaijs/chai/pull/1583/files#diff-e3056660026d4431cbb909136ea4290116f5f8be089125a67d6e9ba8f364402dR3145-R3151

to new Assertion(obj, flagMsg, ssfi, true).to.be.an('iterable'); but ran into issues so I'm coming back to it.

koddsson commented 10 months ago

Something that I found is that string has Symbol.iterator so there might be some edge-cases.

koddsson commented 10 months ago

I was gonna point this PR to #1592 but I guess I can't because it's on a different remote. In any case, I split out the iterable assertion into it's own PR which I think we should merge first and then land this fix.

koddsson commented 9 months ago

I think this is good but might benefit from more tests.