test.concurrent and snapshot support.

[cpojer]

Once we support named snapshots, a snapshot doesn't need to depend on test case state any more. That way test.concurrent can use snapshots as well.

cc @DmitriiAbramov Can we make it so we throw when using toMatchSnapshot without arguments in the concurrent mode? I'm not sure where I'd add this right now.

[cpojer]

cc @DmitriiAbramov ping!

[aaronabramov]

here's what we have right now:

1. Snapshot name consists of file_name, test_name, index.
2. Every time we start running a new test, we save these variables into the global snapshot context
3. Once a call to toMatchSnapshot occurs, we grab those values and generate a snapshot name+index and write/compare the resulting value/object passed to expect

In concurrent mode right now we have two problems:

1. We don't know what test we're currently running
2. While non-concurrent test is running, if a concurrent test calls to toMatchSnapshot, the resulting snapshot well be written into a wrong place.

both can be solved by using named snapshots, although we'll lose the test name in the snapshot name (it'll be file-name+name_of_the_snapshot)

regarding throwing in concurrent mode. not sure about that. If we run tests in concurrent mode, during any test execution you can expect toMatchSnapshot to be called from any other tests. so as long as we don't differentiate these calls (this.expect('lol').toMatchSnapshot()) i don't think it's possible.

We can require named snapshot fro all toMatchSnapshot() calls within the file if at least one test.concurrent has been used. Not sure if it's a good experience though

[rally25rs]

Hey gang 👋 I just discovered this while adding a test to Yarn which is using Jest v20.0.4

I think it's the same issue as this;

I have several test.concurrent in a file, and in one of them I have expect(result).toMatchSnapshot();

Jest succeeds and reports

Snapshot Summary
 › 1 snapshot written in 1 test suite.

However the written snapshot is for the wrong test name.
If I re-run the tests, it also fails the wrong test, not the one with this expect.

I understand there are some concurrency issues, but as a stopgap solution, would it be possible to error with some meaningful message if you even attempt to use a snapshot in a concurrent test?

edit ; just noticed the response that said that this might not be possible to detect... oops :)

[evocateur]

I think it's fair to require named snapshots when using test.concurrent(), given that the concurrent API is opt-in anyway. Sure would be nice to parallelize expensive integration tests in lerna and maintain snapshot identity...

[Isakdl]

Any updates on this? I recently ran into this issue when trying to speed up some slow integration tests.

[l-abels]

I don't use test.concurrent, but I do use async/await in my Jest tests and my snapshots wind up with incorrect names. Would a solution for test.concurrent also need to work for async/await?

[SimenB]

async tests should work fine... are you forgetting an await somewhere? If not, please open up a new issue with a reproduction as it's not really related to .concurrent - it should work fine

[jasonkuhrt]

In the above referenced PR we found that test.concurrent.each with snapshots still does not work. I guess that is to be expected since this issue is still open :D

We found that each run would result in snapshot failures. As if the pairing between saved snaps and test runs was getting confused.

We were naming our snapshots (.toMatchSnapshot('foobar')) but that did not seem to help at all.

Should our team wait for support here or ditch test.each in favour of test files?

Is jest test file concurrency slower than test.concurrent?

[SimenB]

@jasonkuhrt test.each is perfectly fine to use with snapshots, it's only and issue with concurrent.

File concurrency has more overhead as we need to spawn processes and inject the whole test env multiple times. But if the tests themselves are slower than that overhead (which is usually the case with integration tests) then you should have better times using file concurrency

[OlgaKuksa]

Actually, that would be nice to support string passed to toMatchSnapshot() (once it's passed) as the only source of truth for snapshot name. Or there may be object with {name:"snapshot name"} passed.
I might have missed smth - but is there any reason not to do it? It seems to be quite simple to add (from what I can see in src) and would give a workaround for "snapshots-in-concurrent-mode" issue.

[samuelfullerthomas]

I have a draft PR for adding a toMatchNamedSnapshot as a matcher here: #11605

I think it would be a nice way forward since it wouldn't touch existing APIs - any thoughts on it? I know I tagged you in the PR but thought I would also tag you here @SimenB

[github-actions]

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

[github-actions]

This issue was closed because it has been stalled for 30 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

[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.

[SimenB]

https://github.com/jestjs/jest/releases/tag/v29.6.0