From 7fe6c4d63194dfa8e9a1e3c8bdcd5f08b63ad5f9 Mon Sep 17 00:00:00 2001 From: Kengo TODA Date: Fri, 16 Jul 2021 07:28:13 +0800 Subject: [PATCH] use the key in state to save caches in the post process --- __tests__/cache.test.ts | 57 ++++++++++++++++++++++++++++++++-- __tests__/cleanup-java.test.ts | 34 ++++++++++++++------ dist/cleanup/index.js | 9 ++++-- dist/setup/index.js | 9 ++++-- src/cache.ts | 10 ++++-- 5 files changed, 102 insertions(+), 17 deletions(-) diff --git a/__tests__/cache.test.ts b/__tests__/cache.test.ts index fbc6dac0..38995b51 100644 --- a/__tests__/cache.test.ts +++ b/__tests__/cache.test.ts @@ -45,6 +45,7 @@ describe('dependency cache', () => { process.chdir(ORIGINAL_CWD); process.env['GITHUB_WORKSPACE'] = ORIGINAL_GITHUB_WORKSPACE; process.env['RUNNER_OS'] = ORIGINAL_RUNNER_OS; + resetState(); }); describe('restore', () => { @@ -126,13 +127,21 @@ describe('dependency cache', () => { describe('for maven', () => { it('uploads cache even if no pom.xml found', async () => { + createStateForMissingBuildFile(); await save('maven'); expect(spyCacheSave).toBeCalled(); expect(spyWarning).not.toBeCalled(); - expect(spyInfo).toBeCalledWith(expect.stringMatching(/^Cache saved with the key:.*/)); + }); + it('does not upload cache if no restore run before', async () => { + createFile(join(workspace, 'pom.xml')); + + await save('maven'); + expect(spyCacheSave).not.toBeCalled(); + expect(spyWarning).toBeCalledWith('Error retrieving key from state.'); }); it('uploads cache', async () => { createFile(join(workspace, 'pom.xml')); + createStateForSuccessfulRestore(); await save('maven'); expect(spyCacheSave).toBeCalled(); @@ -142,13 +151,22 @@ describe('dependency cache', () => { }); describe('for gradle', () => { it('uploads cache even if no build.gradle found', async () => { + createStateForMissingBuildFile(); + await save('gradle'); expect(spyCacheSave).toBeCalled(); expect(spyWarning).not.toBeCalled(); - expect(spyInfo).toBeCalledWith(expect.stringMatching(/^Cache saved with the key:.*/)); + }); + it('does not upload cache if no restore run before', async () => { + createFile(join(workspace, 'build.gradle')); + + await save('gradle'); + expect(spyCacheSave).not.toBeCalled(); + expect(spyWarning).toBeCalledWith('Error retrieving key from state.'); }); it('uploads cache based on build.gradle', async () => { createFile(join(workspace, 'build.gradle')); + createStateForSuccessfulRestore(); await save('gradle'); expect(spyCacheSave).toBeCalled(); @@ -157,6 +175,7 @@ describe('dependency cache', () => { }); it('uploads cache based on build.gradle.kts', async () => { createFile(join(workspace, 'build.gradle.kts')); + createStateForSuccessfulRestore(); await save('gradle'); expect(spyCacheSave).toBeCalled(); @@ -167,6 +186,40 @@ describe('dependency cache', () => { }); }); +function resetState() { + jest.spyOn(core, 'getState').mockReset(); +} + +/** + * Create states to emulate a restore process without build file. + */ +function createStateForMissingBuildFile() { + jest.spyOn(core, 'getState').mockImplementation(name => { + switch (name) { + case 'cache-primary-key': + return 'setup-java-cache-'; + default: + return ''; + } + }); +} + +/** + * Create states to emulate a successful restore process. + */ +function createStateForSuccessfulRestore() { + jest.spyOn(core, 'getState').mockImplementation(name => { + switch (name) { + case 'cache-primary-key': + return 'setup-java-cache-primary-key'; + case 'cache-matched-key': + return 'setup-java-cache-matched-key'; + default: + return ''; + } + }); +} + function createFile(path: string) { core.info(`created a file at ${path}`); fs.writeFileSync(path, ''); diff --git a/__tests__/cleanup-java.test.ts b/__tests__/cleanup-java.test.ts index ff23f9da..658c2aee 100644 --- a/__tests__/cleanup-java.test.ts +++ b/__tests__/cleanup-java.test.ts @@ -1,25 +1,21 @@ -import { mkdtempSync } from 'fs'; -import { tmpdir } from 'os'; -import { join } from 'path'; -import { restore, save } from '../src/cache'; import { run as cleanup } from '../src/cleanup-java'; -import * as fs from 'fs'; -import * as os from 'os'; import * as core from '@actions/core'; import * as cache from '@actions/cache'; describe('cleanup', () => { - let spyInfo: jest.SpyInstance>; let spyWarning: jest.SpyInstance>; - let spyCacheSave: jest.SpyInstance< ReturnType, Parameters >; + beforeEach(() => { - spyInfo = jest.spyOn(core, 'info'); spyWarning = jest.spyOn(core, 'warning'); spyCacheSave = jest.spyOn(cache, 'saveCache'); + createStateForSuccessfulRestore(); + }); + afterEach(() => { + resetState(); }); it('does not fail nor warn even when the save provess throws a ReserveCacheError', async () => { @@ -49,3 +45,23 @@ describe('cleanup', () => { expect(spyCacheSave).toBeCalled(); }); }); + +function resetState() { + jest.spyOn(core, 'getState').mockReset(); +} + +/** + * Create states to emulate a successful restore process. + */ +function createStateForSuccessfulRestore() { + jest.spyOn(core, 'getState').mockImplementation(name => { + switch (name) { + case 'cache-primary-key': + return 'setup-java-cache-primary-key'; + case 'cache-matched-key': + return 'setup-java-cache-matched-key'; + default: + return ''; + } + }); +} diff --git a/dist/cleanup/index.js b/dist/cleanup/index.js index 638db47b..613d2536 100644 --- a/dist/cleanup/index.js +++ b/dist/cleanup/index.js @@ -64602,9 +64602,14 @@ exports.restore = restore; function save(id) { return __awaiter(this, void 0, void 0, function* () { const packageManager = findPackageManager(id); - const primaryKey = yield computeCacheKey(packageManager); const matchedKey = core.getState(CACHE_MATCHED_KEY); - if (matchedKey === primaryKey) { + // Inputs are re-evaluted before the post action, so we want the original key used for restore + const primaryKey = core.getState(STATE_CACHE_PRIMARY_KEY); + if (!primaryKey) { + core.warning('Error retrieving key from state.'); + return; + } + else if (matchedKey === primaryKey) { // no change in target directories core.info(`Cache hit occurred on the primary key ${primaryKey}, not saving cache.`); return; diff --git a/dist/setup/index.js b/dist/setup/index.js index 745c5560..d7e9e49b 100644 --- a/dist/setup/index.js +++ b/dist/setup/index.js @@ -18998,9 +18998,14 @@ exports.restore = restore; function save(id) { return __awaiter(this, void 0, void 0, function* () { const packageManager = findPackageManager(id); - const primaryKey = yield computeCacheKey(packageManager); const matchedKey = core.getState(CACHE_MATCHED_KEY); - if (matchedKey === primaryKey) { + // Inputs are re-evaluted before the post action, so we want the original key used for restore + const primaryKey = core.getState(STATE_CACHE_PRIMARY_KEY); + if (!primaryKey) { + core.warning('Error retrieving key from state.'); + return; + } + else if (matchedKey === primaryKey) { // no change in target directories core.info(`Cache hit occurred on the primary key ${primaryKey}, not saving cache.`); return; diff --git a/src/cache.ts b/src/cache.ts index 86b7b72e..3ac8d658 100644 --- a/src/cache.ts +++ b/src/cache.ts @@ -90,9 +90,15 @@ export async function restore(id: string) { */ export async function save(id: string) { const packageManager = findPackageManager(id); - const primaryKey = await computeCacheKey(packageManager); const matchedKey = core.getState(CACHE_MATCHED_KEY); - if (matchedKey === primaryKey) { + + // Inputs are re-evaluted before the post action, so we want the original key used for restore + const primaryKey = core.getState(STATE_CACHE_PRIMARY_KEY); + + if (!primaryKey) { + core.warning('Error retrieving key from state.'); + return; + } else if (matchedKey === primaryKey) { // no change in target directories core.info(`Cache hit occurred on the primary key ${primaryKey}, not saving cache.`); return;