From 39d19810a88675c2360d4949b352d94cc453827b Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Fri, 22 Nov 2024 09:01:59 -0800 Subject: [PATCH] Add restore tests --- .../cache/__tests__/restoreCacheV2.test.ts | 327 ++++++++++++++++++ packages/cache/src/cache.ts | 36 +- 2 files changed, 352 insertions(+), 11 deletions(-) create mode 100644 packages/cache/__tests__/restoreCacheV2.test.ts diff --git a/packages/cache/__tests__/restoreCacheV2.test.ts b/packages/cache/__tests__/restoreCacheV2.test.ts new file mode 100644 index 0000000000..87b2d1d0fc --- /dev/null +++ b/packages/cache/__tests__/restoreCacheV2.test.ts @@ -0,0 +1,327 @@ +import * as core from '@actions/core' +import * as path from 'path' +import * as tar from '../src/internal/tar' +import * as config from '../src/internal/config' +import * as cacheUtils from '../src/internal/cacheUtils' +import * as cacheHttpClient from '../src/internal/cacheHttpClient' +import { restoreCache } from '../src/cache' +import { CacheFilename, CompressionMethod } from '../src/internal/constants' +import { ArtifactCacheEntry } from '../src/internal/contracts' +import { CacheServiceClientJSON } from '../src/generated/results/api/v1/cache.twirp' + +jest.mock('../src/internal/cacheHttpClient') +jest.mock('../src/internal/cacheUtils') +jest.mock('../src/internal/config') +jest.mock('../src/internal/tar') + +beforeAll(() => { + jest.spyOn(console, 'log').mockImplementation(() => { }) + jest.spyOn(core, 'debug').mockImplementation(() => { }) + jest.spyOn(core, 'info').mockImplementation(() => { }) + jest.spyOn(core, 'warning').mockImplementation(() => { }) + jest.spyOn(core, 'error').mockImplementation(() => { }) + + jest.spyOn(cacheUtils, 'getCacheFileName').mockImplementation(cm => { + const actualUtils = jest.requireActual('../src/internal/cacheUtils') + return actualUtils.getCacheFileName(cm) + }) + + // Ensure that we're using v2 for these tests + jest.spyOn(config, 'getCacheServiceVersion').mockReturnValue('v2') +}) + +test('restore with no path should fail', async () => { + const paths: string[] = [] + const key = 'node-test' + await expect(restoreCache(paths, key)).rejects.toThrowError( + `Path Validation Error: At least one directory or file path is required` + ) +}) + +test('restore with too many keys should fail', async () => { + const paths = ['node_modules'] + const key = 'node-test' + const restoreKeys = [...Array(20).keys()].map(x => x.toString()) + await expect(restoreCache(paths, key, restoreKeys)).rejects.toThrowError( + `Key Validation Error: Keys are limited to a maximum of 10.` + ) +}) + +test('restore with large key should fail', async () => { + const paths = ['node_modules'] + const key = 'foo'.repeat(512) // Over the 512 character limit + await expect(restoreCache(paths, key)).rejects.toThrowError( + `Key Validation Error: ${key} cannot be larger than 512 characters.` + ) +}) + +test('restore with invalid key should fail', async () => { + const paths = ['node_modules'] + const key = 'comma,comma' + await expect(restoreCache(paths, key)).rejects.toThrowError( + `Key Validation Error: ${key} cannot contain commas.` + ) +}) + +test('restore with no cache found', async () => { + const paths = ['node_modules'] + const key = 'node-test' + + jest + .spyOn(CacheServiceClientJSON.prototype, 'GetCacheEntryDownloadURL') + .mockReturnValue(Promise.resolve({ ok: false, signedDownloadUrl: '' })) + + const cacheKey = await restoreCache(paths, key) + + expect(cacheKey).toBe(undefined) +}) + +test('restore with server error should fail', async () => { + const paths = ['node_modules'] + const key = 'node-test' + const logWarningMock = jest.spyOn(core, 'warning') + + jest + .spyOn(CacheServiceClientJSON.prototype, 'GetCacheEntryDownloadURL') + .mockImplementation(() => { + throw new Error('HTTP Error Occurred') + }) + + const cacheKey = await restoreCache(paths, key) + expect(cacheKey).toBe(undefined) + expect(logWarningMock).toHaveBeenCalledTimes(1) + expect(logWarningMock).toHaveBeenCalledWith( + 'Failed to restore: HTTP Error Occurred' + ) +}) + +// test('restore with restore keys and no cache found', async () => { +// const paths = ['node_modules'] +// const key = 'node-test' +// const restoreKey = 'node-' + +// jest +// .spyOn(CacheServiceClientJSON.prototype, 'GetCacheEntryDownloadURL') +// .mockImplementation(() => { +// return Promise.resolve(null) +// }) +// jest.spyOn(cacheHttpClient, 'getCacheEntry').mockImplementation(async () => { +// return Promise.resolve(null) +// }) + +// const cacheKey = await restoreCache(paths, key, [restoreKey]) + +// expect(cacheKey).toBe(undefined) +// }) + +// test('restore with gzip compressed cache found', async () => { +// const paths = ['node_modules'] +// const key = 'node-test' + +// const cacheEntry: ArtifactCacheEntry = { +// cacheKey: key, +// scope: 'refs/heads/main', +// archiveLocation: 'www.actionscache.test/download' +// } +// const getCacheMock = jest.spyOn(cacheHttpClient, 'getCacheEntry') +// getCacheMock.mockImplementation(async () => { +// return Promise.resolve(cacheEntry) +// }) + +// const tempPath = '/foo/bar' + +// const createTempDirectoryMock = jest.spyOn(cacheUtils, 'createTempDirectory') +// createTempDirectoryMock.mockImplementation(async () => { +// return Promise.resolve(tempPath) +// }) + +// const archivePath = path.join(tempPath, CacheFilename.Gzip) +// const downloadCacheMock = jest.spyOn(cacheHttpClient, 'downloadCache') + +// const fileSize = 142 +// const getArchiveFileSizeInBytesMock = jest +// .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') +// .mockReturnValue(fileSize) + +// const extractTarMock = jest.spyOn(tar, 'extractTar') +// const unlinkFileMock = jest.spyOn(cacheUtils, 'unlinkFile') + +// const compression = CompressionMethod.Gzip +// const getCompressionMock = jest +// .spyOn(cacheUtils, 'getCompressionMethod') +// .mockReturnValue(Promise.resolve(compression)) + +// const cacheKey = await restoreCache(paths, key) + +// expect(cacheKey).toBe(key) +// expect(getCacheMock).toHaveBeenCalledWith([key], paths, { +// compressionMethod: compression, +// enableCrossOsArchive: false +// }) +// expect(createTempDirectoryMock).toHaveBeenCalledTimes(1) +// expect(downloadCacheMock).toHaveBeenCalledWith( +// cacheEntry.archiveLocation, +// archivePath, +// undefined +// ) +// expect(getArchiveFileSizeInBytesMock).toHaveBeenCalledWith(archivePath) + +// expect(extractTarMock).toHaveBeenCalledTimes(1) +// expect(extractTarMock).toHaveBeenCalledWith(archivePath, compression) + +// expect(unlinkFileMock).toHaveBeenCalledTimes(1) +// expect(unlinkFileMock).toHaveBeenCalledWith(archivePath) + +// expect(getCompressionMock).toHaveBeenCalledTimes(1) +// }) + +// test('restore with zstd compressed cache found', async () => { +// const paths = ['node_modules'] +// const key = 'node-test' + +// const infoMock = jest.spyOn(core, 'info') + +// const cacheEntry: ArtifactCacheEntry = { +// cacheKey: key, +// scope: 'refs/heads/main', +// archiveLocation: 'www.actionscache.test/download' +// } +// const getCacheMock = jest.spyOn(cacheHttpClient, 'getCacheEntry') +// getCacheMock.mockImplementation(async () => { +// return Promise.resolve(cacheEntry) +// }) +// const tempPath = '/foo/bar' + +// const createTempDirectoryMock = jest.spyOn(cacheUtils, 'createTempDirectory') +// createTempDirectoryMock.mockImplementation(async () => { +// return Promise.resolve(tempPath) +// }) + +// const archivePath = path.join(tempPath, CacheFilename.Zstd) +// const downloadCacheMock = jest.spyOn(cacheHttpClient, 'downloadCache') + +// const fileSize = 62915000 +// const getArchiveFileSizeInBytesMock = jest +// .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') +// .mockReturnValue(fileSize) + +// const extractTarMock = jest.spyOn(tar, 'extractTar') +// const compression = CompressionMethod.Zstd +// const getCompressionMock = jest +// .spyOn(cacheUtils, 'getCompressionMethod') +// .mockReturnValue(Promise.resolve(compression)) + +// const cacheKey = await restoreCache(paths, key) + +// expect(cacheKey).toBe(key) +// expect(getCacheMock).toHaveBeenCalledWith([key], paths, { +// compressionMethod: compression, +// enableCrossOsArchive: false +// }) +// expect(createTempDirectoryMock).toHaveBeenCalledTimes(1) +// expect(downloadCacheMock).toHaveBeenCalledWith( +// cacheEntry.archiveLocation, +// archivePath, +// undefined +// ) +// expect(getArchiveFileSizeInBytesMock).toHaveBeenCalledWith(archivePath) +// expect(infoMock).toHaveBeenCalledWith(`Cache Size: ~60 MB (62915000 B)`) + +// expect(extractTarMock).toHaveBeenCalledTimes(1) +// expect(extractTarMock).toHaveBeenCalledWith(archivePath, compression) +// expect(getCompressionMock).toHaveBeenCalledTimes(1) +// }) + +// test('restore with cache found for restore key', async () => { +// const paths = ['node_modules'] +// const key = 'node-test' +// const restoreKey = 'node-' + +// const infoMock = jest.spyOn(core, 'info') + +// const cacheEntry: ArtifactCacheEntry = { +// cacheKey: restoreKey, +// scope: 'refs/heads/main', +// archiveLocation: 'www.actionscache.test/download' +// } +// const getCacheMock = jest.spyOn(cacheHttpClient, 'getCacheEntry') +// getCacheMock.mockImplementation(async () => { +// return Promise.resolve(cacheEntry) +// }) +// const tempPath = '/foo/bar' + +// const createTempDirectoryMock = jest.spyOn(cacheUtils, 'createTempDirectory') +// createTempDirectoryMock.mockImplementation(async () => { +// return Promise.resolve(tempPath) +// }) + +// const archivePath = path.join(tempPath, CacheFilename.Zstd) +// const downloadCacheMock = jest.spyOn(cacheHttpClient, 'downloadCache') + +// const fileSize = 142 +// const getArchiveFileSizeInBytesMock = jest +// .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') +// .mockReturnValue(fileSize) + +// const extractTarMock = jest.spyOn(tar, 'extractTar') +// const compression = CompressionMethod.Zstd +// const getCompressionMock = jest +// .spyOn(cacheUtils, 'getCompressionMethod') +// .mockReturnValue(Promise.resolve(compression)) + +// const cacheKey = await restoreCache(paths, key, [restoreKey]) + +// expect(cacheKey).toBe(restoreKey) +// expect(getCacheMock).toHaveBeenCalledWith([key, restoreKey], paths, { +// compressionMethod: compression, +// enableCrossOsArchive: false +// }) +// expect(createTempDirectoryMock).toHaveBeenCalledTimes(1) +// expect(downloadCacheMock).toHaveBeenCalledWith( +// cacheEntry.archiveLocation, +// archivePath, +// undefined +// ) +// expect(getArchiveFileSizeInBytesMock).toHaveBeenCalledWith(archivePath) +// expect(infoMock).toHaveBeenCalledWith(`Cache Size: ~0 MB (142 B)`) + +// expect(extractTarMock).toHaveBeenCalledTimes(1) +// expect(extractTarMock).toHaveBeenCalledWith(archivePath, compression) +// expect(getCompressionMock).toHaveBeenCalledTimes(1) +// }) + +// test('restore with dry run', async () => { +// const paths = ['node_modules'] +// const key = 'node-test' +// const options = { lookupOnly: true } + +// const cacheEntry: ArtifactCacheEntry = { +// cacheKey: key, +// scope: 'refs/heads/main', +// archiveLocation: 'www.actionscache.test/download' +// } +// const getCacheMock = jest.spyOn(cacheHttpClient, 'getCacheEntry') +// getCacheMock.mockImplementation(async () => { +// return Promise.resolve(cacheEntry) +// }) + +// const createTempDirectoryMock = jest.spyOn(cacheUtils, 'createTempDirectory') +// const downloadCacheMock = jest.spyOn(cacheHttpClient, 'downloadCache') + +// const compression = CompressionMethod.Gzip +// const getCompressionMock = jest +// .spyOn(cacheUtils, 'getCompressionMethod') +// .mockReturnValue(Promise.resolve(compression)) + +// const cacheKey = await restoreCache(paths, key, undefined, options) + +// expect(cacheKey).toBe(key) +// expect(getCompressionMock).toHaveBeenCalledTimes(1) +// expect(getCacheMock).toHaveBeenCalledWith([key], paths, { +// compressionMethod: compression, +// enableCrossOsArchive: false +// }) +// // creating a tempDir and downloading the cache are skipped +// expect(createTempDirectoryMock).toHaveBeenCalledTimes(0) +// expect(downloadCacheMock).toHaveBeenCalledTimes(0) +// }) diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index 1450c8ace9..f9863b14b5 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -3,18 +3,18 @@ import * as path from 'path' import * as utils from './internal/cacheUtils' import * as cacheHttpClient from './internal/cacheHttpClient' import * as cacheTwirpClient from './internal/shared/cacheTwirpClient' -import {getCacheServiceVersion, isGhes} from './internal/config' -import {DownloadOptions, UploadOptions} from './options' -import {createTar, extractTar, listTar} from './internal/tar' +import { getCacheServiceVersion, isGhes } from './internal/config' +import { DownloadOptions, UploadOptions } from './options' +import { createTar, extractTar, listTar } from './internal/tar' import { CreateCacheEntryRequest, FinalizeCacheEntryUploadRequest, FinalizeCacheEntryUploadResponse, GetCacheEntryDownloadURLRequest } from './generated/results/api/v1/cache' -import {CacheFileSizeLimit} from './internal/constants' -import {uploadCacheFile} from './internal/blob/upload-cache' -import {downloadCacheFile} from './internal/blob/download-cache' +import { CacheFileSizeLimit } from './internal/constants' +import { uploadCacheFile } from './internal/blob/upload-cache' +import { downloadCacheFile } from './internal/blob/download-cache' export class ValidationError extends Error { constructor(message: string) { super(message) @@ -287,7 +287,13 @@ async function restoreCacheV2( return request.key } catch (error) { - throw new Error(`Failed to restore: ${error.message}`) + const typedError = error as Error + if (typedError.name === ValidationError.name) { + throw error + } else { + // Supress all non-validation cache related errors because caching should be optional + core.warning(`Failed to restore: ${(error as Error).message}`) + } } finally { try { if (archivePath) { @@ -297,6 +303,8 @@ async function restoreCacheV2( core.debug(`Failed to delete archive: ${error}`) } } + + return undefined } /** @@ -397,9 +405,9 @@ async function saveCacheV1( } else if (reserveCacheResponse?.statusCode === 400) { throw new Error( reserveCacheResponse?.error?.message ?? - `Cache size of ~${Math.round( - archiveFileSize / (1024 * 1024) - )} MB (${archiveFileSize} B) is over the data cap limit, not saving cache.` + `Cache size of ~${Math.round( + archiveFileSize / (1024 * 1024) + )} MB (${archiveFileSize} B) is over the data cap limit, not saving cache.` ) } else { throw new ReserveCacheError( @@ -525,7 +533,13 @@ async function saveCacheV2( cacheId = parseInt(finalizeResponse.entryId) } catch (error) { const typedError = error as Error - core.warning(`Failed to save: ${typedError.message}`) + if (typedError.name === ValidationError.name) { + throw error + } else if (typedError.name === ReserveCacheError.name) { + core.info(`Failed to save: ${typedError.message}`) + } else { + core.warning(`Failed to save: ${typedError.message}`) + } } finally { // Try to delete the archive to save space try {