From d175a181a08ea8fe5433c7b0b147f5e611cd5a03 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Mon, 12 Dec 2022 06:53:11 +0000 Subject: [PATCH] Add end to end test for cache using bsd on windows and address review comments --- .github/workflows/cache-windows-test.yml | 90 ++++++++++++++++++++++ packages/cache/__tests__/tar.test.ts | 95 +++++++++++++++++------- packages/cache/src/internal/tar.ts | 56 +++++++------- 3 files changed, 182 insertions(+), 59 deletions(-) create mode 100644 .github/workflows/cache-windows-test.yml diff --git a/.github/workflows/cache-windows-test.yml b/.github/workflows/cache-windows-test.yml new file mode 100644 index 00000000..3868f296 --- /dev/null +++ b/.github/workflows/cache-windows-test.yml @@ -0,0 +1,90 @@ +name: cache-windows-bsd-unit-tests +on: + push: + branches: + - main + paths-ignore: + - '**.md' + pull_request: + paths-ignore: + - '**.md' + +jobs: + build: + name: Build + + runs-on: windows-latest + + steps: + - name: Checkout + uses: actions/checkout@v2 + + - shell: bash + run: | + rm "C:\Program Files\Git\usr\bin\tar.exe" + + - name: Set Node.js 12.x + uses: actions/setup-node@v1 + with: + node-version: 12.x + + # In order to save & restore cache from a shell script, certain env variables need to be set that are only available in the + # node context. This runs a local action that gets and sets the necessary env variables that are needed + - name: Set env variables + uses: ./packages/cache/__tests__/__fixtures__/ + + # Need root node_modules because certain npm packages like jest are configured for the entire repository and it won't be possible + # without these to just compile the cache package + - name: Install root npm packages + run: npm ci + + - name: Compile cache package + run: | + npm ci + npm run tsc + working-directory: packages/cache + + - name: Generate files in working directory + shell: bash + run: packages/cache/__tests__/create-cache-files.sh ${{ runner.os }} test-cache + + - name: Generate files outside working directory + shell: bash + run: packages/cache/__tests__/create-cache-files.sh ${{ runner.os }} ~/test-cache + + # We're using node -e to call the functions directly available in the @actions/cache package + - name: Save cache using saveCache() + run: | + node -e "Promise.resolve(require('./packages/cache/lib/cache').saveCache(['test-cache','~/test-cache'],'test-${{ runner.os }}-${{ github.run_id }}'))" + + - name: Delete cache folders before restoring + shell: bash + run: | + rm -rf test-cache + rm -rf ~/test-cache + + - name: Restore cache using restoreCache() with http-client + run: | + node -e "Promise.resolve(require('./packages/cache/lib/cache').restoreCache(['test-cache','~/test-cache'],'test-${{ runner.os }}-${{ github.run_id }}',[],{useAzureSdk: false}))" + + - name: Verify cache restored with http-client + shell: bash + run: | + packages/cache/__tests__/verify-cache-files.sh ${{ runner.os }} test-cache + packages/cache/__tests__/verify-cache-files.sh ${{ runner.os }} ~/test-cache + + - name: Delete cache folders before restoring + shell: bash + run: | + rm -rf test-cache + rm -rf ~/test-cache + + - name: Restore cache using restoreCache() with Azure SDK + run: | + node -e "Promise.resolve(require('./packages/cache/lib/cache').restoreCache(['test-cache','~/test-cache'],'test-${{ runner.os }}-${{ github.run_id }}'))" + + - name: Verify cache restored with Azure SDK + shell: bash + run: | + packages/cache/__tests__/verify-cache-files.sh ${{ runner.os }} test-cache + packages/cache/__tests__/verify-cache-files.sh ${{ runner.os }} ~/test-cache diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index efd38d85..3e837960 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -73,7 +73,9 @@ test('zstd extract tar', async () => { '--use-compress-program', IS_WINDOWS ? '"zstd -d --long=30"' : 'unzstd --long=30' ]) - .join(' ') + .join(' '), + undefined, + {cwd: undefined} ) }) @@ -92,22 +94,31 @@ test('zstd extract tar with windows BSDtar', async () => { await tar.extractTar(archivePath, CompressionMethod.Zstd) expect(mkdirMock).toHaveBeenCalledWith(workspace) - expect(execMock).toHaveBeenCalledTimes(1) - expect(execMock).toHaveBeenCalledWith( + expect(execMock).toHaveBeenCalledTimes(2) + + expect(execMock).toHaveBeenNthCalledWith( + 1, [ - 'cmd /c "', 'zstd -d --long=30 -o', TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - '&&', + archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/') + ].join(' '), + undefined, + {cwd: undefined} + ) + + expect(execMock).toHaveBeenNthCalledWith( + 2, + [ `"${tarPath}"`, '-xf', TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P', '-C', - workspace?.replace(/\\/g, '/'), - '"' // end cmd /c - ].join(' ') + workspace?.replace(/\\/g, '/') + ].join(' '), + undefined, + {cwd: undefined} ) } }) @@ -137,7 +148,9 @@ test('gzip extract tar', async () => { .concat(IS_WINDOWS ? ['--force-local'] : []) .concat(IS_MAC ? ['--delay-directory-restore'] : []) .concat(['-z']) - .join(' ') + .join(' '), + undefined, + {cwd: undefined} ) }) @@ -164,7 +177,9 @@ test('gzip extract GNU tar on windows with GNUtar in path', async () => { workspace?.replace(/\\/g, '/'), '--force-local', '-z' - ].join(' ') + ].join(' '), + undefined, + {cwd: undefined} ) } }) @@ -232,10 +247,11 @@ test('zstd create tar with windows BSDtar', async () => { const tarPath = SystemTarPathOnWindows - expect(execMock).toHaveBeenCalledTimes(1) - expect(execMock).toHaveBeenCalledWith( + expect(execMock).toHaveBeenCalledTimes(2) + + expect(execMock).toHaveBeenNthCalledWith( + 1, [ - 'cmd /c "', `"${tarPath}"`, '--posix', '-cf', @@ -246,12 +262,20 @@ test('zstd create tar with windows BSDtar', async () => { '-C', workspace?.replace(/\\/g, '/'), '--files-from', - ManifestFilename, - '&&', + ManifestFilename + ].join(' '), + undefined, // args + { + cwd: archiveFolder + } + ) + + expect(execMock).toHaveBeenNthCalledWith( + 2, + [ 'zstd -T0 --long=30 -o', CacheFilename.Zstd.replace(/\\/g, '/'), - TarFilename.replace(/\\/g, '/'), - '"' // end cmd /c + TarFilename.replace(/\\/g, '/') ].join(' '), undefined, // args { @@ -324,7 +348,9 @@ test('zstd list tar', async () => { '--use-compress-program', IS_WINDOWS ? '"zstd -d --long=30"' : 'unzstd --long=30' ]) - .join(' ') + .join(' '), + undefined, + {cwd: undefined} ) }) @@ -340,19 +366,28 @@ test('zstd list tar with windows BSDtar', async () => { const tarPath = SystemTarPathOnWindows expect(execMock).toHaveBeenCalledTimes(1) - expect(execMock).toHaveBeenCalledWith( + + expect(execMock).toHaveBeenNthCalledWith( + 1, [ - 'cmd /c "', 'zstd -d --long=30 -o', TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - '&&', + archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/') + ].join(' '), + undefined, + {cwd: undefined} + ) + + expect(execMock).toHaveBeenNthCalledWith( + 2, + [ `"${tarPath}"`, '-tf', TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - '-P', - '"' // end cmd /c - ].join(' ') + '-P' + ].join(' '), + undefined, + {cwd: undefined} ) } }) @@ -378,7 +413,9 @@ test('zstdWithoutLong list tar', async () => { .concat(IS_WINDOWS ? ['--force-local'] : []) .concat(IS_MAC ? ['--delay-directory-restore'] : []) .concat(['--use-compress-program', IS_WINDOWS ? '"zstd -d"' : 'unzstd']) - .join(' ') + .join(' '), + undefined, + {cwd: undefined} ) }) @@ -402,6 +439,8 @@ test('gzip list tar', async () => { .concat(IS_WINDOWS ? ['--force-local'] : []) .concat(IS_MAC ? ['--delay-directory-restore'] : []) .concat(['-z']) - .join(' ') + .join(' '), + undefined, + {cwd: undefined} ) }) diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index f2f75918..97d36d3a 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -122,12 +122,12 @@ async function getTarArgs( return args } -async function getArgs( +async function getCommands( compressionMethod: CompressionMethod, type: string, archivePath = '' -): Promise { - let args: string +): Promise { + let args const tarPath = await getTarPath() const tarArgs = await getTarArgs( @@ -146,16 +146,16 @@ async function getArgs( IS_WINDOWS if (BSD_TAR_ZSTD && type !== 'create') { - args = [...compressionArgs, ...tarArgs].join(' ') + args = [...compressionArgs, ...tarArgs] } else { - args = [...tarArgs, ...compressionArgs].join(' ') + args = [...tarArgs, ...compressionArgs] } if (BSD_TAR_ZSTD) { - args = ['cmd /c "', args, '"'].join(' ') + return args } - return args + return [args.join(' ')] } function getWorkingDirectory(): string { @@ -182,8 +182,7 @@ async function getDecompressionProgram( ? [ 'zstd -d --long=30 -o', TarFilename, - archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - '&&' + archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/') ] : [ '--use-compress-program', @@ -194,8 +193,7 @@ async function getDecompressionProgram( ? [ 'zstd -d -o', TarFilename, - archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - '&&' + archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/') ] : ['--use-compress-program', IS_WINDOWS ? '"zstd -d"' : 'unzstd'] default: @@ -221,7 +219,6 @@ async function getCompressionProgram( case CompressionMethod.Zstd: return BSD_TAR_ZSTD ? [ - '&&', 'zstd -T0 --long=30 -o', cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), TarFilename @@ -233,7 +230,6 @@ async function getCompressionProgram( case CompressionMethod.ZstdWithoutLong: return BSD_TAR_ZSTD ? [ - '&&', 'zstd -T0 -o', cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), TarFilename @@ -244,16 +240,22 @@ async function getCompressionProgram( } } +async function execCommands(commands: string[], cwd?: string): Promise { + for (const command of commands) { + try { + await exec(command, undefined, {cwd}) + } catch (error) { + throw new Error(`${command[0]} failed with error: ${error?.message}`) + } + } +} + export async function listTar( archivePath: string, compressionMethod: CompressionMethod ): Promise { - const args = await getArgs(compressionMethod, 'list', archivePath) - try { - await exec(args) - } catch (error) { - throw new Error(`Tar failed with error: ${error?.message}`) - } + const commands = await getCommands(compressionMethod, 'list', archivePath) + await execCommands(commands) } export async function extractTar( @@ -263,12 +265,8 @@ export async function extractTar( // Create directory to extract tar into const workingDirectory = getWorkingDirectory() await io.mkdirP(workingDirectory) - const args = await getArgs(compressionMethod, 'extract', archivePath) - try { - await exec(args) - } catch (error) { - throw new Error(`Tar failed with error: ${error?.message}`) - } + const commands = await getCommands(compressionMethod, 'extract', archivePath) + await execCommands(commands) } export async function createTar( @@ -281,10 +279,6 @@ export async function createTar( path.join(archiveFolder, ManifestFilename), sourceDirectories.join('\n') ) - const args = await getArgs(compressionMethod, 'create') - try { - await exec(args, undefined, {cwd: archiveFolder}) - } catch (error) { - throw new Error(`Tar failed with error: ${error?.message}`) - } + const commands = await getCommands(compressionMethod, 'create') + await execCommands(commands, archiveFolder) }