Skip to content

Commit

Permalink
fix(snapshot): fix concurrent SnapshotClient.startCurrentRun with s…
Browse files Browse the repository at this point in the history
…ame filepath
  • Loading branch information
hi-ogawa committed Dec 22, 2023
1 parent 039814b commit 425cd18
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 4 deletions.
18 changes: 14 additions & 4 deletions packages/snapshot/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,19 +53,25 @@ export class SnapshotClient {
constructor(private options: SnapshotClientOptions = {}) {}

async startCurrentRun(filepath: string, name: string, options: SnapshotStateOptions) {
console.log("[SnapshotClient.startCurrentRun]", { filepath, name });

Check failure on line 56 in packages/snapshot/src/client.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected console statement

Check failure on line 56 in packages/snapshot/src/client.ts

View workflow job for this annotation

GitHub Actions / lint

Strings must use singlequote

Check failure on line 56 in packages/snapshot/src/client.ts

View workflow job for this annotation

GitHub Actions / lint

Extra semicolon

this.filepath = filepath
this.name = name

if (this.snapshotState?.testFilePath !== filepath) {
await this.finishCurrentRun()

// Need to move async `SnapshotState.create` outside of synchronous `Map.get/set` to support concurrent test, which does following:
// Promise.all([client.startCurrentRun(file, name1), client.startCurrentRun(file, name2)])
// TODO: it should save redundant `SnapshotState.create` by tracking `Map<string, Promise<SnapshotState>>`
const created = await SnapshotState.create(
filepath,
options,
);

Check failure on line 70 in packages/snapshot/src/client.ts

View workflow job for this annotation

GitHub Actions / lint

Extra semicolon
if (!this.getSnapshotState(filepath)) {
this.snapshotStateMap.set(
filepath,
await SnapshotState.create(
filepath,
options,
),
created,
)
}
this.snapshotState = this.getSnapshotState(filepath)
Expand All @@ -86,6 +92,8 @@ export class SnapshotClient {
}

assert(options: AssertOptions): void {
console.log("[SnapshotClient.assert:in]", options);

Check failure on line 95 in packages/snapshot/src/client.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected console statement

Check failure on line 95 in packages/snapshot/src/client.ts

View workflow job for this annotation

GitHub Actions / lint

Strings must use singlequote

Check failure on line 95 in packages/snapshot/src/client.ts

View workflow job for this annotation

GitHub Actions / lint

Extra semicolon

const {
filepath = this.filepath,
name = this.name,
Expand Down Expand Up @@ -135,6 +143,7 @@ export class SnapshotClient {
inlineSnapshot,
rawSnapshot,
})
console.log("[SnapshotClient.assert:out]", { actual, expected, key, pass });

Check failure on line 146 in packages/snapshot/src/client.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected console statement

Check failure on line 146 in packages/snapshot/src/client.ts

View workflow job for this annotation

GitHub Actions / lint

Strings must use singlequote

Check failure on line 146 in packages/snapshot/src/client.ts

View workflow job for this annotation

GitHub Actions / lint

Extra semicolon

if (!pass)
throw createMismatchError(`Snapshot \`${key || 'unknown'}\` mismatched`, this.snapshotState?.expand, actual?.trim(), expected?.trim())
Expand Down Expand Up @@ -169,6 +178,7 @@ export class SnapshotClient {
if (!this.snapshotState)
return null
const result = await this.snapshotState.pack()
console.log("[SnapshotClient.finishCurrentRun]", result);

this.snapshotState = undefined
return result
Expand Down
13 changes: 13 additions & 0 deletions packages/snapshot/src/port/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export default class SnapshotState {
testFilePath: string,
options: SnapshotStateOptions,
) {
console.log("[SnapshotState.create]", { testFilePath });
const snapshotPath = await options.snapshotEnvironment.resolvePath(testFilePath)
const content = await options.snapshotEnvironment.readSnapshotFile(snapshotPath)
return new SnapshotState(testFilePath, snapshotPath, content, options)
Expand Down Expand Up @@ -227,6 +228,15 @@ export default class SnapshotState {
error,
rawSnapshot,
}: SnapshotMatchOptions): SnapshotReturnOptions {
console.log("[SnapshotState.match]", {
testName,
received,
key,
inlineSnapshot,
isInline,
error,
rawSnapshot
});
this._counters.set(testName, (this._counters.get(testName) || 0) + 1)
const count = Number(this._counters.get(testName))

Expand Down Expand Up @@ -261,6 +271,7 @@ export default class SnapshotState {
const pass = expectedTrimmed === prepareExpected(receivedSerialized)
const hasSnapshot = expected !== undefined
const snapshotIsPersisted = isInline || this._fileExists || (rawSnapshot && rawSnapshot.content != null)
console.log("[SnapshotState.match:2]", { expected, pass, hasSnapshot, snapshotIsPersisted }, [this._updateSnapshot]);

if (pass && !isInline && !rawSnapshot) {
// Executing a snapshot file as JavaScript and writing the strings back
Expand Down Expand Up @@ -298,6 +309,7 @@ export default class SnapshotState {
}
}
else {
console.log("@@@@@@@ this.added++")
this._addSnapshot(key, receivedSerialized, { error, isInline, rawSnapshot })
this.added++
}
Expand Down Expand Up @@ -325,6 +337,7 @@ export default class SnapshotState {
}
}
else {
console.log("@@@@@@@ this.matched++")
this.matched++
return {
actual: '',
Expand Down
16 changes: 16 additions & 0 deletions test/core/test/repro.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
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("hi1").toMatchInlineSnapshot(`"hi1"`);
});

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

0 comments on commit 425cd18

Please sign in to comment.