refactor(cache): consolidate tar fallback handling and tests

Share the Windows BSD tar/zstd predicate and prepare archive and workspace
arguments once per command. Parameterize duplicate BSD-tar read tests and
use the real temporary-directory utilities in the concurrency fixture.

Retain the existing concurrency, path-handling, and failure coverage.

__tests__/tar.test.ts

@@ -84,57 +84,56 @@ test('zstd extract tar', async () => {
   )
 })
 
-test('zstd extract tar with windows BSDtar', async () => {
-  if (IS_WINDOWS) {
+test.each([
+  {operation: 'extract', read: tar.extractTar, flag: '-xf'},
+  {operation: 'list', read: tar.listTar, flag: '-tf'}
+])(
+  'zstd $operation tar with windows BSDtar',
+  async ({operation, read, flag}) => {
+    if (!IS_WINDOWS) {
+      return
+    }
     const mkdirMock = jest.spyOn(io, 'mkdirP')
     const execMock = jest.spyOn(exec, 'exec')
     const tempDirectory = getTempDir()
     jest.spyOn(utils, 'createTempDirectory').mockResolvedValue(tempDirectory)
-    jest
-      .spyOn(utils, 'getGnuTarPathOnWindows')
-      .mockReturnValue(Promise.resolve(''))
+    jest.spyOn(utils, 'getGnuTarPathOnWindows').mockResolvedValue('')
 
     const archivePath = `${process.env['windir']}\\fakepath\\cache.tar`
     const workspace = process.env['GITHUB_WORKSPACE']
-    const tarPath = SystemTarPathOnWindows
+    const options = {
+      cwd: tempDirectory,
+      env: expect.objectContaining(defaultEnv)
+    }
+    const tarArgs = [`"${SystemTarPathOnWindows}"`, flag, TarFilename, '-P']
+    if (operation === 'extract') {
+      tarArgs.push('-C', `"${workspace?.replace(/\\/g, '/')}"`)
+    }
 
-    await tar.extractTar(archivePath, CompressionMethod.Zstd)
+    await read(archivePath, CompressionMethod.Zstd)
 
+    if (operation === 'extract') {
       expect(mkdirMock).toHaveBeenCalledWith(workspace)
+    }
     expect(execMock).toHaveBeenCalledTimes(2)
-
     expect(execMock).toHaveBeenNthCalledWith(
       1,
       [
         'zstd -d --long=30 --force -o',
-        TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'),
-        `"${archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/')}"`
+        TarFilename,
+        `"${archivePath.replace(/\\/g, '/')}"`
       ].join(' '),
       undefined,
-      {
-        cwd: tempDirectory,
-        env: expect.objectContaining(defaultEnv)
-      }
+      options
     )
-
     expect(execMock).toHaveBeenNthCalledWith(
       2,
-      [
-        `"${tarPath}"`,
-        '-xf',
-        TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'),
-        '-P',
-        '-C',
-        `"${workspace?.replace(/\\/g, '/')}"`
-      ].join(' '),
+      tarArgs.join(' '),
       undefined,
-      {
-        cwd: tempDirectory,
-        env: expect.objectContaining(defaultEnv)
-      }
+      options
     )
   }
-})
+)
 
 test('gzip extract tar', async () => {
   const mkdirMock = jest.spyOn(io, 'mkdirP')
@@ -380,52 +379,6 @@ test('zstd list tar', async () => {
   )
 })
 
-test('zstd list tar with windows BSDtar', async () => {
-  if (IS_WINDOWS) {
-    const execMock = jest.spyOn(exec, 'exec')
-    const tempDirectory = getTempDir()
-    jest.spyOn(utils, 'createTempDirectory').mockResolvedValue(tempDirectory)
-    jest
-      .spyOn(utils, 'getGnuTarPathOnWindows')
-      .mockReturnValue(Promise.resolve(''))
-    const archivePath = `${process.env['windir']}\\fakepath\\cache.tar`
-
-    await tar.listTar(archivePath, CompressionMethod.Zstd)
-
-    const tarPath = SystemTarPathOnWindows
-    expect(execMock).toHaveBeenCalledTimes(2)
-
-    expect(execMock).toHaveBeenNthCalledWith(
-      1,
-      [
-        'zstd -d --long=30 --force -o',
-        TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'),
-        `"${archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/')}"`
-      ].join(' '),
-      undefined,
-      {
-        cwd: tempDirectory,
-        env: expect.objectContaining(defaultEnv)
-      }
-    )
-
-    expect(execMock).toHaveBeenNthCalledWith(
-      2,
-      [
-        `"${tarPath}"`,
-        '-tf',
-        TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'),
-        '-P'
-      ].join(' '),
-      undefined,
-      {
-        cwd: tempDirectory,
-        env: expect.objectContaining(defaultEnv)
-      }
-    )
-  }
-})
-
 test('zstdWithoutLong list tar', async () => {
   const execMock = jest.spyOn(exec, 'exec')
 

__tests__/tarConcurrency.test.ts

@@ -1,5 +1,4 @@
 import * as exec from '@actions/exec'
-import * as io from '@actions/io'
 import * as fs from 'fs'
 import * as path from 'path'
 import * as tar from '../src/internal/tar'
@@ -7,10 +6,10 @@ import * as utils from '../src/internal/cacheUtils'
 import {CompressionMethod} from '../src/internal/constants'
 
 jest.mock('@actions/exec')
-jest.mock('@actions/io')
 
 const describeWindows = process.platform === 'win32' ? describe : describe.skip
-type Operation = 'extract' | 'list'
+const readers = {extract: tar.extractTar, list: tar.listTar}
+type Operation = keyof typeof readers
 
 interface Fixture {
   file: string
@@ -25,7 +24,7 @@ describeWindows.each([
     {file: 'first.txt', content: 'first archive'},
     {file: 'second.txt', content: 'second archive'}
   ]
-  const originalWorkspace = process.env['GITHUB_WORKSPACE']
+  const originalEnv = {...process.env}
   let root: string
   let workspace: string
   let archives: string[]
@@ -51,17 +50,12 @@ describeWindows.each([
     listedFiles = []
     failStage = undefined
     afterDecompression = async () => {}
-    process.env['GITHUB_WORKSPACE'] = workspace
+    process.env = {
+      ...originalEnv,
+      GITHUB_WORKSPACE: workspace,
+      RUNNER_TEMP: root
+    }
     jest.spyOn(utils, 'getGnuTarPathOnWindows').mockResolvedValue('')
-    jest.spyOn(utils, 'createTempDirectory').mockImplementation(async () => {
-      return fs.promises.mkdtemp(path.join(root, 'scratch-'))
-    })
-    jest.spyOn(io, 'mkdirP').mockImplementation(async directory => {
-      await fs.promises.mkdir(path.resolve(directory), {recursive: true})
-    })
-    jest.spyOn(io, 'rmRF').mockImplementation(async directory => {
-      await fs.promises.rm(directory, {recursive: true, force: true})
-    })
 
     // Model the filesystem effects of zstd and tar without requiring either
     // program to be installed. The fixture content stands in for archive data.
@@ -82,7 +76,8 @@ describeWindows.each([
             throw new Error('simulated decompression failure')
           }
           await afterDecompression()
-        } else {
+          return 0
+        }
         if (failStage === 'tar') {
           throw new Error('simulated tar failure')
         }
@@ -102,32 +97,16 @@ describeWindows.each([
         } else {
           listedFiles.push(fixture.file)
         }
-        }
         return 0
       })
   })
 
   afterEach(async () => {
     jest.restoreAllMocks()
-    if (originalWorkspace === undefined) {
-      delete process.env['GITHUB_WORKSPACE']
-    } else {
-      process.env['GITHUB_WORKSPACE'] = originalWorkspace
-    }
+    process.env = {...originalEnv}
     await fs.promises.rm(root, {recursive: true, force: true})
   })
 
-  async function readArchive(
-    operation: Operation,
-    archive: string
-  ): Promise<void> {
-    if (operation === 'extract') {
-      await tar.extractTar(archive, compression)
-    } else {
-      await tar.listTar(archive, compression)
-    }
-  }
-
   function expectCleanScratch(): void {
     expect(fs.readdirSync(root).sort()).toEqual([
       'archives',
@@ -166,7 +145,7 @@ describeWindows.each([
     const operations = [first, second]
     await Promise.all(
       operations.map(async (operation, index) => {
-        await readArchive(operation, archives[index])
+        await readers[operation](archives[index], compression)
       })
     )
 
@@ -209,9 +188,9 @@ describeWindows.each([
     async (operation, stage) => {
       failStage = stage
 
-      await expect(readArchive(operation, archives[0])).rejects.toThrow(
-        'simulated'
-      )
+      await expect(
+        readers[operation](archives[0], compression)
+      ).rejects.toThrow('simulated')
 
       expectCleanScratch()
     }

src/internal/tar.ts

@@ -19,6 +19,17 @@ interface TarCommands {
   requiresTempDirectory: boolean
 }
 
+function isBsdTarZstd(
+  tarPath: ArchiveTool,
+  compressionMethod: CompressionMethod
+): boolean {
+  return (
+    IS_WINDOWS &&
+    tarPath.type === ArchiveToolType.BSD &&
+    compressionMethod !== CompressionMethod.Gzip
+  )
+}
+
 // Returns tar path and type: BSD or GNU
 async function getTarPath(): Promise<ArchiveTool> {
   switch (process.platform) {
@@ -63,14 +74,16 @@ async function getTarArgs(
   archivePath = ''
 ): Promise<string[]> {
   const args = [`"${tarPath.path}"`]
-  const cacheFileName = utils.getCacheFileName(compressionMethod)
-  const tarFile = 'cache.tar'
-  const workingDirectory = getWorkingDirectory()
-  // Speficic args for BSD tar on windows for workaround
-  const BSD_TAR_ZSTD =
-    tarPath.type === ArchiveToolType.BSD &&
-    compressionMethod !== CompressionMethod.Gzip &&
-    IS_WINDOWS
+  const BSD_TAR_ZSTD = isBsdTarZstd(tarPath, compressionMethod)
+  const tarFile = BSD_TAR_ZSTD
+    ? TarFilename
+    : (type === 'create'
+        ? utils.getCacheFileName(compressionMethod)
+        : archivePath
+      ).replace(new RegExp(`\\${path.sep}`, 'g'), '/')
+  const workingDirectory = BSD_TAR_ZSTD
+    ? quoteAbsolutePath(getWorkingDirectory())
+    : getWorkingDirectory().replace(new RegExp(`\\${path.sep}`, 'g'), '/')
 
   // Method specific args
   switch (type) {
@@ -78,43 +91,21 @@ async function getTarArgs(
       args.push(
         '--posix',
         '-cf',
-        BSD_TAR_ZSTD
-          ? tarFile
-          : cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'),
+        tarFile,
         '--exclude',
-        BSD_TAR_ZSTD
-          ? tarFile
-          : cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'),
+        tarFile,
         '-P',
         '-C',
-        BSD_TAR_ZSTD
-          ? quoteAbsolutePath(workingDirectory)
-          : workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/'),
+        workingDirectory,
         '--files-from',
         ManifestFilename
       )
       break
     case 'extract':
-      args.push(
-        '-xf',
-        BSD_TAR_ZSTD
-          ? tarFile
-          : archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'),
-        '-P',
-        '-C',
-        BSD_TAR_ZSTD
-          ? quoteAbsolutePath(workingDirectory)
-          : workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/')
-      )
+      args.push('-xf', tarFile, '-P', '-C', workingDirectory)
       break
     case 'list':
-      args.push(
-        '-tf',
-        BSD_TAR_ZSTD
-          ? tarFile
-          : archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'),
-        '-P'
-      )
+      args.push('-tf', tarFile, '-P')
       break
   }
 
@@ -152,10 +143,7 @@ async function getCommands(
     type !== 'create'
       ? await getDecompressionProgram(tarPath, compressionMethod, archivePath)
       : await getCompressionProgram(tarPath, compressionMethod)
-  const BSD_TAR_ZSTD =
-    tarPath.type === ArchiveToolType.BSD &&
-    compressionMethod !== CompressionMethod.Gzip &&
-    IS_WINDOWS
+  const BSD_TAR_ZSTD = isBsdTarZstd(tarPath, compressionMethod)
 
   if (BSD_TAR_ZSTD && type !== 'create') {
     args = [[...compressionArgs].join(' '), [...tarArgs].join(' ')]
@@ -187,10 +175,7 @@ async function getDecompressionProgram(
   // unzstd is equivalent to 'zstd -d'
   // --long=#: Enables long distance matching with # bits. Maximum is 30 (1GB) on 32-bit OS and 31 (2GB) on 64-bit.
   // Using 30 here because we also support 32-bit self-hosted runners.
-  const BSD_TAR_ZSTD =
-    tarPath.type === ArchiveToolType.BSD &&
-    compressionMethod !== CompressionMethod.Gzip &&
-    IS_WINDOWS
+  const BSD_TAR_ZSTD = isBsdTarZstd(tarPath, compressionMethod)
   switch (compressionMethod) {
     case CompressionMethod.Zstd:
       return BSD_TAR_ZSTD
@@ -223,10 +208,7 @@ async function getCompressionProgram(
   compressionMethod: CompressionMethod
 ): Promise<string[]> {
   const cacheFileName = utils.getCacheFileName(compressionMethod)
-  const BSD_TAR_ZSTD =
-    tarPath.type === ArchiveToolType.BSD &&
-    compressionMethod !== CompressionMethod.Gzip &&
-    IS_WINDOWS
+  const BSD_TAR_ZSTD = isBsdTarZstd(tarPath, compressionMethod)
   switch (compressionMethod) {
     case CompressionMethod.Zstd:
       return BSD_TAR_ZSTD