Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: test for concurrent test suite not updating inline snapshots (WIP) #4783

Closed
wants to merge 1 commit into from

Conversation

martypdx
Copy link

@martypdx martypdx commented Dec 19, 2023

Description

Created a working test for #4782. No fix yet

PR checklist

  • It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.
  • Ideally, include a test that fails without this PR but passes with it.
  • Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.

Tests

  • Run the tests with pnpm test:ci.

Documentation

  • If you introduce new functionality, document it. You can run documentation with pnpm run docs command.

Changesets

  • Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.

Copy link

netlify bot commented Dec 19, 2023

Deploy Preview for fastidious-cascaron-4ded94 canceled.

Name Link
🔨 Latest commit 99c5992
🔍 Latest deploy log https://app.netlify.com/sites/fastidious-cascaron-4ded94/deploys/65821cd6ef1583000876a5e3

@hi-ogawa
Copy link
Contributor

hi-ogawa commented Dec 20, 2023

Interesting... I did a quick debug logging around:

  • toMatchInlineSnapshot (packages/vitest/src/integrations/snapshot/chai.ts)
  • SnapshotClient (packages/vitest/src/runtime/runners/test.ts, packages/snapshot/src/client.ts)

and this is what I see:

sample test and log
//// repro.test.ts

import { it, beforeAll } from 'vitest';

// a little trick to restore normal console.log to see genuine chronological order
// https://github.com/vitest-dev/vitest/issues/1405#issuecomment-1858696036
beforeAll(async () => {
  const { Console } = await import("node:console");
  globalThis.console = new Console(process.stdout, process.stderr);
});

it.concurrent('1st', ({ expect }) => {
  expect("hi").toMatchInlineSnapshot();
});

it.concurrent('2nd', ({ expect }) => {
  expect("hi").toMatchInlineSnapshot(`"hi"`);
});
//// log

[snapshotClient.startCurrentRun] 1st
[snapshotClient.startCurrentRun] 2nd    //  snapshot state of "1st" is overwritten immediately?
[toMatchInlineSnapshot] 1st
[toMatchInlineSnapshot] 2nd
[snapshotClient.finishCurrentRun] {
  filepath: '/home/hiroshi/code/others/vitest/test/core/test/repro.test.ts',
  added: 0,
  fileDeleted: false,
  matched: 1,
  unchecked: 0,
  uncheckedKeys: [],
  unmatched: 0,
  updated: 0
}

Since SnapshotClient is singleton, I wonder if this was intended to work even with local context expect.

let _client: SnapshotClient
export function getSnapshotClient(): SnapshotClient {
if (!_client) {
_client = new SnapshotClient({
isEqual: (received, expected) => {
return equals(received, expected, [iterableEquality, subsetEquality])
},
})
}
return _client
}


EDIT: Actually SnapshotClient single might be fine since each SnapshotClient.assert is called with "test name". It should work as long as SnapshotClient.assert and SnapshotState.match can keep track of independent states base on "test name".

getSnapshotClient().assert({
received: expected,
message,
isInline: true,
properties,
inlineSnapshot,
error,
errorMessage,
...getTestNames(test),
})

Actually, there's already concurrent snapshot test in https://github.com/vitest-dev/vitest/blob/main/test/snapshots/test/shapshots-concurrent.test.ts#L10

@sheremet-va
Copy link
Member

This was fixed by #4796

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants