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

Deep equality checks for arrays, objects, etc. containing big numbers #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pf92
Copy link

@pf92 pf92 commented Sep 20, 2019

This pull request enables deep equality checks for arrays, objects, etc. that contain big numbers.

E.g., now the following check works:
const array1 = [new BN(1), '2', new BN(-10)];
const array2 = [new BN(1), '2', '-10'];
expect(array1).to.have.a.bignumber.and.to.deep.equal(array2);

If bignumber and deep are present in the chain, the assertion will check whether actual deeply equals expected.

@nventuro
Copy link

nventuro commented Oct 7, 2019

Hello @pf92, sorry for taking so long to review this! I was out of office, and the notification for this PR must've slipped under my radar.

I did a cursory review and everything seems okay, but what would be really neat would be to address this in a way so that #5 would be fixed. Having members allows for interesting assertions when coupled with have and include, and is generally more flexible.

The code shouldn't be too different from what you've already written, do you think you could explore taking that approach instead? Thanks!

@nventuro
Copy link

nventuro commented Oct 8, 2019

After thinking about this some, perhaps a full-fledged members et al implementation is unnecessary, and something to simply check for BNs in arrays is enough. That said, I'm not sure if using deep is the best way to achieve this, since it also includes functionality for testing deep equality of objects, nested arrays, etc.

The issue with this is that we don't just test for BN objects, but also convert other types to them (namely strings), so when running a comparison on e.g. [ BN(3), '3', 'value' ], instead of getting a true deep equality, we'll get an error due to 'value' failing to be converted to a BN.

A simpler implementation with a custom flag (e.g. array), that assumes unidimensional arrays where all elements are either BNs or BN-convertible seems to fit our needs much better.

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.

2 participants