From dd137a579c6ffeea43f52a79b76619a6ed910167 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Thu, 17 Sep 2026 15:47:36 +0200 Subject: [PATCH] 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 | 103 +++++++++---------------------- __tests__/tarConcurrency.test.ts | 85 ++++++++++--------------- src/internal/tar.ts | 76 +++++++++-------------- 3 files changed, 89 insertions(+), 175 deletions(-) diff --git a/__tests__/tar.test.ts b/__tests__/tar.test.ts index 2788b34..f85a51d 100644 --- a/__tests__/tar.test.ts +++ b/__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) - expect(mkdirMock).toHaveBeenCalledWith(workspace) + 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') diff --git a/__tests__/tarConcurrency.test.ts b/__tests__/tarConcurrency.test.ts index 1e061e7..0d84d89 100644 --- a/__tests__/tarConcurrency.test.ts +++ b/__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,26 +76,26 @@ describeWindows.each([ throw new Error('simulated decompression failure') } await afterDecompression() - } else { - if (failStage === 'tar') { - throw new Error('simulated tar failure') - } - const extracting = argv.includes('-xf') - const source = path.resolve( - cwd, - argv[argv.indexOf(extracting ? '-xf' : '-tf') + 1] + return 0 + } + if (failStage === 'tar') { + throw new Error('simulated tar failure') + } + const extracting = argv.includes('-xf') + const source = path.resolve( + cwd, + argv[argv.indexOf(extracting ? '-xf' : '-tf') + 1] + ) + const fixture: Fixture = JSON.parse(fs.readFileSync(source, 'utf8')) + if (extracting) { + const destination = path.resolve(cwd, argv[argv.indexOf('-C') + 1]) + await fs.promises.mkdir(destination, {recursive: true}) + fs.writeFileSync( + path.join(destination, fixture.file), + fixture.content ) - const fixture: Fixture = JSON.parse(fs.readFileSync(source, 'utf8')) - if (extracting) { - const destination = path.resolve(cwd, argv[argv.indexOf('-C') + 1]) - await fs.promises.mkdir(destination, {recursive: true}) - fs.writeFileSync( - path.join(destination, fixture.file), - fixture.content - ) - } else { - listedFiles.push(fixture.file) - } + } else { + listedFiles.push(fixture.file) } return 0 }) @@ -109,25 +103,10 @@ describeWindows.each([ 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 { - 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() } diff --git a/src/internal/tar.ts b/src/internal/tar.ts index 30bc044..1e80329 100644 --- a/src/internal/tar.ts +++ b/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 { switch (process.platform) { @@ -63,14 +74,16 @@ async function getTarArgs( archivePath = '' ): Promise { 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 { 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