From 3cfd90a8035ec180d7c74a21ce1d1d4e70e6245d Mon Sep 17 00:00:00 2001 From: Aarni Koskela Date: Fri, 10 Nov 2023 10:18:59 +0200 Subject: [PATCH 1/3] Fix subheadings in caching guide --- docs/advanced-usage.md | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/docs/advanced-usage.md b/docs/advanced-usage.md index 37c3218df..a3ae55cc1 100644 --- a/docs/advanced-usage.md +++ b/docs/advanced-usage.md @@ -293,7 +293,8 @@ steps: ## Caching packages -**Caching pipenv dependencies:** +### Caching pipenv dependencies + ```yaml steps: - uses: actions/checkout@v4 @@ -306,7 +307,8 @@ steps: - run: pipenv install ``` -**Caching poetry dependencies:** +### Caching poetry dependencies + ```yaml steps: - uses: actions/checkout@v4 @@ -320,7 +322,8 @@ steps: - run: poetry run pytest ``` -**Using a list of file paths to cache dependencies** +### Using a list of file paths to cache dependencies + ```yaml steps: - uses: actions/checkout@v4 @@ -335,7 +338,9 @@ steps: run: curl https://raw.githubusercontent.com/pypa/pipenv/master/get-pipenv.py | python - run: pipenv install ``` -**Using wildcard patterns to cache dependencies** + +### Using wildcard patterns to cache dependencies + ```yaml steps: - uses: actions/checkout@v4 @@ -347,7 +352,8 @@ steps: - run: pip install -r subdirectory/requirements-dev.txt ``` -**Using a list of wildcard patterns to cache dependencies** +### Using a list of wildcard patterns to cache dependencies + ```yaml steps: - uses: actions/checkout@v4 @@ -361,7 +367,7 @@ steps: - run: pip install -e . -r subdirectory/requirements-dev.txt ``` -**Caching projects that use setup.py:** +### Caching projects that use setup.py (or pyproject.toml) ```yaml steps: From 7e8faf07b9d24d03183aa04a9936d244d5a6a1a8 Mon Sep 17 00:00:00 2001 From: Aarni Koskela Date: Fri, 10 Nov 2023 15:33:21 +0200 Subject: [PATCH 2/3] Tests: do not mock getInput with an incompatible implementation --- __tests__/cache-save.test.ts | 52 ++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 29 deletions(-) diff --git a/__tests__/cache-save.test.ts b/__tests__/cache-save.test.ts index 26f7da24b..ae55ed151 100644 --- a/__tests__/cache-save.test.ts +++ b/__tests__/cache-save.test.ts @@ -20,7 +20,6 @@ describe('run', () => { let debugSpy: jest.SpyInstance; let saveStateSpy: jest.SpyInstance; let getStateSpy: jest.SpyInstance; - let getInputSpy: jest.SpyInstance; let setFailedSpy: jest.SpyInstance; // cache spy @@ -29,10 +28,17 @@ describe('run', () => { // exec spy let getExecOutputSpy: jest.SpyInstance; - let inputs = {} as any; + function setInput(name: string, value: string): void { + process.env[`INPUT_${name.replace(/ /g, '_').toUpperCase()}`] = value; + } beforeEach(() => { process.env['RUNNER_OS'] = process.env['RUNNER_OS'] ?? 'linux'; + for(const key in process.env) { + if(key.startsWith('INPUT_')) { + delete process.env[key]; + } + } infoSpy = jest.spyOn(core, 'info'); infoSpy.mockImplementation(input => undefined); @@ -56,9 +62,6 @@ describe('run', () => { setFailedSpy = jest.spyOn(core, 'setFailed'); - getInputSpy = jest.spyOn(core, 'getInput'); - getInputSpy.mockImplementation(input => inputs[input]); - getExecOutputSpy = jest.spyOn(exec, 'getExecOutput'); getExecOutputSpy.mockImplementation((input: string) => { if (input.includes('pip')) { @@ -74,10 +77,9 @@ describe('run', () => { describe('Package manager validation', () => { it('Package manager is not provided, skip caching', async () => { - inputs['cache'] = ''; + setInput('cache', ''); await run(); - expect(getInputSpy).toHaveBeenCalled(); expect(infoSpy).not.toHaveBeenCalled(); expect(saveCacheSpy).not.toHaveBeenCalled(); expect(setFailedSpy).not.toHaveBeenCalled(); @@ -86,12 +88,11 @@ describe('run', () => { describe('Validate unchanged cache is not saved', () => { it('should not save cache for pip', async () => { - inputs['cache'] = 'pip'; - inputs['python-version'] = '3.10.0'; + setInput('cache', 'pip'); + setInput('python-version', '3.10.0'); await run(); - expect(getInputSpy).toHaveBeenCalled(); expect(debugSpy).toHaveBeenCalledWith( `paths for caching are ${__dirname}` ); @@ -103,12 +104,11 @@ describe('run', () => { }); it('should not save cache for pipenv', async () => { - inputs['cache'] = 'pipenv'; - inputs['python-version'] = '3.10.0'; + setInput('cache', 'pipenv'); + setInput('python-version', '3.10.0'); await run(); - expect(getInputSpy).toHaveBeenCalled(); expect(debugSpy).toHaveBeenCalledWith( `paths for caching are ${__dirname}` ); @@ -122,8 +122,8 @@ describe('run', () => { describe('action saves the cache', () => { it('saves cache from pip', async () => { - inputs['cache'] = 'pip'; - inputs['python-version'] = '3.10.0'; + setInput('cache', 'pip'); + setInput('python-version', '3.10.0'); getStateSpy.mockImplementation((name: string) => { if (name === State.CACHE_MATCHED_KEY) { return requirementsHash; @@ -136,7 +136,6 @@ describe('run', () => { await run(); - expect(getInputSpy).toHaveBeenCalled(); expect(getStateSpy).toHaveBeenCalledTimes(3); expect(infoSpy).not.toHaveBeenCalledWith( `Cache hit occurred on the primary key ${requirementsHash}, not saving cache.` @@ -149,8 +148,8 @@ describe('run', () => { }); it('saves cache from pipenv', async () => { - inputs['cache'] = 'pipenv'; - inputs['python-version'] = '3.10.0'; + setInput('cache', 'pipenv'); + setInput('python-version', '3.10.0'); getStateSpy.mockImplementation((name: string) => { if (name === State.CACHE_MATCHED_KEY) { return pipFileLockHash; @@ -163,7 +162,6 @@ describe('run', () => { await run(); - expect(getInputSpy).toHaveBeenCalled(); expect(getStateSpy).toHaveBeenCalledTimes(3); expect(infoSpy).not.toHaveBeenCalledWith( `Cache hit occurred on the primary key ${pipFileLockHash}, not saving cache.` @@ -176,8 +174,8 @@ describe('run', () => { }); it('saves cache from poetry', async () => { - inputs['cache'] = 'poetry'; - inputs['python-version'] = '3.10.0'; + setInput('cache', 'poetry'); + setInput('python-version', '3.10.0'); getStateSpy.mockImplementation((name: string) => { if (name === State.CACHE_MATCHED_KEY) { return poetryLockHash; @@ -190,7 +188,6 @@ describe('run', () => { await run(); - expect(getInputSpy).toHaveBeenCalled(); expect(getStateSpy).toHaveBeenCalledTimes(3); expect(infoSpy).not.toHaveBeenCalledWith( `Cache hit occurred on the primary key ${poetryLockHash}, not saving cache.` @@ -203,8 +200,8 @@ describe('run', () => { }); it('saves with -1 cacheId , should not fail workflow', async () => { - inputs['cache'] = 'poetry'; - inputs['python-version'] = '3.10.0'; + setInput('cache', 'poetry'); + setInput('python-version', '3.10.0'); getStateSpy.mockImplementation((name: string) => { if (name === State.STATE_CACHE_PRIMARY_KEY) { return poetryLockHash; @@ -221,7 +218,6 @@ describe('run', () => { await run(); - expect(getInputSpy).toHaveBeenCalled(); expect(getStateSpy).toHaveBeenCalledTimes(3); expect(infoSpy).not.toHaveBeenCalled(); expect(saveCacheSpy).toHaveBeenCalled(); @@ -232,8 +228,8 @@ describe('run', () => { }); it('saves with error from toolkit, should not fail the workflow', async () => { - inputs['cache'] = 'npm'; - inputs['python-version'] = '3.10.0'; + setInput('cache', 'npm'); + setInput('python-version', '3.10.0'); getStateSpy.mockImplementation((name: string) => { if (name === State.STATE_CACHE_PRIMARY_KEY) { return poetryLockHash; @@ -250,7 +246,6 @@ describe('run', () => { await run(); - expect(getInputSpy).toHaveBeenCalled(); expect(getStateSpy).toHaveBeenCalledTimes(3); expect(infoSpy).not.toHaveBeenCalledWith(); expect(saveCacheSpy).toHaveBeenCalled(); @@ -261,6 +256,5 @@ describe('run', () => { afterEach(() => { jest.resetAllMocks(); jest.clearAllMocks(); - inputs = {}; }); }); From 8f0a0b0eda11605aea32f2feb55056c981f66b7f Mon Sep 17 00:00:00 2001 From: Aarni Koskela Date: Fri, 10 Nov 2023 10:24:46 +0200 Subject: [PATCH 3/3] Add `cache-save: false` option --- __tests__/cache-save.test.ts | 16 ++++++++++++++-- action.yml | 3 +++ dist/cache-save/index.js | 20 +++++++++++++++++--- docs/advanced-usage.md | 36 ++++++++++++++++++++++++++++++++++++ src/cache-save.ts | 19 +++++++++++++++---- 5 files changed, 85 insertions(+), 9 deletions(-) diff --git a/__tests__/cache-save.test.ts b/__tests__/cache-save.test.ts index ae55ed151..bf77bcd13 100644 --- a/__tests__/cache-save.test.ts +++ b/__tests__/cache-save.test.ts @@ -34,8 +34,8 @@ describe('run', () => { beforeEach(() => { process.env['RUNNER_OS'] = process.env['RUNNER_OS'] ?? 'linux'; - for(const key in process.env) { - if(key.startsWith('INPUT_')) { + for (const key in process.env) { + if (key.startsWith('INPUT_')) { delete process.env[key]; } } @@ -251,6 +251,18 @@ describe('run', () => { expect(saveCacheSpy).toHaveBeenCalled(); expect(setFailedSpy).not.toHaveBeenCalled(); }); + + it('should not save the cache when requested not to', async () => { + setInput('cache', 'pip'); + setInput('cache-save', 'false'); + setInput('python-version', '3.10.0'); + await run(); + expect(infoSpy).toHaveBeenCalledWith( + 'Not saving cache since `cache-save` is false' + ); + expect(saveCacheSpy).not.toHaveBeenCalled(); + expect(setFailedSpy).not.toHaveBeenCalled(); + }); }); afterEach(() => { diff --git a/action.yml b/action.yml index 5c4669759..43ebb5879 100644 --- a/action.yml +++ b/action.yml @@ -20,6 +20,9 @@ inputs: default: ${{ github.server_url == 'https://github.com' && github.token || '' }} cache-dependency-path: description: "Used to specify the path to dependency files. Supports wildcards or a list of file names for caching multiple dependencies." + cache-save: + description: "Set this option if you want the action to save the cache after the run. Defaults to true. It can be useful to set this to false if you have e.g. optional dependencies that only some workflows require, and they should not be cached." + default: true update-environment: description: "Set this option if you want the action to update environment variables." default: true diff --git a/dist/cache-save/index.js b/dist/cache-save/index.js index c0424699d..cd2332bc8 100644 --- a/dist/cache-save/index.js +++ b/dist/cache-save/index.js @@ -80475,9 +80475,23 @@ function run(earlyExit) { try { const cache = core.getInput('cache'); if (cache) { - yield saveCache(cache); - if (earlyExit) { - process.exit(0); + let shouldSave = true; + try { + shouldSave = core.getBooleanInput('cache-save', { required: false }); + } + catch (e) { + // If we fail to parse the input, assume it's + // > "Input does not meet YAML 1.2 "core schema" specification." + // and assume it's the `true` default. + } + if (shouldSave) { + yield saveCache(cache); + if (earlyExit) { + process.exit(0); + } + } + else { + core.info('Not saving cache since `cache-save` is false'); } } } diff --git a/docs/advanced-usage.md b/docs/advanced-usage.md index a3ae55cc1..9b97cc417 100644 --- a/docs/advanced-usage.md +++ b/docs/advanced-usage.md @@ -381,6 +381,42 @@ steps: # Or pip install -e '.[test]' to install test dependencies ``` +### Skipping cache saving + +For some scenarios, it may be useful to only save a given subset of dependencies, +but restore more of them for other workflows. For instance, there may be a heavy +`extras` dependency that you do not need your entire test matrix to download, but +you want to download and test it separately without it being saved in the cache +archive for all runs. + +To achieve this, you can use `cache-save: false` on the run that uses the heavy +dependency. + + +```yaml +test: + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v4 + with: + python-version: '3.11' + cache: 'pip' + cache-dependency-path: pyproject.toml + - run: pip install -e . + +test-heavy-extra: + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v4 + with: + python-version: '3.11' + cache: 'pip' + cache-dependency-path: pyproject.toml + cache-save: false + - run: pip install -e '.[heavy-extra]' +``` + + # Outputs and environment variables ## Outputs diff --git a/src/cache-save.ts b/src/cache-save.ts index fbb481c17..e6e848060 100644 --- a/src/cache-save.ts +++ b/src/cache-save.ts @@ -11,10 +11,21 @@ export async function run(earlyExit?: boolean) { try { const cache = core.getInput('cache'); if (cache) { - await saveCache(cache); - - if (earlyExit) { - process.exit(0); + let shouldSave = true; + try { + shouldSave = core.getBooleanInput('cache-save', {required: false}); + } catch (e) { + // If we fail to parse the input, assume it's + // > "Input does not meet YAML 1.2 "core schema" specification." + // and assume it's the `true` default. + } + if (shouldSave) { + await saveCache(cache); + if (earlyExit) { + process.exit(0); + } + } else { + core.info('Not saving cache since `cache-save` is false'); } } } catch (error) {