Skip to content

Commit 01adadb

Browse files
fix(cli): check and snapshot keep their record while another app has the history open (#5845)
* fix(cli): check and snapshot keep their record while another app has the history open * test(cli): a held history no longer drops the snapshot record
1 parent ab2e939 commit 01adadb

4 files changed

Lines changed: 58 additions & 20 deletions

File tree

‎packages/cli/src/commands/checkArtifacts.test.ts‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ it("only warns when the report cannot be recorded; the exit status is the check'
7171
}
7272
});
7373

74-
it("still reports the check when another process holds history, without waiting for it", async () => {
74+
it("records the check when another process holds history, without waiting for it", async () => {
7575
const project = mkdtempSync(join(tmpdir(), "hf-check-history-held-"));
7676
const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
7777
const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined);
@@ -90,12 +90,10 @@ it("still reports the check when another process holds history, without waiting
9090
try {
9191
await runCommand(check, { rawArgs: [project, "--json"] });
9292
const output = JSON.parse(String(log.mock.calls.at(-1)?.[0])) as CheckReport;
93-
expect(output).not.toHaveProperty("saved");
93+
expect(output).toHaveProperty("saved.id");
9494
expect(output.browserSkipped).toBe(true);
9595
expect(consumeCommandResult().exitCode).toBe(checkExitCode(output));
96-
expect(
97-
warn.mock.calls.some((call) => String(call[0]).includes("Check report was not recorded")),
98-
).toBe(true);
96+
expect(warn.mock.calls.flat().join("\n")).not.toContain("not recorded");
9997
} finally {
10098
historyDeps.ownerWaitMs = previousWait;
10199
await held.close();

‎packages/cli/src/commands/snapshot-contactSheet.test.ts‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ describe("snapshot contact sheet failures", () => {
152152
expect(console.error).not.toHaveBeenCalled();
153153
});
154154

155-
it("keeps the captured frames when another process holds history", async () => {
155+
it("keeps and records the captured frames when another process holds history", async () => {
156156
const args = {
157157
rawArgs: [
158158
project,
@@ -181,12 +181,13 @@ describe("snapshot contact sheet failures", () => {
181181
expect(readFileSync(frame)).toEqual(Buffer.from("replaced PNG"));
182182
}
183183
const after = JSON.parse(readFileSync(statePath, "utf8"));
184-
expect(after.snapshots.unversioned).toEqual([]);
185-
expect(after.snapshots.latest).toBeNull();
186-
expect(console.error).not.toHaveBeenCalled();
187-
expect(console.warn).toHaveBeenCalledWith(
188-
expect.stringContaining("Snapshots were not recorded"),
184+
expect(after.snapshots.unversioned.map((record: { image: string }) => record.image)).toEqual(
185+
["0.5", "1"].map((time, index) =>
186+
join(project, "snapshots", `frame-0${index}-at-${time}s.png`),
187+
),
189188
);
189+
expect(console.error).not.toHaveBeenCalled();
190+
expect(console.warn).not.toHaveBeenCalledWith(expect.stringContaining("not recorded"));
190191
} finally {
191192
historyDeps.ownerWaitMs = previousWait;
192193
await held.close();

‎packages/cli/src/utils/artifactCapture.test.ts‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1+
import { spawnSync } from "node:child_process";
12
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
23
import type { AddressInfo } from "node:net";
3-
import { join } from "node:path";
4+
import { join, resolve } from "node:path";
45
import { tmpdir } from "node:os";
6+
import { fileURLToPath } from "node:url";
57
import { createAdaptorServer } from "@hono/node-server";
68
import {
79
createStudioApi,
@@ -82,6 +84,43 @@ it("does not assign results when inputs change during capture", async () => {
8284
expect(f.state().snapshots.latest?.revision).toBeNull();
8385
});
8486

87+
it("a check run while the app keeps the history open is still recorded, never under an unfiled revision", async () => {
88+
const f = await fixture();
89+
// Held here as the desktop app holds every open project's history; the CLI runs as the app runs it, a child.
90+
const held = await openProjectHistory({
91+
projectDir: f.dir,
92+
historyRoot: historyDeps.historyRoot,
93+
undoScope: "everyone",
94+
});
95+
const cli = resolve(fileURLToPath(import.meta.url), "..", "..", "cli.ts");
96+
const checkFromChild = () => {
97+
const run = spawnSync("bun", ["run", cli, "check", "--json", f.dir], {
98+
encoding: "utf-8",
99+
env: {
100+
...process.env,
101+
HYPERFRAMES_HISTORY_ROOT: historyDeps.historyRoot,
102+
HYPERFRAMES_SKIP_UPDATE_CHECK: "1",
103+
HYPERFRAMES_NO_TELEMETRY: "1",
104+
},
105+
});
106+
expect(run.stderr).not.toContain("not recorded");
107+
};
108+
try {
109+
writeFileSync(join(f.dir, "index.html"), "<html>filed</html>");
110+
await held.checkpoint();
111+
checkFromChild();
112+
expect(f.state().currentRevision).toBeTruthy();
113+
expect(f.state().checks.latest?.revision).toBe(f.state().currentRevision);
114+
115+
writeFileSync(join(f.dir, "index.html"), "<html>not filed yet</html>");
116+
checkFromChild();
117+
expect(f.state().checks.latest?.revision).toBeNull();
118+
expect(f.state().checks.unversioned).toHaveLength(1);
119+
} finally {
120+
await held.close();
121+
}
122+
});
123+
85124
it("posts a preview checkpoint instead of sending GET", async () => {
86125
const dir = mkdtempSync(join(tmpdir(), "hf-artifact-preview-"));
87126
roots.push(dir);
Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
1-
import { beginArtifactCapture } from "@hyperframes/studio-server/history";
1+
import { HistoryBusyError, beginArtifactCapture } from "@hyperframes/studio-server/history";
22
import { withOwner } from "./historyOwner.js";
33

44
const SKIP_BUSY_HISTORY = { ownerWaitMs: 0 };
55

6+
function unlessAnotherProcessKeepsTheHistory(error: unknown): void {
7+
if (!(error instanceof HistoryBusyError)) throw error;
8+
}
9+
610
export async function captureArtifactRevision(projectDir: string) {
7-
return withOwner(
8-
projectDir,
9-
async (owner) => {
10-
await owner.checkpoint();
11-
return beginArtifactCapture(projectDir);
12-
},
13-
SKIP_BUSY_HISTORY,
11+
await withOwner(projectDir, (owner) => owner.checkpoint(), SKIP_BUSY_HISTORY).catch(
12+
unlessAnotherProcessKeepsTheHistory,
1413
);
14+
return beginArtifactCapture(projectDir);
1515
}

0 commit comments

Comments
 (0)