Add end to end test for cache using bsd on windows

and address review comments

.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

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).toHaveBeenCalledTimes(2)
-    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}"`,
         '-xf',
         TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'),
         '-P',
         '-C',
-        workspace?.replace(/\\/g, '/'),
+        workspace?.replace(/\\/g, '/')
-        '"' // end cmd /c
+      ].join(' '),
-      ].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).toHaveBeenCalledTimes(2)
-    expect(execMock).toHaveBeenCalledWith(
+
+    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, '/'),
+        TarFilename.replace(/\\/g, '/')
-        '"' // end cmd /c
       ].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',
+        '-P'
-        '"' // end cmd /c
+      ].join(' '),
-      ].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}
   )
 })

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<string> {
+): Promise<string[]> {
-  let args: string
+  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<void> {
+  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<void> {
-  const args = await getArgs(compressionMethod, 'list', archivePath)
+  const commands = await getCommands(compressionMethod, 'list', archivePath)
-  try {
+  await execCommands(commands)
-    await exec(args)
-  } catch (error) {
-    throw new Error(`Tar failed with error: ${error?.message}`)
-  }
 }
 
 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)
+  const commands = await getCommands(compressionMethod, 'extract', archivePath)
-  try {
+  await execCommands(commands)
-    await exec(args)
-  } catch (error) {
-    throw new Error(`Tar failed with error: ${error?.message}`)
-  }
 }
 
 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')
+  const commands = await getCommands(compressionMethod, 'create')
-  try {
+  await execCommands(commands, archiveFolder)
-    await exec(args, undefined, {cwd: archiveFolder})
-  } catch (error) {
-    throw new Error(`Tar failed with error: ${error?.message}`)
-  }
 }