Skip to content

Fix expect(new Array(1)).toEqual([undefined]) failures on *some* platforms - #1048

Merged
rreusser merged 4 commits into
masterfrom
fix-deep-equal-sloppiness
Oct 17, 2016
Merged

Fix expect(new Array(1)).toEqual([undefined]) failures on *some* platforms#1048
rreusser merged 4 commits into
masterfrom
fix-deep-equal-sloppiness

Conversation

@rreusser

@rreusserrreusser commented Oct 17, 2016

Copy link
Copy Markdown
Contributor

This test adds a helper function to the lib test in order to loosen the comparison to what we actually care about, which is array access, not array content.

Gross.

Comment threadtest/jasmine/tests/lib_test.js Outdated
var i;
if(Array.isArray(x)) {
for(i = 0; i < x.length; i++) {
x[i] = x[i];

@rreusserrreusserOct 17, 2016

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

😐

Comment threadtest/jasmine/tests/lib_test.js Outdated
var expected = {marker: [undefined, {size: {magnitude: 8}}]};
var computed = Lib.expandObjectPaths(input);
expect(computed).toEqual(expected);
expectLooseDeepEqual(computed, expected);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Instead of that custom assert routine, why not just have to expect statements:

expect(computed.marker[0]).toBeUndefined();expect(computed.marker[1]).toEqual({size: {magnitude: 8}});

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I can move the function inside this block, but ideally wanted to avoid manually digging through the objects.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah that make sense.

What about moving your new expectLooseDeepEqual to assets/custom_matchers.js ?

@etpinardetpinard mentioned this pull request Oct 17, 2016
@rreusser

rreusser commented Oct 17, 2016

Copy link
Copy Markdown
ContributorAuthor

@etpinard added expect(...).toLooseDeepEqual(...). It wasn't straightforward to try to hook into jasmine's internals, so I just did the check via the deep-equal module. If it passes, that should get it.

@etpinard

Copy link
Copy Markdown
Contributor

💃 once test pass! Thanks!

@rreusser
rreusser merged commit 17102d8 into masterOct 17, 2016
@rreusser
rreusser deleted the fix-deep-equal-sloppiness branch October 17, 2016 21:39
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugsomething broken

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@rreusser@etpinard