From 422c071dabd1c49aa97cf073fd25773a61f2b530 Mon Sep 17 00:00:00 2001 From: dhvcc <1337kwiz@gmail.com> Date: Sun, 3 Apr 2022 01:42:29 +0300 Subject: [PATCH 1/6] Remove useless await in getCacheDistributor calls --- __tests__/cache-restore.test.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/__tests__/cache-restore.test.ts b/__tests__/cache-restore.test.ts index 4dce56a17..7b4dcf02c 100644 --- a/__tests__/cache-restore.test.ts +++ b/__tests__/cache-restore.test.ts @@ -27,6 +27,7 @@ virtualenvs.path = "{cache-dir}/virtualenvs" # /Users/patrick/Library/Caches/py let debugSpy: jest.SpyInstance; let saveSatetSpy: jest.SpyInstance; let getStateSpy: jest.SpyInstance; + let setOutputSpy: jest.SpyInstance; // cache spy let restoreCacheSpy: jest.SpyInstance; @@ -64,6 +65,9 @@ virtualenvs.path = "{cache-dir}/virtualenvs" # /Users/patrick/Library/Caches/py return {stdout: '', stderr: 'Error occured', exitCode: 2}; }); + setOutputSpy = jest.spyOn(core, 'setOutput'); + setOutputSpy.mockImplementation(input => undefined); + restoreCacheSpy = jest.spyOn(cache, 'restoreCache'); restoreCacheSpy.mockImplementation( (cachePaths: string[], primaryKey: string, restoreKey?: string) => { @@ -100,7 +104,7 @@ virtualenvs.path = "{cache-dir}/virtualenvs" # /Users/patrick/Library/Caches/py ])( 'restored dependencies for %s by primaryKey', async (packageManager, pythonVersion, dependencyFile, fileHash) => { - const cacheDistributor = await getCacheDistributor( + const cacheDistributor = getCacheDistributor( packageManager, pythonVersion, dependencyFile @@ -126,7 +130,7 @@ virtualenvs.path = "{cache-dir}/virtualenvs" # /Users/patrick/Library/Caches/py dependencyFile, cacheDependencyPath ) => { - const cacheDistributor = await getCacheDistributor( + const cacheDistributor = getCacheDistributor( packageManager, pythonVersion, dependencyFile @@ -162,7 +166,7 @@ virtualenvs.path = "{cache-dir}/virtualenvs" # /Users/patrick/Library/Caches/py return primaryKey !== fileHash && restoreKey ? pipFileLockHash : ''; } ); - const cacheDistributor = await getCacheDistributor( + const cacheDistributor = getCacheDistributor( packageManager, pythonVersion, dependencyFile From c933f3c50ea2d98bc61f4112402f951e73d26ed8 Mon Sep 17 00:00:00 2001 From: dhvcc <1337kwiz@gmail.com> Date: Sun, 3 Apr 2022 02:33:19 +0300 Subject: [PATCH 2/6] Added cache-hit output --- __tests__/cache-restore.test.ts | 31 ++++++++++++++++++++ action.yml | 2 ++ src/cache-distributions/cache-distributor.ts | 8 +++++ src/utils.ts | 9 ++++++ 4 files changed, 50 insertions(+) diff --git a/__tests__/cache-restore.test.ts b/__tests__/cache-restore.test.ts index 7b4dcf02c..55cc571bc 100644 --- a/__tests__/cache-restore.test.ts +++ b/__tests__/cache-restore.test.ts @@ -191,6 +191,37 @@ virtualenvs.path = "{cache-dir}/virtualenvs" # /Users/patrick/Library/Caches/py ); }); + describe('Check if handleMatchResult', () => { + it.each([ + ['pip', '3.8.12', 'requirements.txt', 'someKey', true], + ['pipenv', '3.9.1', 'requirements.txt', 'someKey', true], + ['poetry', '3.8.12', 'requirements.txt', 'someKey', true], + ['pip', '3.9.2', 'requirements.txt', undefined, false], + ['pipenv', '3.8.12', 'requirements.txt', undefined, false], + ['poetry', '3.9.12', 'requirements.txt', undefined, false] + ])( + 'sets correct outputs', + async ( + packageManager, + pythonVersion, + dependencyFile, + matchedKey, + expectedOutputValue + ) => { + const cacheDistributor = getCacheDistributor( + packageManager, + pythonVersion, + dependencyFile + ); + cacheDistributor.handleMatchResult(matchedKey); + expect(setOutputSpy).toHaveBeenCalledWith( + 'cache-hit', + expectedOutputValue + ); + } + ); + }); + afterEach(() => { jest.resetAllMocks(); jest.clearAllMocks(); diff --git a/action.yml b/action.yml index bd9ee686c..bda521dd1 100644 --- a/action.yml +++ b/action.yml @@ -19,6 +19,8 @@ inputs: outputs: python-version: description: "The installed python version. Useful when given a version range as input." + cache-hit: + description: 'A boolean value to indicate a cache entry was found' runs: using: 'node16' main: 'dist/setup/index.js' diff --git a/src/cache-distributions/cache-distributor.ts b/src/cache-distributions/cache-distributor.ts index ef0be0ef1..3b30076c7 100644 --- a/src/cache-distributions/cache-distributor.ts +++ b/src/cache-distributions/cache-distributor.ts @@ -1,5 +1,6 @@ import * as cache from '@actions/cache'; import * as core from '@actions/core'; +import {PromiseReturnType} from '../utils'; export enum State { STATE_CACHE_PRIMARY_KEY = 'cache-primary-key', @@ -41,12 +42,19 @@ abstract class CacheDistributor { restoreKey ); + this.handleMatchResult(matchedKey); + } + + public handleMatchResult( + matchedKey: PromiseReturnType + ) { if (matchedKey) { core.saveState(State.CACHE_MATCHED_KEY, matchedKey); core.info(`Cache restored from key: ${matchedKey}`); } else { core.info(`${this.packageManager} cache is not found`); } + core.setOutput('cache-hit', Boolean(matchedKey)); } } diff --git a/src/utils.ts b/src/utils.ts index eb3a1ba68..f0dca7e18 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -119,3 +119,12 @@ export function isCacheFeatureAvailable(): boolean { return true; } + +// Awaited (typescript 4.5+) polyfill. Not ideal, so use with care +export type AwaitedPolyfill = T extends PromiseLike + ? AwaitedPolyfill + : T; +// Extract return type from promise +export type PromiseReturnType< + T extends (...args: any) => any +> = AwaitedPolyfill>; From fa50ad538807f67c405fa3aa67683425a2488d4d Mon Sep 17 00:00:00 2001 From: dhvcc <1337kwiz@gmail.com> Date: Sun, 3 Apr 2022 02:47:14 +0300 Subject: [PATCH 3/6] Build action with cache-hit output --- dist/cache-save/index.js | 19 ++++++++++++------- dist/setup/index.js | 19 ++++++++++++------- src/cache-distributions/cache-distributor.ts | 1 + 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/dist/cache-save/index.js b/dist/cache-save/index.js index bf48c7a6e..478fd5ed0 100644 --- a/dist/cache-save/index.js +++ b/dist/cache-save/index.js @@ -37234,15 +37234,20 @@ class CacheDistributor { core.saveState(State.CACHE_PATHS, cachePath); core.saveState(State.STATE_CACHE_PRIMARY_KEY, primaryKey); const matchedKey = yield cache.restoreCache(cachePath, primaryKey, restoreKey); - if (matchedKey) { - core.saveState(State.CACHE_MATCHED_KEY, matchedKey); - core.info(`Cache restored from key: ${matchedKey}`); - } - else { - core.info(`${this.packageManager} cache is not found`); - } + this.handleMatchResult(matchedKey); }); } + handleMatchResult(matchedKey) { + if (matchedKey) { + core.saveState(State.CACHE_MATCHED_KEY, matchedKey); + core.info(`Cache restored from key: ${matchedKey}`); + } + else { + core.info(`${this.packageManager} cache is not found`); + } + core.info('cache was hit'); + core.setOutput('cache-hit', Boolean(matchedKey)); + } } exports.default = CacheDistributor; diff --git a/dist/setup/index.js b/dist/setup/index.js index 1c8512e02..fc44f28db 100644 --- a/dist/setup/index.js +++ b/dist/setup/index.js @@ -42579,15 +42579,20 @@ class CacheDistributor { core.saveState(State.CACHE_PATHS, cachePath); core.saveState(State.STATE_CACHE_PRIMARY_KEY, primaryKey); const matchedKey = yield cache.restoreCache(cachePath, primaryKey, restoreKey); - if (matchedKey) { - core.saveState(State.CACHE_MATCHED_KEY, matchedKey); - core.info(`Cache restored from key: ${matchedKey}`); - } - else { - core.info(`${this.packageManager} cache is not found`); - } + this.handleMatchResult(matchedKey); }); } + handleMatchResult(matchedKey) { + if (matchedKey) { + core.saveState(State.CACHE_MATCHED_KEY, matchedKey); + core.info(`Cache restored from key: ${matchedKey}`); + } + else { + core.info(`${this.packageManager} cache is not found`); + } + core.info('cache was hit'); + core.setOutput('cache-hit', Boolean(matchedKey)); + } } exports.default = CacheDistributor; diff --git a/src/cache-distributions/cache-distributor.ts b/src/cache-distributions/cache-distributor.ts index 3b30076c7..c939ec2eb 100644 --- a/src/cache-distributions/cache-distributor.ts +++ b/src/cache-distributions/cache-distributor.ts @@ -54,6 +54,7 @@ abstract class CacheDistributor { } else { core.info(`${this.packageManager} cache is not found`); } + core.info('cache was hit'); core.setOutput('cache-hit', Boolean(matchedKey)); } } From 08415fd28f852ff65459a61cb484b21092edeccf Mon Sep 17 00:00:00 2001 From: dhvcc <1337kwiz@gmail.com> Date: Sun, 3 Apr 2022 18:52:43 +0300 Subject: [PATCH 4/6] Remove PromiseReturnType, add matchedKey == primaryKey check --- dist/cache-save/index.js | 7 +++---- dist/setup/index.js | 7 +++---- src/cache-distributions/cache-distributor.ts | 10 +++------- src/utils.ts | 9 --------- 4 files changed, 9 insertions(+), 24 deletions(-) diff --git a/dist/cache-save/index.js b/dist/cache-save/index.js index 478fd5ed0..12ea09fe9 100644 --- a/dist/cache-save/index.js +++ b/dist/cache-save/index.js @@ -37234,18 +37234,17 @@ class CacheDistributor { core.saveState(State.CACHE_PATHS, cachePath); core.saveState(State.STATE_CACHE_PRIMARY_KEY, primaryKey); const matchedKey = yield cache.restoreCache(cachePath, primaryKey, restoreKey); - this.handleMatchResult(matchedKey); + this.handleMatchResult(matchedKey, primaryKey); }); } - handleMatchResult(matchedKey) { - if (matchedKey) { + handleMatchResult(matchedKey, primaryKey) { + if (matchedKey == primaryKey) { core.saveState(State.CACHE_MATCHED_KEY, matchedKey); core.info(`Cache restored from key: ${matchedKey}`); } else { core.info(`${this.packageManager} cache is not found`); } - core.info('cache was hit'); core.setOutput('cache-hit', Boolean(matchedKey)); } } diff --git a/dist/setup/index.js b/dist/setup/index.js index fc44f28db..b7c141851 100644 --- a/dist/setup/index.js +++ b/dist/setup/index.js @@ -42579,18 +42579,17 @@ class CacheDistributor { core.saveState(State.CACHE_PATHS, cachePath); core.saveState(State.STATE_CACHE_PRIMARY_KEY, primaryKey); const matchedKey = yield cache.restoreCache(cachePath, primaryKey, restoreKey); - this.handleMatchResult(matchedKey); + this.handleMatchResult(matchedKey, primaryKey); }); } - handleMatchResult(matchedKey) { - if (matchedKey) { + handleMatchResult(matchedKey, primaryKey) { + if (matchedKey == primaryKey) { core.saveState(State.CACHE_MATCHED_KEY, matchedKey); core.info(`Cache restored from key: ${matchedKey}`); } else { core.info(`${this.packageManager} cache is not found`); } - core.info('cache was hit'); core.setOutput('cache-hit', Boolean(matchedKey)); } } diff --git a/src/cache-distributions/cache-distributor.ts b/src/cache-distributions/cache-distributor.ts index c939ec2eb..4aaba033a 100644 --- a/src/cache-distributions/cache-distributor.ts +++ b/src/cache-distributions/cache-distributor.ts @@ -1,6 +1,5 @@ import * as cache from '@actions/cache'; import * as core from '@actions/core'; -import {PromiseReturnType} from '../utils'; export enum State { STATE_CACHE_PRIMARY_KEY = 'cache-primary-key', @@ -42,19 +41,16 @@ abstract class CacheDistributor { restoreKey ); - this.handleMatchResult(matchedKey); + this.handleMatchResult(matchedKey, primaryKey); } - public handleMatchResult( - matchedKey: PromiseReturnType - ) { - if (matchedKey) { + public handleMatchResult(matchedKey: string | undefined, primaryKey: string) { + if (matchedKey == primaryKey) { core.saveState(State.CACHE_MATCHED_KEY, matchedKey); core.info(`Cache restored from key: ${matchedKey}`); } else { core.info(`${this.packageManager} cache is not found`); } - core.info('cache was hit'); core.setOutput('cache-hit', Boolean(matchedKey)); } } diff --git a/src/utils.ts b/src/utils.ts index f0dca7e18..eb3a1ba68 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -119,12 +119,3 @@ export function isCacheFeatureAvailable(): boolean { return true; } - -// Awaited (typescript 4.5+) polyfill. Not ideal, so use with care -export type AwaitedPolyfill = T extends PromiseLike - ? AwaitedPolyfill - : T; -// Extract return type from promise -export type PromiseReturnType< - T extends (...args: any) => any -> = AwaitedPolyfill>; From 56d358493b02d27d77fc7a50178ff9c439c2051d Mon Sep 17 00:00:00 2001 From: Alexey <1337kwiz@gmail.com> Date: Sun, 3 Apr 2022 20:41:15 +0300 Subject: [PATCH 5/6] Update cache-distributor.ts --- src/cache-distributions/cache-distributor.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cache-distributions/cache-distributor.ts b/src/cache-distributions/cache-distributor.ts index 4aaba033a..f24c78dab 100644 --- a/src/cache-distributions/cache-distributor.ts +++ b/src/cache-distributions/cache-distributor.ts @@ -45,13 +45,13 @@ abstract class CacheDistributor { } public handleMatchResult(matchedKey: string | undefined, primaryKey: string) { - if (matchedKey == primaryKey) { + if (matchedKey) { core.saveState(State.CACHE_MATCHED_KEY, matchedKey); core.info(`Cache restored from key: ${matchedKey}`); } else { core.info(`${this.packageManager} cache is not found`); } - core.setOutput('cache-hit', Boolean(matchedKey)); + core.setOutput('cache-hit', matchedKey === primaryKey); } } From 168d1eac87c5d23d88a075a8e04a49583fa711f6 Mon Sep 17 00:00:00 2001 From: dhvcc <1337kwiz@gmail.com> Date: Mon, 4 Apr 2022 15:20:52 +0300 Subject: [PATCH 6/6] Fix tests and rebuild --- __tests__/cache-restore.test.ts | 15 ++++++++------- dist/cache-save/index.js | 4 ++-- dist/setup/index.js | 4 ++-- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/__tests__/cache-restore.test.ts b/__tests__/cache-restore.test.ts index 55cc571bc..99b674ef8 100644 --- a/__tests__/cache-restore.test.ts +++ b/__tests__/cache-restore.test.ts @@ -193,12 +193,12 @@ virtualenvs.path = "{cache-dir}/virtualenvs" # /Users/patrick/Library/Caches/py describe('Check if handleMatchResult', () => { it.each([ - ['pip', '3.8.12', 'requirements.txt', 'someKey', true], - ['pipenv', '3.9.1', 'requirements.txt', 'someKey', true], - ['poetry', '3.8.12', 'requirements.txt', 'someKey', true], - ['pip', '3.9.2', 'requirements.txt', undefined, false], - ['pipenv', '3.8.12', 'requirements.txt', undefined, false], - ['poetry', '3.9.12', 'requirements.txt', undefined, false] + ['pip', '3.8.12', 'requirements.txt', 'someKey', 'someKey', true], + ['pipenv', '3.9.1', 'requirements.txt', 'someKey', 'someKey', true], + ['poetry', '3.8.12', 'requirements.txt', 'someKey', 'someKey', true], + ['pip', '3.9.2', 'requirements.txt', undefined, 'someKey', false], + ['pipenv', '3.8.12', 'requirements.txt', undefined, 'someKey', false], + ['poetry', '3.9.12', 'requirements.txt', undefined, 'someKey', false] ])( 'sets correct outputs', async ( @@ -206,6 +206,7 @@ virtualenvs.path = "{cache-dir}/virtualenvs" # /Users/patrick/Library/Caches/py pythonVersion, dependencyFile, matchedKey, + restoredKey, expectedOutputValue ) => { const cacheDistributor = getCacheDistributor( @@ -213,7 +214,7 @@ virtualenvs.path = "{cache-dir}/virtualenvs" # /Users/patrick/Library/Caches/py pythonVersion, dependencyFile ); - cacheDistributor.handleMatchResult(matchedKey); + cacheDistributor.handleMatchResult(matchedKey, restoredKey); expect(setOutputSpy).toHaveBeenCalledWith( 'cache-hit', expectedOutputValue diff --git a/dist/cache-save/index.js b/dist/cache-save/index.js index 12ea09fe9..c1c654f28 100644 --- a/dist/cache-save/index.js +++ b/dist/cache-save/index.js @@ -37238,14 +37238,14 @@ class CacheDistributor { }); } handleMatchResult(matchedKey, primaryKey) { - if (matchedKey == primaryKey) { + if (matchedKey) { core.saveState(State.CACHE_MATCHED_KEY, matchedKey); core.info(`Cache restored from key: ${matchedKey}`); } else { core.info(`${this.packageManager} cache is not found`); } - core.setOutput('cache-hit', Boolean(matchedKey)); + core.setOutput('cache-hit', matchedKey === primaryKey); } } exports.default = CacheDistributor; diff --git a/dist/setup/index.js b/dist/setup/index.js index b7c141851..e4c20d1ad 100644 --- a/dist/setup/index.js +++ b/dist/setup/index.js @@ -42583,14 +42583,14 @@ class CacheDistributor { }); } handleMatchResult(matchedKey, primaryKey) { - if (matchedKey == primaryKey) { + if (matchedKey) { core.saveState(State.CACHE_MATCHED_KEY, matchedKey); core.info(`Cache restored from key: ${matchedKey}`); } else { core.info(`${this.packageManager} cache is not found`); } - core.setOutput('cache-hit', Boolean(matchedKey)); + core.setOutput('cache-hit', matchedKey === primaryKey); } } exports.default = CacheDistributor;