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

Test diff output shouldn't depend on object property order #206

Closed
gijsk opened this issue Mar 2, 2012 · 3 comments
Closed

Test diff output shouldn't depend on object property order #206

gijsk opened this issue Mar 2, 2012 · 3 comments

Comments

@gijsk
Copy link
Contributor

gijsk commented Mar 2, 2012

If one uses deepEqual to compare two objects and the equality test fails, the diff output seems to depend on the order in which properties were set on the object (probably because it uses for...in). I made a trivial example using jsfiddle

I haven't quite figured out how to trick it into behaving its worst, but I've definitely seen it produce rather terrible output when property order doesn't match across objects. For more easily figuring out 'where' in the object differences occur, it would be nice if the output were improved (perhaps by sorting the properties of each object before doing a diff?).

@gijsk gijsk closed this as completed Mar 2, 2012
@gijsk gijsk reopened this Mar 2, 2012
@jzaefferer
Copy link
Member

A little edgy casy, isn't it? Would you be interested in helping to patch that? You know, pull request and such.

@gijsk
Copy link
Contributor Author

gijsk commented Mar 2, 2012

Definitely interested. I haven't checked how hard it is to fix this yet, at work, crunch time, yada yada. Will give it a shot when I'm on my commute home.

@gijsk
Copy link
Contributor Author

gijsk commented Mar 2, 2012

Done, see issue #208. I suck too much at github to get the pull request for multiple commits to show up in this issue. Not sure if you want to close this one in favour of #208, so leaving it open for now.

rwaldron pushed a commit to rwaldron/qunit that referenced this issue Mar 13, 2012
…s where properties were set in a different order. Fixes qunitjs#206
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants