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

Canonicalize objects before stringifying and diffing them #1079

Conversation

papandreou
Copy link
Contributor

Assertion libraries can set the showDiff of the thrown Error instances to true, which makes mocha serialize the actual and expected properties, then do a string diff -- effectively a poor man's object diff. However, it produces an unnecessarily big diff when the key definition order in objects are different:

var expect = require('unexpected');

describe('the thing', function () {
    it('should DTRT', function () {
        expect({foo: 123, bar: 456, quux: 789}, 'to equal', {quux: 78, foo: 123, bar: 456});
    });
});
  0 passing (5ms)
  1 failing

  1) the thing should DTRT:

      + expected - actual

       {
      +  "quux": 78,
         "foo": 123,
      +  "bar": 456
      -  "bar": 456,
      -  "quux": 789
       }

This patch canonicalizes objects before comparing them so the above example produces:

  0 passing (5ms)
  1 failing

  1) the thing should DTRT:

      + expected - actual

       {
         "bar": 456,
         "foo": 123,
      +  "quux": 78
      -  "quux": 789
       }

... which I've found to be a great help when diffing large objects.

@rprieto
Copy link
Contributor

rprieto commented Dec 22, 2013

This would be great. I was about to submit a similar patch using json-stable-stringify, which should be a single-line fix:

base.js

var stableStringify = require('json-stable-stringify');

// line 456
// return JSON.stringify(obj, null, 2);
   return stableStringify(obj, {space: 2, cycles: true})

If we don't mind adding a dependency, let me know if we're interested in it:
https://github.com/rprieto/mocha/commit/dc324f6a063569da691dffcb992f603bb30b9f1f

@travisjeffery
Copy link
Contributor

makes sense. could you write a test though? thanks

@ai
Copy link

ai commented Dec 25, 2013

Good idea. I have often problems with different order props in objects.

@papandreou
Copy link
Contributor Author

@travisjeffery All right. Should the test be added to https://github.com/visionmedia/mocha/blob/master/test/acceptance/diffs.js? I have a little trouble understanding how the tests are structured.

I'm on holiday for the rest of the year, so if someone with time on their hands wants to see this land sooner, don't hesitate to take over :)

travisjeffery added a commit that referenced this pull request Dec 29, 2013
@travisjeffery
Copy link
Contributor

💥 done

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.

None yet

4 participants