Open itsgdpierzchalski opened 7 months ago
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();
});
I found something similar in the jest repo: https://github.com/jestjs/jest/issues/7493
Interesting, maybe we can borrow some ideas from their PR https://github.com/jestjs/jest/pull/8629. Thanks for the pointer!
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:
Then, the global test state can aggregate all results from smaller states inside itself.
Is there a way to make vitest
throw errors if snapshots do not exist? And not to create a new one?
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.
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.
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?
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.
--no-update
should be after the test file filter, or you need to do --update=false
--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.
--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 :(
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
@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 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 useUPDATE_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.
I'm experiencing the same issue. When I set the retry
parameters, new snapshots are created with each run.
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
test.spec.js.snap
System Info
Used Package Manager
npm
Validations