jest.retryTimes() gets confused with snapshots

[phawxby]

🐛 Bug Report

Each retry of a test with a snapshot is identified as a different snapshot. In addition a snapshot test that eventually passes exits as a test failure (exit code 1).

$ jest snapshot.test.js
 PASS  __tests__/snapshot.test.js
  √ simple

 › 2 snapshots failed.
Snapshot Summary
 › 2 snapshots failed from 1 test suite. Inspect your code changes or run `yarn test -u` to update them.

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   2 failed, 1 passed, 3 total
Time:        1.518s
Ran all test suites matching /snapshot.test.js/i.
error Command failed with exit code 1.

To Reproduce

jest-bug.zip

// snapshot.test.js
jest.retryTimes(3);

let count = 0;

test('simple', () => {
    count++;
    const name = count === 3 ? 'pass' : 'fail';
    expect('PASS').toMatchSnapshot(name);
});

Snapshot

// snapshot.test.js.snap
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`simple: fail 1`] = `FAIL`;

exports[`simple: fail 2`] = `"FAIL"`;

exports[`simple: pass 1`] = `"PASS"`;

Expected behavior

Snapshot retries should maintain the same name and should exit with 0 if it eventually passed

Link to repl or repo (highly encouraged)

jest-bug.zip

Run npx envinfo --preset jest

  System:
    OS: Windows 10
    CPU: (16) x64 AMD Ryzen 7 1700 Eight-Core Processor
  Binaries:
    Node: 10.14.1 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.12.3 - C:\Program Files\nodejs\yarn.CMD
    npm: 6.4.1 - C:\Program Files\nodejs\npm.CMD

[SimenB]

/cc @palmerj3

[sbekrin]

I've encountered same issue, apparently jest-circus doesn't reset snapshotState in between retries.

[palmerj3]

I'll take a look! Thanks for reporting.

[palmerj3]

So I took a look at this just now.

When I test this against jest master there doesn't appear to be a problem.

Using the test suite attached in the description it initially failed to match the included snapshot. But updating the snapshot (-u) or clearing out the snapshots folder results in a snapshot that looks correct:

// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`simple: fail 1`] = `"PASS"`;

I have a feeling I'm missing some context here though. Please let me know if so. If you could provide a reproducible failure based on jest master that would be really helpful.

[palmerj3]

Also when the test suite passes the exit code is 0, not 1, for me.

[phawxby]

The bug is a couple of months old, I wonder if it has been addressed already. I'll retry it.

[itajaja]

do you guys know the status of this? happy to take a stab if it's up for grabs

[itajaja]

with a test like this:

// this should return 1, but sometimes, since it's flaky, it returns 2
function functionToTest() {
    return 1;
}

describe('foo', () => {
  it('works', () => {
    const result = functionToTest()
    expect(2).toMatchSnapshot('a');
  });
});

the first time it runs, it matches the snapshot. Now, if we substitute return 1 for return 2, this will create a second snapshot and then be done:

exports[`foo works 1`] = `1`;

exports[`foo works 2`] = `2`;

another way to reproduce a similar behavior is:

describe('foo', () => {
  it('works', () => {
    expect(1).toMatchSnapshot();
    throw new Error();
  });
});

and suppose retryTimes is set to 3, the snapshot will look like:

exports[`foo works 1`] = `1`;

exports[`foo works 2`] = `1`;

exports[`foo works 3`] = `1`;

exports[`foo works 4`] = `1`;

I am pretty sure this is not the expected behavior

running on 24.1, couldn't try this on master

[palmerj3]

Closing for now. If you create a repo that reproduces this problem on the latest version of jest I'll take another look.

[SimenB]

Seems to reproduce on jest 24.1from the last report?

@itajaja if possible, you could put together a failing e2e test in this repo and send a PR with it? Including a fix would of course be awesome, but a failing test is the best reproduction there is :)
Even if the test pass we should probably add it so we can avoid any regressions

[itajaja]

will do, but literally this:

describe('foo', () => {
  it('works', () => {
    expect(1).toMatchSnapshot();
    throw new Error();
  });
});

will create these snapshots

exports[`foo works 1`] = `1`;

exports[`foo works 2`] = `1`;

exports[`foo works 3`] = `1`;

exports[`foo works 4`] = `1`;

This is a problem between circus and snapshot, because snapshot always increments the counter before doing the match, so even looking at the code statically, this is apparent (this line specifically tells it all).

[palmerj3]

I'll take a look again today. But a repo with reproducible steps is best for these things moving forward and that is what is said in the issue template.