From 36b7b57007634b5d947fd73fcd85c7b44863efec Mon Sep 17 00:00:00 2001 From: soujasK <121026960+soujasK@users.noreply.github.com> Date: Mon, 26 Oct 2026 00:08:39 +0530 Subject: [PATCH] fix: only report a saved cache when saving succeeded (#1110) ## Summary The post step logs `uv cache saved with key: ...` (and `Python cache saved with key: ...`) even when the save failed. The log in #769 shows it right after `Warning: Failed to save: ...`: ``` Warning: Failed to save: "C:\Program failed with error: The process 'C:\Program Files\Git\usr\bin\tar.exe' failed with exit code 2 uv cache saved with key: setup-uv-2-x86_64-pc-windows-msvc-windows-11-3.12.12-pruned-... ``` `@actions/cache`'s `saveCache` doesn't throw for these failures. It logs them and returns `-1`, and `saveCacheToKey` ignored the return value. This now checks for `-1` before logging, which is the same check the `actions/cache` save step uses (`if (cacheId != -1) core.info("Cache saved with key: ...")`). `@actions/cache` already logs the reason, so nothing else is logged on failure, and the exit code stays the same. This only changes the log line. It doesn't address the underlying tar/gzip failure in #769. One limitation: with the legacy (v1) cache service, an upload that fails after the cache was reserved still returns the reserved id, so that one case is still reported as saved. That's on the `@actions/cache` side and affects `actions/cache` the same way. Related to #769 ## Test plan - Added `__tests__/cache/save-cache.test.ts`. The `-1` case fails before the change and passes after it. - `npm run all` on Ubuntu (WSL) with Node 24.19.0 / npm 11.17.0: build, Biome (no fixes), packaging, and all 161 tests across 16 suites pass. The regenerated `dist/save-cache/index.cjs` only differs in the changed lines. Co-authored-by: Claude Opus 5.5 --- __tests__/cache/save-cache.test.ts | 87 ++++++++++++++++++++++++++++++ dist/save-cache/index.cjs | 6 ++- src/save-cache.ts | 6 ++- 3 files changed, 95 insertions(+), 4 deletions(-) create mode 100644 __tests__/cache/save-cache.test.ts diff --git a/__tests__/cache/save-cache.test.ts b/__tests__/cache/save-cache.test.ts new file mode 100644 index 0000000..b276c7b --- /dev/null +++ b/__tests__/cache/save-cache.test.ts @@ -0,0 +1,87 @@ +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + afterAll, + beforeEach, + describe, + expect, + it, + jest, +} from "@jest/globals"; +import { CacheLocalSource, type SetupInputs } from "../../src/utils/inputs"; +import { createSetupInputs } from "../helpers/setup-inputs"; + +const mockSaveCache = jest.fn<() => Promise>(); +const mockInfo = jest.fn(); +const mockLoadInputs = jest.fn<() => SetupInputs>(() => + createSetupInputs({ enableCache: false }), +); +const state: Record = {}; + +jest.unstable_mockModule("@actions/cache", () => ({ + saveCache: mockSaveCache, +})); + +jest.unstable_mockModule("@actions/core", () => ({ + getState: jest.fn((name: string) => state[name] ?? ""), + setFailed: jest.fn(), +})); + +jest.unstable_mockModule("../../src/utils/inputs", () => ({ + loadInputs: mockLoadInputs, +})); + +jest.unstable_mockModule("../../src/utils/logging", () => ({ + info: mockInfo, + warning: jest.fn(), +})); + +jest.spyOn(process, "exit").mockImplementation((() => undefined) as never); + +const { run } = await import("../../src/save-cache"); + +const cacheDir = mkdtempSync(join(tmpdir(), "setup-uv-save-cache-")); +const cacheKey = "setup-uv-2-x86_64-unknown-linux-gnu-ubuntu-24.04-3.12-hash"; + +function infoMessages(): string[] { + return mockInfo.mock.calls.map(([msg]) => msg as string); +} + +beforeEach(() => { + delete process.env.UV_CACHE_DIR; + state["cache-key"] = cacheKey; + state["cache-matched-key"] = ""; + mockLoadInputs.mockReturnValue( + createSetupInputs({ + cacheLocalPath: { path: cacheDir, source: CacheLocalSource.Input }, + }), + ); +}); + +afterAll(() => { + rmSync(cacheDir, { force: true, recursive: true }); +}); + +describe("saveCache", () => { + it("reports the saved key when the cache is saved", async () => { + mockSaveCache.mockResolvedValue(42); + + await run(); + + expect(mockSaveCache).toHaveBeenCalledWith([cacheDir], cacheKey); + expect(infoMessages()).toContain(`uv cache saved with key: ${cacheKey}`); + }); + + it("does not report the cache as saved when saving fails", async () => { + // @actions/cache logs the failure itself and returns -1 + mockSaveCache.mockResolvedValue(-1); + + await run(); + + expect(mockSaveCache).toHaveBeenCalledWith([cacheDir], cacheKey); + expect(infoMessages()).not.toContain( + `uv cache saved with key: ${cacheKey}`, + ); + }); +}); diff --git a/dist/save-cache/index.cjs b/dist/save-cache/index.cjs index 49a5299..1d25968 100644 --- a/dist/save-cache/index.cjs +++ b/dist/save-cache/index.cjs @@ -66671,8 +66671,10 @@ async function saveCacheToKey(cacheKey, cachePath, stateKey, cacheName) { return; } info2(`Including ${cacheName} path: ${cachePath}`); - await saveCache2([cachePath], cacheKey); - info2(`${cacheName} saved with key: ${cacheKey}`); + const cacheId = await saveCache2([cachePath], cacheKey); + if (cacheId !== -1) { + info2(`${cacheName} saved with key: ${cacheKey}`); + } } __name(saveCacheToKey, "saveCacheToKey"); run(); diff --git a/src/save-cache.ts b/src/save-cache.ts index 80b15d6..bc75adb 100644 --- a/src/save-cache.ts +++ b/src/save-cache.ts @@ -159,8 +159,10 @@ async function saveCacheToKey( } log.info(`Including ${cacheName} path: ${cachePath}`); - await cache.saveCache([cachePath], cacheKey); - log.info(`${cacheName} saved with key: ${cacheKey}`); + const cacheId = await cache.saveCache([cachePath], cacheKey); + if (cacheId !== -1) { + log.info(`${cacheName} saved with key: ${cacheKey}`); + } } run();