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.11k stars 694 forks source link

Integrate `chai-subset` into chai core #1621

Open koddsson opened 3 months ago

koddsson commented 3 months ago

I tried to make sure to preserve the chai-subset history when doing this.

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

43081j commented 3 months ago

it looks good to me but i have a couple of thoughts:

1 - i know you put effort into keeping the git history but i'd actually squash this anyway if we merge it as i don't think the subset history makes sense in our tree. it may be better to just note somewhere this was originally built by andrii

2 - something's bugging me about the overlap here of includes. we have basically the same algorithm in includes (written differently) than we do here, but in that one we don't do partial equality on the deep properties.

i wonder if we can at least extract the traversal logic so they both share it, or we make includes have a flag that achieves what subset does. im cautious of putting too much logic in includes though as its already a pretty big, multi-use assertion.

keithamus commented 3 months ago

2 - something's bugging me about the overlap here of includes.

We'll likely have similar logic in deep-eql too. I'll add that deep-eql is also very well optimised. I wonder if we can leverage it and it's custom comparators to effectively traverse the first layer?

koddsson commented 3 months ago

I'll start looking at squaring this up according to the comments.