Skip to content

Snapshot feature shows empty diff when changing order of keys on object #1220

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

Closed
raymond-h opened this issue Feb 2, 2017 · 4 comments · Fixed by #1317 or singapore/gl-got#20
Closed
Labels
bug current functionality does not work as desired

Comments

@raymond-h
Copy link

raymond-h commented Feb 2, 2017

Description

When using the snapshot feature, if the new value is an object that is identical to the snapshot but with a different key order, the diff shown is empty.

Test Source

Before:

	...
	t.snapshot({
		coolNumber: 5 + 8,
		otherNumber: 9 + 8,
	});
	...

After altering the test so that the keys are in reversed order:

	...
	t.snapshot({
		otherNumber: 9 + 8,
		coolNumber: 5 + 8,
	});
	...

Error Message & Stack Trace

  1 failed

  banana
  test/test.js:6

   5: test('banana', async t => {
   6:   t.snapshot({             
   7:     otherNumber: 9 + 8,    

  Difference:

 
  Please check your code or --update-snapshots

  Test.<anonymous> (test/test.js:6:4)
  Test.fn (test/test.js:5:1)

Config

package.json:

{
  "ava": {
    "require": [
      "babel-register"
    ],
    "babel": "inherit"
  }
}

.babelrc:

{
    "plugins": [
        "transform-runtime"
    ],
    "presets": [
        "stage-0",
        "es2015-node4"
    ]
}

Command-Line Arguments

ava

Environment

Node.js v4.6.2
linux 4.8.0-34-generic

AVA 0.18.0

npm 3.10.9
@novemberborn
Copy link
Member

@vadimdemedes I assume magic assert doesn't care about key order? I suppose the snapshot comparison does, which seems fair.

@novemberborn novemberborn added the bug current functionality does not work as desired label Feb 17, 2017
@novemberborn
Copy link
Member

Please see #1275 for context.

@vadimdemedes
Copy link
Contributor

@vadimdemedes I assume magic assert doesn't care about key order? I suppose the snapshot comparison does, which seems fair.

Nope, it doesn't. t.deepEqual() doesn't care about key order, so you won't simply get an error, so it won't even "get to" magic-assert.

@novemberborn
Copy link
Member

The issue is that we're actually comparing JSON strings inside jest-snapshot, so key order suddenly matters. That's not how jest-snapshot behaves otherwise (I think because pretty-format sorts the keys).

novemberborn added a commit that referenced this issue Mar 21, 2017
* Make the Runner manage the snapshot state. Thread an accessor to the
`t.snapshot()` assertion.

* Save snapshots when runner has finished. Fixes #1218.

* Use jest-snapshot directly, without serializing values. Use jest-diff
to generate the diff if the snapshot doesn't match. This does mean the
output is not colored and has other subtle differences with how other
assertions format values, but that's OK for now. Fixes #1220, #1254.

* Pin jest-snapshot and jest-diff versions. This isn't ideal but we're
using private APIs and making other assumptions, so I'm not comfortable
with using a loose SemVer range.
novemberborn added a commit that referenced this issue Mar 23, 2017
* Make the Runner manage the snapshot state. Thread an accessor to the
`t.snapshot()` assertion.

* Save snapshots when runner has finished. Fixes #1218.

* Use jest-snapshot directly, without serializing values. Use jest-diff
to generate the diff if the snapshot doesn't match. This does mean the
output is not colored and has other subtle differences with how other
assertions format values, but that's OK for now. Fixes #1220, #1254.

* Pin jest-snapshot and jest-diff versions. This isn't ideal but we're
using private APIs and making other assumptions, so I'm not comfortable
with using a loose SemVer range.
novemberborn added a commit that referenced this issue Mar 23, 2017
* Make the Runner manage the snapshot state. Thread an accessor to the
`t.snapshot()` assertion.

* Save snapshots when runner has finished. Fixes #1218.

* Use jest-snapshot directly, without serializing values. Use jest-diff
to generate the diff if the snapshot doesn't match. This does mean the
output is not colored and has other subtle differences with how other
assertions format values, but that's OK for now. Fixes #1220, #1254.

* Pin jest-snapshot and jest-diff versions. This isn't ideal but we're
using private APIs and making other assumptions, so I'm not comfortable
with using a loose SemVer range.
novemberborn added a commit that referenced this issue Mar 26, 2017
* Make the Runner manage the snapshot state. Thread an accessor to the
`t.snapshot()` assertion.

* Save snapshots when runner has finished. Fixes #1218.

* Use jest-snapshot directly, without serializing values. Use jest-diff
to generate the diff if the snapshot doesn't match. This does mean the
output is not colored and has other subtle differences with how other
assertions format values, but that's OK for now. Fixes #1220, #1254.

* Pin jest-snapshot and jest-diff versions. This isn't ideal but we're
using private APIs and making other assumptions, so I'm not comfortable
with using a loose SemVer range.
novemberborn added a commit that referenced this issue Apr 2, 2017
* Make the Runner manage the snapshot state. Thread an accessor to the
`t.snapshot()` assertion.

* Save snapshots when runner has finished. Fixes #1218.

* Use jest-snapshot directly, without serializing values. Use jest-diff
to generate the diff if the snapshot doesn't match. This does mean the
output is not colored and has other subtle differences with how other
assertions format values, but that's OK for now. Fixes #1220, #1254.

* Pin jest-snapshot and jest-diff versions. This isn't ideal but we're
using private APIs and making other assumptions, so I'm not comfortable
with using a loose SemVer range.
novemberborn added a commit that referenced this issue Apr 2, 2017
* Make the Runner manage the snapshot state. Thread an accessor to the
`t.snapshot()` assertion.

* Save snapshots when runner has finished. Fixes #1218.

* Use jest-snapshot directly, without serializing values. Use jest-diff
to generate the diff if the snapshot doesn't match. This does mean the
output is not colored and has other subtle differences with how other
assertions format values, but that's OK for now. Fixes #1220, #1254.

* Pin jest-snapshot and jest-diff versions. This isn't ideal but we're
using private APIs and making other assumptions, so I'm not comfortable
with using a loose SemVer range.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug current functionality does not work as desired
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants