mirror of
https://github.com/actions/setup-go.git
synced 2026-04-07 05:09:25 +00:00
fix: allow multiple invocations with caching enabled
This fix addresses the issue where calling setup-go multiple times with
caching enabled in the same workflow would fail because the second
invocation attempted to save to the same cache key.
Changes:
- Add tracking of processed cache keys using state variables to prevent
duplicate cache save attempts
- Add helper functions in constants.ts for state management:
- getAlreadyCachedKey()/setAlreadyCachedKey(): Track keys already in cache
- getPrimaryCacheKey()/setPrimaryCacheKey(): Track the primary key for
each invocation
- getCachedGoModPath()/setCachedGoModPath(): Track which go.mod was cached
- Modify cache-restore.ts to store state about the cache operation
- Modify cache-save.ts to check if cache was already saved for this
go.mod path before attempting to save again
- Add comprehensive tests for the multiple invocation scenario
This enables workflows that need to setup Go with different configurations
(e.g., different working directories) multiple times without cache
conflicts.
Assisted-By: cagent
This commit is contained in:
parent
27fdb267c1
commit
49fe0b8fcc
7 changed files with 642 additions and 31 deletions
|
|
@ -6,6 +6,7 @@ import fs from 'fs';
|
|||
import * as cacheRestore from '../src/cache-restore';
|
||||
import * as cacheUtils from '../src/cache-utils';
|
||||
import {PackageManagerInfo} from '../src/package-managers';
|
||||
import {State} from '../src/constants';
|
||||
|
||||
describe('restoreCache', () => {
|
||||
let hashFilesSpy: jest.SpyInstance;
|
||||
|
|
@ -13,22 +14,34 @@ describe('restoreCache', () => {
|
|||
let restoreCacheSpy: jest.SpyInstance;
|
||||
let infoSpy: jest.SpyInstance;
|
||||
let setOutputSpy: jest.SpyInstance;
|
||||
let saveStateSpy: jest.SpyInstance;
|
||||
let getStateSpy: jest.SpyInstance;
|
||||
|
||||
const versionSpec = '1.13.1';
|
||||
const packageManager = 'default';
|
||||
const cacheDependencyPath = 'path';
|
||||
|
||||
let originalWorkspace: string | undefined;
|
||||
let stateStore: Record<string, string>;
|
||||
|
||||
beforeEach(() => {
|
||||
originalWorkspace = process.env.GITHUB_WORKSPACE;
|
||||
process.env.GITHUB_WORKSPACE = '/test/workspace';
|
||||
//Arrange
|
||||
stateStore = {};
|
||||
|
||||
hashFilesSpy = jest.spyOn(glob, 'hashFiles');
|
||||
getCacheDirectoryPathSpy = jest.spyOn(cacheUtils, 'getCacheDirectoryPath');
|
||||
restoreCacheSpy = jest.spyOn(cache, 'restoreCache');
|
||||
infoSpy = jest.spyOn(core, 'info');
|
||||
setOutputSpy = jest.spyOn(core, 'setOutput');
|
||||
saveStateSpy = jest
|
||||
.spyOn(core, 'saveState')
|
||||
.mockImplementation((key, value) => {
|
||||
stateStore[key] = value as string;
|
||||
});
|
||||
getStateSpy = jest.spyOn(core, 'getState').mockImplementation(key => {
|
||||
return stateStore[key] || '';
|
||||
});
|
||||
|
||||
getCacheDirectoryPathSpy.mockImplementation(
|
||||
(PackageManager: PackageManagerInfo) => {
|
||||
|
|
@ -46,9 +59,7 @@ describe('restoreCache', () => {
|
|||
});
|
||||
|
||||
it('should throw if dependency file path is not valid', async () => {
|
||||
// Arrange
|
||||
hashFilesSpy.mockImplementation(() => Promise.resolve(''));
|
||||
// Act + Assert
|
||||
await expect(
|
||||
cacheRestore.restoreCache(
|
||||
versionSpec,
|
||||
|
|
@ -61,10 +72,8 @@ describe('restoreCache', () => {
|
|||
});
|
||||
|
||||
it('should inform if cache hit is not occurred', async () => {
|
||||
// Arrange
|
||||
hashFilesSpy.mockImplementation(() => Promise.resolve('file_hash'));
|
||||
restoreCacheSpy.mockImplementation(() => Promise.resolve(''));
|
||||
// Act + Assert
|
||||
await cacheRestore.restoreCache(
|
||||
versionSpec,
|
||||
packageManager,
|
||||
|
|
@ -74,10 +83,8 @@ describe('restoreCache', () => {
|
|||
});
|
||||
|
||||
it('should set output if cache hit is occurred', async () => {
|
||||
// Arrange
|
||||
hashFilesSpy.mockImplementation(() => Promise.resolve('file_hash'));
|
||||
restoreCacheSpy.mockImplementation(() => Promise.resolve('cache_key'));
|
||||
// Act + Assert
|
||||
await cacheRestore.restoreCache(
|
||||
versionSpec,
|
||||
packageManager,
|
||||
|
|
@ -90,13 +97,125 @@ describe('restoreCache', () => {
|
|||
jest.spyOn(fs, 'readdirSync').mockReturnValue(['main.go'] as any);
|
||||
|
||||
await expect(
|
||||
cacheRestore.restoreCache(
|
||||
versionSpec,
|
||||
packageManager
|
||||
// No cacheDependencyPath
|
||||
)
|
||||
cacheRestore.restoreCache(versionSpec, packageManager)
|
||||
).rejects.toThrow(
|
||||
'Dependencies file is not found in /test/workspace. Supported file pattern: go.mod'
|
||||
);
|
||||
});
|
||||
|
||||
describe('multiple invocations', () => {
|
||||
it('should skip restore if same key was already processed', async () => {
|
||||
hashFilesSpy.mockImplementation(() => Promise.resolve('file_hash'));
|
||||
restoreCacheSpy.mockImplementation(() => Promise.resolve('cache_key'));
|
||||
|
||||
// First invocation
|
||||
await cacheRestore.restoreCache(
|
||||
versionSpec,
|
||||
packageManager,
|
||||
cacheDependencyPath
|
||||
);
|
||||
expect(restoreCacheSpy).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Second invocation with same parameters should skip
|
||||
await cacheRestore.restoreCache(
|
||||
versionSpec,
|
||||
packageManager,
|
||||
cacheDependencyPath
|
||||
);
|
||||
|
||||
// restoreCache should not be called again
|
||||
expect(restoreCacheSpy).toHaveBeenCalledTimes(1);
|
||||
expect(infoSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('already processed in this job')
|
||||
);
|
||||
});
|
||||
|
||||
it('should restore cache for different versions', async () => {
|
||||
hashFilesSpy.mockImplementation(() => Promise.resolve('file_hash'));
|
||||
restoreCacheSpy.mockImplementation(() => Promise.resolve('cache_key'));
|
||||
|
||||
// First invocation with version 1.13.1
|
||||
await cacheRestore.restoreCache(
|
||||
'1.13.1',
|
||||
packageManager,
|
||||
cacheDependencyPath
|
||||
);
|
||||
expect(restoreCacheSpy).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Second invocation with different version
|
||||
await cacheRestore.restoreCache(
|
||||
'1.20.0',
|
||||
packageManager,
|
||||
cacheDependencyPath
|
||||
);
|
||||
|
||||
// Both should call restoreCache
|
||||
expect(restoreCacheSpy).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('should accumulate primary keys for multiple invocations', async () => {
|
||||
hashFilesSpy.mockImplementation(() => Promise.resolve('file_hash'));
|
||||
restoreCacheSpy.mockImplementation(() => Promise.resolve(''));
|
||||
|
||||
await cacheRestore.restoreCache(
|
||||
'1.13.1',
|
||||
packageManager,
|
||||
cacheDependencyPath
|
||||
);
|
||||
await cacheRestore.restoreCache(
|
||||
'1.20.0',
|
||||
packageManager,
|
||||
cacheDependencyPath
|
||||
);
|
||||
|
||||
// Check that CachePrimaryKeys state contains both keys
|
||||
const keysJson = stateStore[State.CachePrimaryKeys];
|
||||
expect(keysJson).toBeDefined();
|
||||
const keys = JSON.parse(keysJson);
|
||||
expect(keys).toHaveLength(2);
|
||||
expect(keys[0]).toContain('go-1.13.1');
|
||||
expect(keys[1]).toContain('go-1.20.0');
|
||||
});
|
||||
|
||||
it('should accumulate matched keys for cache hits', async () => {
|
||||
hashFilesSpy.mockImplementation(() => Promise.resolve('file_hash'));
|
||||
restoreCacheSpy
|
||||
.mockImplementationOnce(() => Promise.resolve('cache_key_1'))
|
||||
.mockImplementationOnce(() => Promise.resolve('cache_key_2'));
|
||||
|
||||
await cacheRestore.restoreCache(
|
||||
'1.13.1',
|
||||
packageManager,
|
||||
cacheDependencyPath
|
||||
);
|
||||
await cacheRestore.restoreCache(
|
||||
'1.20.0',
|
||||
packageManager,
|
||||
cacheDependencyPath
|
||||
);
|
||||
|
||||
// Check that CacheMatchedKeys state contains both matched keys
|
||||
const keysJson = stateStore[State.CacheMatchedKeys];
|
||||
expect(keysJson).toBeDefined();
|
||||
const keys = JSON.parse(keysJson);
|
||||
expect(keys).toHaveLength(2);
|
||||
expect(keys).toContain('cache_key_1');
|
||||
expect(keys).toContain('cache_key_2');
|
||||
});
|
||||
|
||||
it('should maintain backward compatibility with legacy state keys', async () => {
|
||||
hashFilesSpy.mockImplementation(() => Promise.resolve('file_hash'));
|
||||
restoreCacheSpy.mockImplementation(() => Promise.resolve('cache_key'));
|
||||
|
||||
await cacheRestore.restoreCache(
|
||||
versionSpec,
|
||||
packageManager,
|
||||
cacheDependencyPath
|
||||
);
|
||||
|
||||
// Legacy keys should still be set
|
||||
expect(stateStore[State.CachePrimaryKey]).toBeDefined();
|
||||
expect(stateStore[State.CacheMatchedKey]).toBe('cache_key');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
220
__tests__/cache-save.test.ts
Normal file
220
__tests__/cache-save.test.ts
Normal file
|
|
@ -0,0 +1,220 @@
|
|||
import * as cache from '@actions/cache';
|
||||
import * as core from '@actions/core';
|
||||
import fs from 'fs';
|
||||
|
||||
import * as cacheSave from '../src/cache-save';
|
||||
import * as cacheUtils from '../src/cache-utils';
|
||||
import {PackageManagerInfo} from '../src/package-managers';
|
||||
import {State} from '../src/constants';
|
||||
|
||||
describe('cache-save', () => {
|
||||
let getCacheDirectoryPathSpy: jest.SpyInstance;
|
||||
let saveCacheSpy: jest.SpyInstance;
|
||||
let infoSpy: jest.SpyInstance;
|
||||
let warningSpy: jest.SpyInstance;
|
||||
let getBooleanInputSpy: jest.SpyInstance;
|
||||
let getStateSpy: jest.SpyInstance;
|
||||
let existsSyncSpy: jest.SpyInstance;
|
||||
|
||||
let stateStore: Record<string, string>;
|
||||
|
||||
beforeEach(() => {
|
||||
stateStore = {};
|
||||
|
||||
getCacheDirectoryPathSpy = jest.spyOn(cacheUtils, 'getCacheDirectoryPath');
|
||||
saveCacheSpy = jest.spyOn(cache, 'saveCache');
|
||||
infoSpy = jest.spyOn(core, 'info').mockImplementation();
|
||||
warningSpy = jest.spyOn(core, 'warning').mockImplementation();
|
||||
getBooleanInputSpy = jest.spyOn(core, 'getBooleanInput');
|
||||
getStateSpy = jest.spyOn(core, 'getState').mockImplementation(key => {
|
||||
return stateStore[key] || '';
|
||||
});
|
||||
existsSyncSpy = jest.spyOn(fs, 'existsSync').mockReturnValue(true);
|
||||
|
||||
getCacheDirectoryPathSpy.mockImplementation(
|
||||
(PackageManager: PackageManagerInfo) => {
|
||||
return Promise.resolve(['/home/runner/go/pkg/mod']);
|
||||
}
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
jest.restoreAllMocks();
|
||||
});
|
||||
|
||||
describe('run', () => {
|
||||
it('should skip cache save when cache input is false', async () => {
|
||||
getBooleanInputSpy.mockReturnValue(false);
|
||||
|
||||
await cacheSave.run(false);
|
||||
|
||||
expect(saveCacheSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should save cache with legacy single key', async () => {
|
||||
getBooleanInputSpy.mockReturnValue(true);
|
||||
stateStore[State.CachePrimaryKey] = 'primary-key-123';
|
||||
stateStore[State.CacheMatchedKey] = '';
|
||||
saveCacheSpy.mockResolvedValue(12345);
|
||||
|
||||
await cacheSave.run(false);
|
||||
|
||||
expect(saveCacheSpy).toHaveBeenCalledTimes(1);
|
||||
expect(saveCacheSpy).toHaveBeenCalledWith(
|
||||
['/home/runner/go/pkg/mod'],
|
||||
'primary-key-123'
|
||||
);
|
||||
});
|
||||
|
||||
it('should skip save when cache hit occurred (legacy mode)', async () => {
|
||||
getBooleanInputSpy.mockReturnValue(true);
|
||||
stateStore[State.CachePrimaryKey] = 'primary-key-123';
|
||||
stateStore[State.CacheMatchedKey] = 'primary-key-123';
|
||||
|
||||
await cacheSave.run(false);
|
||||
|
||||
expect(saveCacheSpy).not.toHaveBeenCalled();
|
||||
expect(infoSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Cache hit occurred on the primary key')
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('multiple invocations', () => {
|
||||
it('should save cache for multiple keys from multiple invocations', async () => {
|
||||
getBooleanInputSpy.mockReturnValue(true);
|
||||
stateStore[State.CachePrimaryKeys] = JSON.stringify([
|
||||
'key-go-1.13.1',
|
||||
'key-go-1.20.0'
|
||||
]);
|
||||
stateStore[State.CacheMatchedKeys] = JSON.stringify(['', '']);
|
||||
saveCacheSpy.mockResolvedValue(12345);
|
||||
|
||||
await cacheSave.run(false);
|
||||
|
||||
expect(saveCacheSpy).toHaveBeenCalledTimes(2);
|
||||
expect(saveCacheSpy).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
['/home/runner/go/pkg/mod'],
|
||||
'key-go-1.13.1'
|
||||
);
|
||||
expect(saveCacheSpy).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
['/home/runner/go/pkg/mod'],
|
||||
'key-go-1.20.0'
|
||||
);
|
||||
});
|
||||
|
||||
it('should skip save for keys that had cache hits', async () => {
|
||||
getBooleanInputSpy.mockReturnValue(true);
|
||||
stateStore[State.CachePrimaryKeys] = JSON.stringify([
|
||||
'key-go-1.13.1',
|
||||
'key-go-1.20.0'
|
||||
]);
|
||||
// First key had a cache hit, second didn't
|
||||
stateStore[State.CacheMatchedKeys] = JSON.stringify([
|
||||
'key-go-1.13.1',
|
||||
''
|
||||
]);
|
||||
saveCacheSpy.mockResolvedValue(12345);
|
||||
|
||||
await cacheSave.run(false);
|
||||
|
||||
// Should only save for the second key
|
||||
expect(saveCacheSpy).toHaveBeenCalledTimes(1);
|
||||
expect(saveCacheSpy).toHaveBeenCalledWith(
|
||||
['/home/runner/go/pkg/mod'],
|
||||
'key-go-1.20.0'
|
||||
);
|
||||
});
|
||||
|
||||
it('should handle cache already exists error gracefully', async () => {
|
||||
getBooleanInputSpy.mockReturnValue(true);
|
||||
stateStore[State.CachePrimaryKeys] = JSON.stringify([
|
||||
'key-go-1.13.1',
|
||||
'key-go-1.20.0'
|
||||
]);
|
||||
stateStore[State.CacheMatchedKeys] = JSON.stringify(['', '']);
|
||||
|
||||
saveCacheSpy
|
||||
.mockRejectedValueOnce(new Error('Cache already exists'))
|
||||
.mockResolvedValueOnce(12345);
|
||||
|
||||
await cacheSave.run(false);
|
||||
|
||||
expect(saveCacheSpy).toHaveBeenCalledTimes(2);
|
||||
expect(infoSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Cache already exists')
|
||||
);
|
||||
expect(infoSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Cache saved with the key: key-go-1.20.0')
|
||||
);
|
||||
});
|
||||
|
||||
it('should handle empty state gracefully', async () => {
|
||||
getBooleanInputSpy.mockReturnValue(true);
|
||||
// No state set
|
||||
|
||||
await cacheSave.run(false);
|
||||
|
||||
expect(saveCacheSpy).not.toHaveBeenCalled();
|
||||
expect(infoSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Primary key was not generated')
|
||||
);
|
||||
});
|
||||
|
||||
it('should prefer multi-key state over legacy single-key state', async () => {
|
||||
getBooleanInputSpy.mockReturnValue(true);
|
||||
// Both legacy and multi-key state present
|
||||
stateStore[State.CachePrimaryKey] = 'legacy-key';
|
||||
stateStore[State.CacheMatchedKey] = '';
|
||||
stateStore[State.CachePrimaryKeys] = JSON.stringify(['multi-key-1']);
|
||||
stateStore[State.CacheMatchedKeys] = JSON.stringify(['']);
|
||||
saveCacheSpy.mockResolvedValue(12345);
|
||||
|
||||
await cacheSave.run(false);
|
||||
|
||||
// Should use multi-key state
|
||||
expect(saveCacheSpy).toHaveBeenCalledTimes(1);
|
||||
expect(saveCacheSpy).toHaveBeenCalledWith(
|
||||
['/home/runner/go/pkg/mod'],
|
||||
'multi-key-1'
|
||||
);
|
||||
});
|
||||
|
||||
it('should log summary for multiple invocations', async () => {
|
||||
getBooleanInputSpy.mockReturnValue(true);
|
||||
stateStore[State.CachePrimaryKeys] = JSON.stringify([
|
||||
'key-go-1.13.1',
|
||||
'key-go-1.20.0',
|
||||
'key-go-1.21.0'
|
||||
]);
|
||||
// First had cache hit, second and third didn't
|
||||
stateStore[State.CacheMatchedKeys] = JSON.stringify([
|
||||
'key-go-1.13.1',
|
||||
'',
|
||||
''
|
||||
]);
|
||||
saveCacheSpy.mockResolvedValue(12345);
|
||||
|
||||
await cacheSave.run(false);
|
||||
|
||||
expect(saveCacheSpy).toHaveBeenCalledTimes(2);
|
||||
expect(infoSpy).toHaveBeenCalledWith(expect.stringContaining('Saved: 2'));
|
||||
});
|
||||
|
||||
it('should warn when cache folder does not exist', async () => {
|
||||
getBooleanInputSpy.mockReturnValue(true);
|
||||
stateStore[State.CachePrimaryKeys] = JSON.stringify(['key-go-1.13.1']);
|
||||
stateStore[State.CacheMatchedKeys] = JSON.stringify(['']);
|
||||
existsSyncSpy.mockReturnValue(false);
|
||||
|
||||
await cacheSave.run(false);
|
||||
|
||||
expect(warningSpy).toHaveBeenCalledWith(
|
||||
'There are no cache folders on the disk'
|
||||
);
|
||||
expect(saveCacheSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue