Test after retry should check the same snapshot, not create a new one

[itsgdpierzchalski]

Describe the bug

If I add retry to the test and for some reason it didn't match the snapshot on the first loop, it shouldn't create a new snapshot and passing the test, but check the same snapshot again.

Reproduction

test.spec.js

import { test, expect } from 'vitest';

const foo = {
  bar: 1,
  bar2: 'test',
};

test('foo', { retry: 1 }, () => {
  expect(foo).toMatchSnapshot();
});

test.spec.js.snap

// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html

exports[`foo 1`] = `
{
  "bar2": "test",
}
`;

npx vitest run src/test.spec.js

System Info

System:
    OS: Windows 11 10.0.22631
    CPU: (24) x64 13th Gen Intel(R) Core(TM) i7-13700K
    Memory: 18.34 GB / 31.76 GB
  Binaries:
    Node: 18.16.0 - C:\Program Files\nodejs\node.EXE
    npm: 9.5.1 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Chromium (122.0.2365.52)
    Internet Explorer: 11.0.22621.1
  npmPackages:
    @vitejs/plugin-vue: 4.2.3 => 4.2.3
    @vitest/ui: 1.3.1 => 1.3.1
    vite: 5.0.10 => 5.0.10
    vitest: 1.3.1 => 1.3.1

Used Package Manager

npm

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[hi-ogawa]

Thanks for raising an issue. Not sure if it's possible to support this but it looks like a weird bug. I made a reproduction on stackblitz with comments to explain the issue:
https://stackblitz.com/edit/vitest-dev-vitest-pcvtxp?file=test%2Ffile.test.ts

import { test, expect } from 'vitest';

test('file', { retry: 1 }, () => {
  //
  // Initially
  //   expect('first').toMatchSnapshot();
  ///
  // which will create snapshot file with:
  //   exports[`file 1`] = `"first"`;
  //
  // Changing it to "second" will create a following snapshot
  //   exports[`file 1`] = `"first"`;
  //   exports[`file 2`] = `"second"`;
  //
  // After that, running "u" (update snapshot) cannot remove the first one.
  //
  expect('second').toMatchSnapshot();
});

[itsgdpierzchalski]

I found something similar in the jest repo:
jestjs/jest#7493

[hi-ogawa]

Interesting, maybe we can borrow some ideas from their PR jestjs/jest#8629. Thanks for the pointer!

[sheremet-va]

We need to rewrite how the snapshot client works to make this work. Since #4796 there is only a single snapshot state per file, so we cannot rollback changes depending on the test state (in jest, the state is per test, but it's also a singleton which doesn't support concurrent retriable tests).

I think we need to have a map of states based on the test ID (or the combination of filepath + name) and reset it in the onBeforeTryTask hook:

vitest/packages/vitest/src/runtime/runners/test.ts

Line 122 in bc4b607

onBeforeTryTask(test: Task) {

Then, the global test state can aggregate all results from smaller states inside itself.

[Rihyx]

Is there a way to make vitest throw errors if snapshots do not exist? And not to create a new one?

[sheremet-va]

Is there a way to make vitest throw errors if snapshots do not exist? And not to create a new one?

You can run Vitest with --no-update flag.

[Rihyx]

Is there a way to make vitest throw errors if snapshots do not exist? And not to create a new one?

You can run Vitest with --no-update flag.

As I understand this flag will not prevent creating a new snapshot. For example, there can be a case when there is a function which got refactored and changed the name and if the function name is in the test then the old snapshot will get obsolete. In the CI the test will not fail because a new snapshot will be created. But there is a high possibility of the code being broken.

[sheremet-va]

Is there a way to make vitest throw errors if snapshots do not exist? And not to create a new one?

You can run Vitest with --no-update flag.

As I understand this flag will not prevent creating a new snapshot. For example, there can be a case when there is a function which got refactored and changed the name and if the function name is in the test then the old snapshot will get obsolete. In the CI the test will not fail because a new snapshot will be created. But there is a high possibility of the code being broken.

Did you run Vitest with this flag before reporting this?

[Rihyx]

Is there a way to make vitest throw errors if snapshots do not exist? And not to create a new one?

You can run Vitest with --no-update flag.

As I understand this flag will not prevent creating a new snapshot. For example, there can be a case when there is a function which got refactored and changed the name and if the function name is in the test then the old snapshot will get obsolete. In the CI the test will not fail because a new snapshot will be created. But there is a high possibility of the code being broken.

Did you run Vitest with this flag before reporting this?

Code:

export function something() {
  return 'something';
}

Test File:

import { describe, expect, it } from 'vitest';

import { something } from './something.ts';

describe('Lets test some snapshots', () => {
  describe(`${something.name}`, () => {
    it('should return something', () => {
      expect(something()).toMatchSnapshot();
    });
  });
});

First Run:
npx vitest --no-update src/rihatest/something.test.ts

 ✓ src/rihatest/something.test.ts (1)
   ✓ Lets test some snapshots (1)
     ✓ something (1)
       ✓ should return something

  Snapshots  1 written
 Test Files  1 passed (1)
      Tests  1 passed (1)
   Start at  11:12:41
   Duration  250ms (transform 17ms, setup 0ms, collect 15ms, tests 2ms, environment 0ms, prepare 54ms)

Changed the code:

export function somethingElse() {
  return 'something' + 'else';
}

Test file:

import { describe, expect, it } from 'vitest';

import { somethingElse } from './something.ts';

describe('Lets test some snapshots', () => {
  describe(`${somethingElse.name}`, () => {
    it('should return something', () => {
      expect(somethingElse()).toMatchSnapshot();
    });
  });
});

Test run:
npx vitest --no-update src/rihatest/something.test.ts

 src/rihatest/something.test.ts (1)
   ✓ Lets test some snapshots (1)
     ✓ somethingElse (1)
       ✓ should return something

  Snapshots  1 written
             1 obsolete
             ↳ src/rihatest/something.test.ts
               · Lets test some snapshots > something > should return something 1

 Test Files  1 passed (1)
      Tests  1 passed (1)
   Start at  11:15:56
   Duration  292ms (transform 21ms, setup 0ms, collect 16ms, tests 2ms, environment 0ms, prepare 47ms)

Both run's snapshots didn't exist before the run. I know maybe putting the function name into the description is not the best idea but still. Snapshots are created all the time.

[sheremet-va]

--no-update should be after the test file filter, or you need to do --update=false

[sheremet-va]

--no-update should be after the test file filter, or you need to do --update=false

I did check it here and it indeed doesn't work as expected: https://stackblitz.com/edit/vitest-dev-vitest-2kw17c?file=package.json&initialPath=__vitest__/

Vitest shouldn't create snapshot files in CI (update is false there by default) - this is a bug, it didn't work like that before.

[Rihyx]

--no-update should be after the test file filter, or you need to do --update=false

I did check it here and it indeed doesn't work as expected: https://stackblitz.com/edit/vitest-dev-vitest-2kw17c?file=package.json&initialPath=__vitest__/

Vitest shouldn't create snapshot files in CI (update is false there by default) - this is a bug, it didn't work like that before.

Thanks for verifying. Yeah this has to be fixed :(

[sheremet-va]

Thanks for verifying. Yeah this has to be fixed :(

I also checked with CI=true and it did throw an error correctly, so it looks like it's just an issue with --update flag

[sheremet-va]

@Rihyx Ok, I checked the implementation. --no-update only affects if snapshots are updated - they will always be created in a non-CI environment (there is no flag to disable this) - you can also use UPDATE_SNAPSHOT env, by the way (needs to be truthy). Please create a separate issue regarding this behaviour if you want it changed - this issue is unrelated.

[Rihyx]

@Rihyx Ok, I checked the implementation. --no-update only affects if snapshots are updated - they will always be created in a non-CI environment (there is no flag to disable this) - you can also use UPDATE_SNAPSHOT env, by the way (needs to be truthy). Please create a separate issue regarding this behaviour if you want it changed - this issue is unrelated.

All right, thanks a lot. I will create a separate issue for that.

[jimmyn]

I'm experiencing the same issue. When I set the retry parameters, new snapshots are created with each run.