Skip to content

Commit b250484

Browse files
committed
fix: demote benign cache reserve-race to info instead of warning
In a build matrix, jobs that share a cache key (same OS/arch/lockfile across different Node versions) race to save it in the post step. Only one job can reserve a given key; the losers get cacheId === -1 from @actions/cache, which is expected and benign. setup-vp was emitting `warning("Cache save failed or was skipped.")` on that path, producing noisy warning annotations even though caching worked correctly. Demote that branch to an info message. A genuinely thrown error from saveCache still warns, since that is a real, unexpected failure. Adds src/cache-save.test.ts covering the reserve-race (no warning), success, and thrown-error paths.
1 parent 8b7d810 commit b250484

3 files changed

Lines changed: 82 additions & 2 deletions

File tree

dist/index.mjs

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

src/cache-save.test.ts

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
import { describe, it, expect, beforeEach, vi } from "vite-plus/test";
2+
3+
// Mock external shells before importing the SUT so the module's in-file
4+
// references resolve to the mocked versions.
5+
vi.mock("@actions/cache", () => ({
6+
saveCache: vi.fn(),
7+
}));
8+
vi.mock("@actions/core", () => ({
9+
getState: vi.fn(),
10+
info: vi.fn(),
11+
warning: vi.fn(),
12+
}));
13+
14+
import { saveCache as saveCacheAction } from "@actions/cache";
15+
import { getState, info, warning } from "@actions/core";
16+
import { saveCache } from "./cache-save.js";
17+
import { State } from "./types.js";
18+
19+
const mockedSaveCacheAction = vi.mocked(saveCacheAction);
20+
const mockedGetState = vi.mocked(getState);
21+
const mockedInfo = vi.mocked(info);
22+
const mockedWarning = vi.mocked(warning);
23+
24+
// Drive getState to return values for the save path:
25+
// a primary key, distinct matched key (so we don't short-circuit on a hit),
26+
// and non-empty cache paths.
27+
function stubSaveState(): void {
28+
mockedGetState.mockImplementation((name: string) => {
29+
switch (name) {
30+
case State.CachePrimaryKey:
31+
return "vite-plus-Linux-x64-pnpm-abc123";
32+
case State.CacheMatchedKey:
33+
return "";
34+
case State.CachePaths:
35+
return JSON.stringify(["/home/runner/.cache/pnpm"]);
36+
default:
37+
return "";
38+
}
39+
});
40+
}
41+
42+
describe("saveCache", () => {
43+
beforeEach(() => {
44+
vi.clearAllMocks();
45+
});
46+
47+
it("does not warn when the cache key was already reserved by a concurrent matrix job", async () => {
48+
// In a build matrix, jobs that share a cache key (e.g. same OS/arch/lockfile
49+
// across Node versions) race to save it. The losers get cacheId === -1 from
50+
// @actions/cache. That is expected and benign, not warning-worthy.
51+
stubSaveState();
52+
mockedSaveCacheAction.mockResolvedValue(-1);
53+
54+
await saveCache();
55+
56+
expect(mockedWarning).not.toHaveBeenCalled();
57+
expect(mockedInfo).toHaveBeenCalled();
58+
});
59+
60+
it("logs a success message when the cache is saved", async () => {
61+
stubSaveState();
62+
mockedSaveCacheAction.mockResolvedValue(42);
63+
64+
await saveCache();
65+
66+
expect(mockedWarning).not.toHaveBeenCalled();
67+
expect(mockedInfo).toHaveBeenCalledWith(
68+
expect.stringContaining("vite-plus-Linux-x64-pnpm-abc123"),
69+
);
70+
});
71+
72+
it("warns when saveCache throws an unexpected error", async () => {
73+
stubSaveState();
74+
mockedSaveCacheAction.mockRejectedValue(new Error("boom"));
75+
76+
await saveCache();
77+
78+
expect(mockedWarning).toHaveBeenCalledWith(expect.stringContaining("boom"));
79+
});
80+
});

src/cache-save.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ export async function saveCache(): Promise<void> {
3333
try {
3434
const cacheId = await saveCacheAction(cachePaths, primaryKey);
3535
if (cacheId === -1) {
36-
warning("Cache save failed or was skipped.");
36+
info("Cache not saved (key already reserved by a concurrent job, or save was skipped).");
3737
return;
3838
}
3939
info(`Cache saved with key: ${primaryKey}`);

0 commit comments

Comments
 (0)