refactor: simplify split-cache handling

Restore cache entries sequentially, keep saves parallel with per-entry error handling, and simplify the cache test setup.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
qmuntal
2026-11-09 12:29:31 +01:00
co-authored by Copilot
parent aa2cac01fd
commit 57097ea729
9 changed files with 74 additions and 421 deletions
+10 -51
View File
@@ -2,7 +2,6 @@
import {jest, describe, it, expect, beforeEach, afterEach} from '@jest/globals';
import fs from 'fs';
import path from 'path';
jest.unstable_mockModule('@actions/cache', () => ({
saveCache: jest.fn(),
@@ -88,10 +87,6 @@ describe('restoreCache', () => {
];
const originalEnv = {...process.env};
const originalArch = Object.getOwnPropertyDescriptor(process, 'arch')!;
const originalPlatform = Object.getOwnPropertyDescriptor(
process,
'platform'
)!;
beforeEach(() => {
jest.resetAllMocks();
@@ -102,7 +97,6 @@ describe('restoreCache', () => {
ImageOS: 'ubuntu24'
};
Object.defineProperty(process, 'arch', {value: 'x64'});
Object.defineProperty(process, 'platform', {value: 'linux'});
hashFilesSpy.mockResolvedValue('file_hash');
getCacheDirectoryPathSpy.mockResolvedValue(directories);
restoreCacheSpy.mockResolvedValue(undefined);
@@ -112,7 +106,6 @@ describe('restoreCache', () => {
afterEach(() => {
process.env = {...originalEnv};
Object.defineProperty(process, 'arch', originalArch);
Object.defineProperty(process, 'platform', originalPlatform);
jest.restoreAllMocks();
});
@@ -153,15 +146,9 @@ describe('restoreCache', () => {
expect(infoSpy).toHaveBeenCalledWith('build cache is not found');
});
it.each([
{platform: 'linux', firstFails: false},
{platform: 'linux', firstFails: true},
{platform: 'win32', firstFails: false},
{platform: 'win32', firstFails: true}
])(
'starts both restores concurrently on $platform and waits for both (first fails=$firstFails)',
async ({platform, firstFails}) => {
Object.defineProperty(process, 'platform', {value: platform});
it.each([false, true])(
'restores entries sequentially and waits for both (first fails=%s)',
async firstFails => {
const started = [deferred(), deferred()];
const release = [deferred(), deferred()];
let completed = false;
@@ -179,10 +166,14 @@ describe('restoreCache', () => {
completed = true;
});
try {
await Promise.all(started.map(signal => signal.promise));
await started[0].promise;
expect(restoreCacheSpy).toHaveBeenCalledTimes(1);
expect(core.saveState).not.toHaveBeenCalled();
expect(setOutputSpy).not.toHaveBeenCalledWith('cache-hit', true);
release[0].resolve();
// Let the first completion propagate while the second stays blocked.
await new Promise<void>(resolve => setImmediate(resolve));
await started[1].promise;
expect(restoreCacheSpy).toHaveBeenCalledTimes(2);
expect(completed).toBe(false);
expect(core.saveState).not.toHaveBeenCalled();
expect(setOutputSpy).not.toHaveBeenCalledWith('cache-hit', true);
@@ -196,38 +187,6 @@ describe('restoreCache', () => {
}
);
it.each(['linux', 'win32'])(
'restores overlapping directories sequentially on %s',
async platform => {
Object.defineProperty(process, 'platform', {value: platform});
getCacheDirectoryPathSpy.mockResolvedValue([
directories[0],
{kind: 'build', path: path.join(modulePath, 'build')}
]);
const firstStarted = deferred();
const releaseFirst = deferred();
restoreCacheSpy.mockImplementation(async (paths, key) => {
if (paths[0] === modulePath) {
firstStarted.resolve();
await releaseFirst.promise;
}
return key;
});
const pending = restore();
try {
await firstStarted.promise;
expect(restoreCacheSpy).toHaveBeenCalledTimes(1);
} finally {
releaseFirst.resolve();
await pending;
}
expect(restoreCacheSpy).toHaveBeenCalledTimes(2);
expect(setOutputSpy).toHaveBeenLastCalledWith('cache-hit', true);
}
);
it.each([
{modules: false, build: false},
{modules: true, build: false},
+12 -33
View File
@@ -41,16 +41,17 @@ function deferred() {
describe('cache-save', () => {
let entries: CacheEntry[];
let getBooleanInputSpy: jest.Mock<typeof core.getBooleanInput>;
let getStateSpy: jest.Mock<typeof core.getState>;
let infoSpy: jest.Mock<typeof core.info>;
let warningSpy: jest.Mock<typeof core.warning>;
let debugSpy: jest.Mock<typeof core.debug>;
let setFailedSpy: jest.Mock<typeof core.setFailed>;
let saveCacheSpy: jest.Mock<typeof cache.saveCache>;
let existsSpy: jest.Mock<typeof fs.existsSync>;
const getBooleanInputSpy = jest.mocked(core.getBooleanInput);
const getStateSpy = jest.mocked(core.getState);
const infoSpy = jest.mocked(core.info);
const warningSpy = jest.mocked(core.warning);
const debugSpy = jest.mocked(core.debug);
const setFailedSpy = jest.mocked(core.setFailed);
const saveCacheSpy = jest.mocked(cache.saveCache);
const existsSpy = jest.mocked(fs.existsSync);
beforeEach(() => {
jest.resetAllMocks();
entries = [
{
kind: 'modules',
@@ -64,36 +65,14 @@ describe('cache-save', () => {
}
];
getBooleanInputSpy = core.getBooleanInput as jest.Mock<
typeof core.getBooleanInput
>;
getBooleanInputSpy.mockReturnValue(true);
getStateSpy = core.getState as jest.Mock<typeof core.getState>;
getStateSpy.mockImplementation(() => JSON.stringify(entries));
infoSpy = core.info as jest.Mock<typeof core.info>;
infoSpy.mockImplementation(() => undefined);
warningSpy = core.warning as jest.Mock<typeof core.warning>;
warningSpy.mockImplementation(() => undefined);
debugSpy = core.debug as jest.Mock<typeof core.debug>;
debugSpy.mockImplementation(() => undefined);
setFailedSpy = core.setFailed as jest.Mock<typeof core.setFailed>;
setFailedSpy.mockImplementation(() => undefined);
saveCacheSpy = cache.saveCache as jest.Mock<typeof cache.saveCache>;
saveCacheSpy.mockImplementation(() => Promise.resolve(0));
existsSpy = fs.existsSync as jest.Mock<typeof fs.existsSync>;
existsSpy.mockImplementation(() => true);
saveCacheSpy.mockResolvedValue(0);
existsSpy.mockReturnValue(true);
});
afterEach(() => {
jest.restoreAllMocks();
jest.clearAllMocks();
});
it('does not save cache when the cache input is false', async () => {
@@ -146,7 +125,7 @@ describe('cache-save', () => {
);
it('does not save cache when there are no cache folders on the disk', async () => {
existsSpy.mockImplementation(() => false);
existsSpy.mockReturnValue(false);
await run();
+1 -144
View File
@@ -1,14 +1,4 @@
import {
jest,
describe,
it,
expect,
beforeEach,
afterEach,
afterAll
} from '@jest/globals';
import fs from 'fs';
import path from 'path';
import {jest, describe, it, expect, beforeEach, afterAll} from '@jest/globals';
jest.unstable_mockModule('@actions/exec', () => ({
exec: jest.fn(),
@@ -212,139 +202,6 @@ describe('getCacheDirectoryPath', () => {
});
});
describe('canRestoreCachesInParallel', () => {
const originalPlatform = Object.getOwnPropertyDescriptor(
process,
'platform'
)!;
const directoryStat = fs.lstatSync(process.cwd());
beforeEach(() => {
Object.defineProperty(process, 'platform', {value: 'linux'});
jest.spyOn(fs, 'lstatSync').mockReturnValue(directoryStat);
jest
.spyOn(fs, 'realpathSync')
.mockImplementation(directory => path.resolve(directory.toString()));
});
afterEach(() => {
Object.defineProperty(process, 'platform', originalPlatform);
jest.restoreAllMocks();
});
it.each([
{modules: 'cache/modules', build: 'cache/build', parallel: true},
{modules: 'cache', build: 'cache-build', parallel: true},
{modules: 'cache', build: 'cache', parallel: false},
{modules: 'cache', build: 'cache/build', parallel: false},
{modules: 'cache/modules', build: 'cache', parallel: false},
{modules: 'cache', build: 'cache/../cache', parallel: false}
])(
'checks overlapping paths: $modules and $build',
({modules, build, parallel}) => {
expect(
cacheUtils.canRestoreCachesInParallel([
{kind: 'modules', path: modules},
{kind: 'build', path: build}
])
).toBe(parallel);
}
);
it('allows concurrent Windows restores with disjoint paths', () => {
Object.defineProperty(process, 'platform', {value: 'win32'});
expect(
cacheUtils.canRestoreCachesInParallel([
{kind: 'modules', path: 'cache/modules'},
{kind: 'build', path: 'cache/build'}
])
).toBe(true);
});
it.each(['darwin', 'win32'])(
'handles case-insensitive paths conservatively on %s',
platform => {
Object.defineProperty(process, 'platform', {value: platform});
expect(
cacheUtils.canRestoreCachesInParallel([
{kind: 'modules', path: 'cache'},
{kind: 'build', path: 'CACHE/build'}
])
).toBe(false);
}
);
it.each([true, false])(
'serializes symlink aliases with missing cache directories (overlap=%s)',
overlap => {
const alias = path.resolve('alias');
const target = path.resolve('target');
jest.spyOn(fs, 'lstatSync').mockImplementation(directory => {
if (directory !== alias && directory !== target) {
throw Object.assign(new Error('Missing path'), {code: 'ENOENT'});
}
return directoryStat;
});
jest.spyOn(fs, 'realpathSync').mockReturnValue(target);
expect(
cacheUtils.canRestoreCachesInParallel([
{kind: 'modules', path: path.join(alias, 'modules')},
{
kind: 'build',
path: path.join(target, overlap ? 'modules/build' : 'build')
}
])
).toBe(false);
}
);
it.each(['nested alias', 'alias through module cache'])(
'serializes restores when the %s resolves outside the module cache',
scenario => {
const modules = path.resolve('modules');
const target = path.resolve('external-build');
const alias = path.join(modules, 'build-link');
const build =
scenario === 'nested alias'
? path.join(alias, 'cache')
: path.resolve('build-alias', 'cache');
jest.spyOn(fs, 'realpathSync').mockImplementation(directory => {
const absolute = path.resolve(directory.toString());
if (absolute === build) {
return path.join(target, 'cache');
}
if (absolute === path.dirname(build)) {
return scenario === 'nested alias' ? target : alias;
}
return absolute;
});
expect(
cacheUtils.canRestoreCachesInParallel([
{kind: 'modules', path: modules},
{kind: 'build', path: build}
])
).toBe(false);
}
);
it('falls back to sequential restores when path resolution fails', () => {
jest.spyOn(fs, 'realpathSync').mockImplementation(() => {
throw new Error('Cannot resolve path');
});
expect(
cacheUtils.canRestoreCachesInParallel([
{kind: 'modules', path: 'cache/modules'},
{kind: 'build', path: 'cache/build'}
])
).toBe(false);
});
});
describe('isCacheFeatureAvailable', () => {
//Arrange
const isFeatureAvailableSpy = cache.isFeatureAvailable as jest.Mock<
+23 -25
View File
@@ -96684,35 +96684,33 @@ const cachePackages = async () => {
info('Cache keys were not generated. Please check the log messages above for more errors or information');
return;
}
await Promise.all(entries.map(async (entry) => {
try {
await saveCacheEntry(entry);
}
catch (error) {
const message = error instanceof Error ? error.message : String(error);
warning(`Save ${entry.kind} cache failed: ${message}`);
}
}));
await Promise.all(entries.map(saveCacheEntry));
};
const saveCacheEntry = async (entry) => {
const { kind, path, primaryKey, matchedKey } = entry;
if (primaryKey === matchedKey) {
info(`Cache hit occurred on the primary key ${primaryKey}, not saving cache.`);
return;
try {
const { kind, path, primaryKey, matchedKey } = entry;
if (primaryKey === matchedKey) {
info(`Cache hit occurred on the primary key ${primaryKey}, not saving cache.`);
return;
}
if (!external_fs_default().existsSync(path)) {
logWarning(`${kind} cache folder doesn't exist on disk: ${path}`);
return;
}
const cacheId = await cache_saveCache([path], primaryKey);
if (cacheId === -1) {
// saveCache returns -1 without throwing when the cache was not saved, e.g.
// a reserve collision or a read-only token (fork PR). @actions/cache has
// already logged the reason at the appropriate severity, so just trace it.
core_debug(`Cache was not saved for the key: ${primaryKey}`);
return;
}
info(`Cache saved with the key: ${primaryKey}`);
}
if (!external_fs_default().existsSync(path)) {
logWarning(`${kind} cache folder doesn't exist on disk: ${path}`);
return;
catch (error) {
const message = error instanceof Error ? error.message : String(error);
warning(`Save ${entry.kind} cache failed: ${message}`);
}
const cacheId = await cache_saveCache([path], primaryKey);
if (cacheId === -1) {
// saveCache returns -1 without throwing when the cache was not saved, e.g.
// a reserve collision or a read-only token (fork PR). @actions/cache has
// already logged the reason at the appropriate severity, so just trace it.
core_debug(`Cache was not saved for the key: ${primaryKey}`);
return;
}
info(`Cache saved with the key: ${primaryKey}`);
};
function logWarning(message) {
const warningPrefix = '[warning]';
+1 -63
View File
@@ -97991,8 +97991,6 @@ const supportedPackageManagers = {
const getCommandOutput = async (toolCommand) => {
let { stdout, stderr, exitCode } = await getExecOutput(toolCommand, undefined, { ignoreReturnCode: true });
if (exitCode) {
@@ -98031,58 +98029,6 @@ const getCacheDirectoryPath = async (packageManagerInfo) => {
}
return cachePaths;
};
function canRestoreCachesInParallel(directories) {
try {
const paths = directories.map(directory => {
const resolvedPath = resolveCachePath(directory.path);
// Conservatively handle case-insensitive Windows/macOS filesystems.
return process.platform === 'darwin' || process.platform === 'win32'
? resolvedPath.toLowerCase()
: resolvedPath;
});
// A cache archive can replace symlinks traversed by the other restore.
// Canonical destinations alone do not capture that extraction footprint.
if (paths.some((resolvedPath, index) => {
const absolutePath = external_path_default().resolve(directories[index].path);
const normalizedPath = process.platform === 'darwin' || process.platform === 'win32'
? absolutePath.toLowerCase()
: absolutePath;
return resolvedPath !== normalizedPath;
})) {
return false;
}
return paths.every((parent, index) => paths.every((child, otherIndex) => {
if (index === otherIndex) {
return true;
}
const relative = external_path_default().relative(parent, child);
return (relative === '..' ||
relative.startsWith(`..${(external_path_default()).sep}`) ||
external_path_default().isAbsolute(relative));
}));
}
catch {
// If the paths cannot be resolved safely, avoid concurrent extraction.
return false;
}
}
function resolveCachePath(directory) {
const absolutePath = external_path_default().resolve(directory);
try {
external_fs_default().lstatSync(absolutePath);
}
catch (error) {
const parent = external_path_default().dirname(absolutePath);
if (error.code !== 'ENOENT' ||
parent === absolutePath) {
throw error;
}
// Cold cache directories may not exist yet. Resolve their existing
// ancestors so symlink aliases still participate in the overlap check.
return external_path_default().join(resolveCachePath(parent), external_path_default().basename(absolutePath));
}
return external_fs_default().realpathSync(absolutePath);
}
function cache_utils_isGhes() {
const ghUrl = new URL(process.env['GITHUB_SERVER_URL'] || 'https://github.com');
const hostname = ghUrl.hostname.trimEnd().toUpperCase();
@@ -98135,7 +98081,7 @@ const cache_restore_restoreCache = async (versionSpec, packageManager, cacheDepe
...directory,
primaryKey: primaryKeys[directory.kind]
}));
const restoreEntry = async (entry) => {
for (const entry of entries) {
core_debug(`${entry.kind} primary key is ${entry.primaryKey}`);
try {
entry.matchedKey = await restoreCache([entry.path], entry.primaryKey);
@@ -98150,14 +98096,6 @@ const cache_restore_restoreCache = async (versionSpec, packageManager, cacheDepe
const message = error instanceof Error ? error.message : String(error);
warning(`Restore ${entry.kind} cache failed: ${message}`);
}
};
if (canRestoreCachesInParallel(entries)) {
await Promise.all(entries.map(restoreEntry));
}
else {
for (const entry of entries) {
await restoreEntry(entry);
}
}
// Preserve the resolved paths so the post step uses the same cache versions,
// even if a later step changes the Go environment.
+1 -1
View File
@@ -248,7 +248,7 @@ The action restores and saves two independent cache entries:
Both entries use the content hash of files listed by `cache-dependency-path`, or the root `go.mod` by default. Adding source files or build settings to that input invalidates both entries when those files change. Custom cache locations and the cache service's normal access restrictions still apply.
Each entry is restored and saved independently. An exact hit is not uploaded again. A failure for one entry does not prevent caching the other. The `cache-hit` output is `true` only when both entries are restored on their primary keys.
Each entry is restored and saved independently. Restores run sequentially to avoid concurrent extraction into overlapping custom cache directories. An exact hit is not uploaded again. A failure for one entry does not prevent caching the other. The `cache-hit` output is `true` only when both entries are restored on their primary keys.
### Caching in monorepos
+1 -10
View File
@@ -8,7 +8,6 @@ import {State, Outputs} from './constants.js';
import {PackageManagerInfo} from './package-managers.js';
import {
CacheEntry,
canRestoreCachesInParallel,
getCacheDirectoryPath,
getPackageManagerInfo
} from './cache-utils.js';
@@ -49,7 +48,7 @@ export const restoreCache = async (
primaryKey: primaryKeys[directory.kind]
}));
const restoreEntry = async (entry: CacheEntry) => {
for (const entry of entries) {
core.debug(`${entry.kind} primary key is ${entry.primaryKey}`);
try {
entry.matchedKey = await cache.restoreCache(
@@ -65,14 +64,6 @@ export const restoreCache = async (
const message = error instanceof Error ? error.message : String(error);
core.warning(`Restore ${entry.kind} cache failed: ${message}`);
}
};
if (canRestoreCachesInParallel(entries)) {
await Promise.all(entries.map(restoreEntry));
} else {
for (const entry of entries) {
await restoreEntry(entry);
}
}
// Preserve the resolved paths so the post step uses the same cache versions,
+25 -29
View File
@@ -49,41 +49,37 @@ const cachePackages = async () => {
return;
}
await Promise.all(
entries.map(async entry => {
try {
await saveCacheEntry(entry);
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
core.warning(`Save ${entry.kind} cache failed: ${message}`);
}
})
);
await Promise.all(entries.map(saveCacheEntry));
};
const saveCacheEntry = async (entry: CacheEntry) => {
const {kind, path, primaryKey, matchedKey} = entry;
if (primaryKey === matchedKey) {
core.info(
`Cache hit occurred on the primary key ${primaryKey}, not saving cache.`
);
return;
}
try {
const {kind, path, primaryKey, matchedKey} = entry;
if (primaryKey === matchedKey) {
core.info(
`Cache hit occurred on the primary key ${primaryKey}, not saving cache.`
);
return;
}
if (!fs.existsSync(path)) {
logWarning(`${kind} cache folder doesn't exist on disk: ${path}`);
return;
}
if (!fs.existsSync(path)) {
logWarning(`${kind} cache folder doesn't exist on disk: ${path}`);
return;
}
const cacheId = await cache.saveCache([path], primaryKey);
if (cacheId === -1) {
// saveCache returns -1 without throwing when the cache was not saved, e.g.
// a reserve collision or a read-only token (fork PR). @actions/cache has
// already logged the reason at the appropriate severity, so just trace it.
core.debug(`Cache was not saved for the key: ${primaryKey}`);
return;
const cacheId = await cache.saveCache([path], primaryKey);
if (cacheId === -1) {
// saveCache returns -1 without throwing when the cache was not saved, e.g.
// a reserve collision or a read-only token (fork PR). @actions/cache has
// already logged the reason at the appropriate severity, so just trace it.
core.debug(`Cache was not saved for the key: ${primaryKey}`);
return;
}
core.info(`Cache saved with the key: ${primaryKey}`);
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
core.warning(`Save ${entry.kind} cache failed: ${message}`);
}
core.info(`Cache saved with the key: ${primaryKey}`);
};
function logWarning(message: string): void {
-65
View File
@@ -1,8 +1,6 @@
import * as cache from '@actions/cache';
import * as core from '@actions/core';
import * as exec from '@actions/exec';
import fs from 'fs';
import path from 'path';
import {
supportedPackageManagers,
PackageManagerInfo,
@@ -75,69 +73,6 @@ export const getCacheDirectoryPath = async (
return cachePaths;
};
export function canRestoreCachesInParallel(
directories: CacheDirectory[]
): boolean {
try {
const paths = directories.map(directory => {
const resolvedPath = resolveCachePath(directory.path);
// Conservatively handle case-insensitive Windows/macOS filesystems.
return process.platform === 'darwin' || process.platform === 'win32'
? resolvedPath.toLowerCase()
: resolvedPath;
});
// A cache archive can replace symlinks traversed by the other restore.
// Canonical destinations alone do not capture that extraction footprint.
if (
paths.some((resolvedPath, index) => {
const absolutePath = path.resolve(directories[index].path);
const normalizedPath =
process.platform === 'darwin' || process.platform === 'win32'
? absolutePath.toLowerCase()
: absolutePath;
return resolvedPath !== normalizedPath;
})
) {
return false;
}
return paths.every((parent, index) =>
paths.every((child, otherIndex) => {
if (index === otherIndex) {
return true;
}
const relative = path.relative(parent, child);
return (
relative === '..' ||
relative.startsWith(`..${path.sep}`) ||
path.isAbsolute(relative)
);
})
);
} catch {
// If the paths cannot be resolved safely, avoid concurrent extraction.
return false;
}
}
function resolveCachePath(directory: string): string {
const absolutePath = path.resolve(directory);
try {
fs.lstatSync(absolutePath);
} catch (error) {
const parent = path.dirname(absolutePath);
if (
(error as NodeJS.ErrnoException).code !== 'ENOENT' ||
parent === absolutePath
) {
throw error;
}
// Cold cache directories may not exist yet. Resolve their existing
// ancestors so symlink aliases still participate in the overlap check.
return path.join(resolveCachePath(parent), path.basename(absolutePath));
}
return fs.realpathSync(absolutePath);
}
export function isGhes(): boolean {
const ghUrl = new URL(
process.env['GITHUB_SERVER_URL'] || 'https://github.com'