Snapshots for strings are sometimes being transformed into objects/arrays

[morungos]

🐛 Bug Report

There's a strange and subtle change in the way snapshots are handled, showing some counter-intuitive displays when there is a difference. The issue manifests with string values in .toMatchSnapshot() when a snapshot name is passed as an argument.

It feels as if strings, are sometimes being incorrectly turned into arrays. The issue was not observed in Jest 23.

To Reproduce

Use the following test

describe('basic', () => {
  it('should match', () => {
    expect("Wibble wibble").toMatchSnapshot({}, "base");
  });
});

Note that without the additional arguments to .toMatchSnapshot(), this breaking behaviour is not seen. Using .toMatchSnapshot("base") (to specify a name) also works. However, the docs (https://jestjs.io/docs/en/expect#tomatchsnapshotpropertymatchers-snapshotname) do not imply that the propertyMatchers argument is optional.

Expected behavior

The stored snapshot would be the string "Wibble wibble". Instead, it is stored -- and matched -- as an object like this:

exports[`basic should match: base 1`] = `
Object {
  "0": "W",
  "1": "i",
  "10": "b",
  "11": "l",
  "12": "e",
  "2": "b",
  "3": "b",
  "4": "l",
  "5": "e",
  "6": " ",
  "7": "w",
  "8": "i",
  "9": "b",
}
`; 

Note that during testing, when there is a snapshot mismatch, even in the string, the reverse happens. A simple string like "Wobble wobble" shows as an object, with characters out of order. This is applied to the received value, making debugging with longer strings very hard.

Link to repl or repo (highly encouraged)

The file above is enough, in a Jest 24.1.0 environment.

The workaround of .toMatchSnapshot("base") is enough, and maybe this is better handled as documentation fix, but there's definitely been some change since Jest 23 in this part of the code base.

Run npx envinfo --preset jest

  System:
    OS: macOS 10.14.3
    CPU: (4) x64 Intel(R) Core(TM) i5-5257U CPU @ 2.70GHz
  Binaries:
    Node: 8.10.0 - ~/.nvm/versions/node/v8.10.0/bin/node
    Yarn: 1.2.1 - /usr/local/bin/yarn
    npm: 6.8.0 - ~/.nvm/versions/node/v8.10.0/bin/npm
  npmPackages:
    jest: ^24.1.0 => 24.1.0 

[pedrottimark]

Yes, I agree that the documentation is misleading:

https://jestjs.io/docs/en/expect#tomatchsnapshotpropertymatchers-snapshotname

I confirmed the incorrect snapshot in Jest 23.6 do you know from which version you upgraded?

when there is a snapshot mismatch, even in the string, the reverse happens.

Not sure what you mean by the reverse. Here is report if received string is one character short:

[pedrottimark]

The change is from #6528 in Jest 23.5.0 or later: If toMatchSnapshot has a property matchers argument even {} then it assumes that the received value is an object when it deep merges asymmetric matcher serializations into the baseline snapshot.

@rickhanlonii @SimenB I had already started working on snapshot matchers as part 14 of improve reports series and noticed the need to throw some matcher errors for run time argument validation.

Help me with semver: Is that a breaking change for Jest 25 or for a minor 24.x.0 version?

[jeysal]

@pedrottimark I think as soon as it throws a matcher error when the assertion might have otherwise passed it's breaking. If it's only for cases that would be failed assertions anyway IMO it could go as a kind of "error message improvement", but that's probably not the case for your snapshot matcher cases?

[pedrottimark]

Yes, thank you for explaining so clearly.

I find only a few snapshot matcher tests. They are under e2e.

I’ll write some new tests locally and split changes into pull requests for minor and for major.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.