From 4de30f744eb65b2f721d1a7993516d8c01c475d8 Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Mon, 25 Nov 2024 03:53:03 -0800 Subject: [PATCH] Add more tests for restoreCacheV2 --- .../cache/__tests__/restoreCacheV2.test.ts | 372 ++++++++++-------- packages/cache/src/cache.ts | 18 +- .../cache/src/internal/blob/download-cache.ts | 5 +- 3 files changed, 216 insertions(+), 179 deletions(-) diff --git a/packages/cache/__tests__/restoreCacheV2.test.ts b/packages/cache/__tests__/restoreCacheV2.test.ts index 87b2d1d0..707432ca 100644 --- a/packages/cache/__tests__/restoreCacheV2.test.ts +++ b/packages/cache/__tests__/restoreCacheV2.test.ts @@ -3,11 +3,12 @@ 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' +import * as downloadCacheModule from '../src/internal/blob/download-cache' +import {restoreCache} from '../src/cache' +import {CacheFilename, CompressionMethod} from '../src/internal/constants' +import {CacheServiceClientJSON} from '../src/generated/results/api/v1/cache.twirp' +import {BlobDownloadResponseParsed} from '@azure/storage-blob' +// import {executePromisesSequentially} from '@azure/ms-rest-js' jest.mock('../src/internal/cacheHttpClient') jest.mock('../src/internal/cacheUtils') @@ -15,222 +16,257 @@ 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(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) - }) + 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') + // 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` - ) + 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.` - ) + 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.` - ) + 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.` - ) + 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' + const paths = ['node_modules'] + const key = 'node-test' - jest - .spyOn(CacheServiceClientJSON.prototype, 'GetCacheEntryDownloadURL') - .mockReturnValue(Promise.resolve({ ok: false, signedDownloadUrl: '' })) + jest + .spyOn(CacheServiceClientJSON.prototype, 'GetCacheEntryDownloadURL') + .mockReturnValue(Promise.resolve({ok: false, signedDownloadUrl: ''})) - const cacheKey = await restoreCache(paths, key) + const cacheKey = await restoreCache(paths, key) - expect(cacheKey).toBe(undefined) + 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') + 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') - }) + 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' - ) + 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-' +test('restore with restore keys and no cache found', async () => { + const paths = ['node_modules'] + const key = 'node-test' + const restoreKey = 'node-' + const logWarningMock = jest.spyOn(core, 'warning') -// jest -// .spyOn(CacheServiceClientJSON.prototype, 'GetCacheEntryDownloadURL') -// .mockImplementation(() => { -// return Promise.resolve(null) -// }) -// jest.spyOn(cacheHttpClient, 'getCacheEntry').mockImplementation(async () => { -// return Promise.resolve(null) -// }) + jest + .spyOn(CacheServiceClientJSON.prototype, 'GetCacheEntryDownloadURL') + .mockReturnValue(Promise.resolve({ok: false, signedDownloadUrl: ''})) -// const cacheKey = await restoreCache(paths, key, [restoreKey]) + const cacheKey = await restoreCache(paths, key, [restoreKey]) -// expect(cacheKey).toBe(undefined) -// }) + expect(cacheKey).toBe(undefined) + expect(logWarningMock).toHaveBeenCalledWith( + `Cache not found for keys: ${[key, restoreKey].join(', ')}` + ) +}) -// test('restore with gzip compressed cache found', async () => { -// const paths = ['node_modules'] -// const key = 'node-test' +test('restore with gzip compressed cache found', async () => { + const paths = ['node_modules'] + const key = 'node-test' + const logInfoMock = jest.spyOn(core, 'info') + const compressionMethod = CompressionMethod.Gzip + const signedDownloadUrl = 'https://blob-storage.local?signed=true' + const cacheVersion = + 'd90f107aaeb22920dba0c637a23c37b5bc497b4dfa3b07fe3f79bf88a273c11b' -// 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 getCacheVersionMock = jest.spyOn(cacheUtils, 'getCacheVersion') + getCacheVersionMock.mockReturnValue(cacheVersion) -// const tempPath = '/foo/bar' + const compressionMethodMock = jest.spyOn(cacheUtils, 'getCompressionMethod') + compressionMethodMock.mockReturnValue(Promise.resolve(compressionMethod)) -// const createTempDirectoryMock = jest.spyOn(cacheUtils, 'createTempDirectory') -// createTempDirectoryMock.mockImplementation(async () => { -// return Promise.resolve(tempPath) -// }) + const getCacheDownloadURLMock = jest.spyOn( + CacheServiceClientJSON.prototype, + 'GetCacheEntryDownloadURL' + ) + getCacheDownloadURLMock.mockReturnValue( + Promise.resolve({ok: true, signedDownloadUrl}) + ) -// const archivePath = path.join(tempPath, CacheFilename.Gzip) -// const downloadCacheMock = jest.spyOn(cacheHttpClient, 'downloadCache') + const tempPath = '/foo/bar' -// const fileSize = 142 -// const getArchiveFileSizeInBytesMock = jest -// .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') -// .mockReturnValue(fileSize) + const createTempDirectoryMock = jest.spyOn(cacheUtils, 'createTempDirectory') + createTempDirectoryMock.mockImplementation(async () => { + return Promise.resolve(tempPath) + }) -// const extractTarMock = jest.spyOn(tar, 'extractTar') -// const unlinkFileMock = jest.spyOn(cacheUtils, 'unlinkFile') + const archivePath = path.join(tempPath, CacheFilename.Gzip) + const downloadCacheFileMock = jest.spyOn( + downloadCacheModule, + 'downloadCacheFile' + ) + downloadCacheFileMock.mockReturnValue( + Promise.resolve({} as BlobDownloadResponseParsed) + ) -// const compression = CompressionMethod.Gzip -// const getCompressionMock = jest -// .spyOn(cacheUtils, 'getCompressionMethod') -// .mockReturnValue(Promise.resolve(compression)) + const fileSize = 142 + const getArchiveFileSizeInBytesMock = jest + .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') + .mockReturnValue(fileSize) -// const cacheKey = await restoreCache(paths, key) + const extractTarMock = jest.spyOn(tar, 'extractTar') + const unlinkFileMock = jest.spyOn(cacheUtils, 'unlinkFile') -// 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) + const cacheKey = await restoreCache(paths, key) -// expect(extractTarMock).toHaveBeenCalledTimes(1) -// expect(extractTarMock).toHaveBeenCalledWith(archivePath, compression) + expect(cacheKey).toBe(key) + expect(getCacheVersionMock).toHaveBeenCalledWith( + paths, + compressionMethod, + false + ) + expect(getCacheDownloadURLMock).toHaveBeenCalledWith({ + key, + restoreKeys: [], + version: cacheVersion + }) + expect(createTempDirectoryMock).toHaveBeenCalledTimes(1) + expect(downloadCacheFileMock).toHaveBeenCalledWith( + signedDownloadUrl, + archivePath + ) + expect(getArchiveFileSizeInBytesMock).toHaveBeenCalledWith(archivePath) + expect(logInfoMock).toHaveBeenCalledWith(`Cache Size: ~0 MB (142 B)`) -// expect(unlinkFileMock).toHaveBeenCalledTimes(1) -// expect(unlinkFileMock).toHaveBeenCalledWith(archivePath) + expect(extractTarMock).toHaveBeenCalledTimes(1) + expect(extractTarMock).toHaveBeenCalledWith(archivePath, compressionMethod) -// expect(getCompressionMock).toHaveBeenCalledTimes(1) -// }) + expect(unlinkFileMock).toHaveBeenCalledTimes(1) + expect(unlinkFileMock).toHaveBeenCalledWith(archivePath) -// test('restore with zstd compressed cache found', async () => { -// const paths = ['node_modules'] -// const key = 'node-test' + expect(compressionMethodMock).toHaveBeenCalledTimes(1) +}) -// const infoMock = jest.spyOn(core, 'info') +test('restore with zstd compressed cache found', async () => { + const paths = ['node_modules'] + const key = 'node-test' + const logInfoMock = jest.spyOn(core, 'info') + const compressionMethod = CompressionMethod.Zstd + const signedDownloadUrl = 'https://blob-storage.local?signed=true' + const cacheVersion = + '8e2e96a184cb0cd6b48285b176c06a418f3d7fce14c29d9886fd1bb4f05c513d' -// 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 getCacheVersionMock = jest.spyOn(cacheUtils, 'getCacheVersion') + getCacheVersionMock.mockReturnValue(cacheVersion) -// const createTempDirectoryMock = jest.spyOn(cacheUtils, 'createTempDirectory') -// createTempDirectoryMock.mockImplementation(async () => { -// return Promise.resolve(tempPath) -// }) + const compressionMethodMock = jest.spyOn(cacheUtils, 'getCompressionMethod') + compressionMethodMock.mockReturnValue(Promise.resolve(compressionMethod)) -// const archivePath = path.join(tempPath, CacheFilename.Zstd) -// const downloadCacheMock = jest.spyOn(cacheHttpClient, 'downloadCache') + const getCacheDownloadURLMock = jest.spyOn( + CacheServiceClientJSON.prototype, + 'GetCacheEntryDownloadURL' + ) + getCacheDownloadURLMock.mockReturnValue( + Promise.resolve({ok: true, signedDownloadUrl}) + ) -// const fileSize = 62915000 -// const getArchiveFileSizeInBytesMock = jest -// .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') -// .mockReturnValue(fileSize) + const tempPath = '/foo/bar' -// const extractTarMock = jest.spyOn(tar, 'extractTar') -// const compression = CompressionMethod.Zstd -// const getCompressionMock = jest -// .spyOn(cacheUtils, 'getCompressionMethod') -// .mockReturnValue(Promise.resolve(compression)) + const createTempDirectoryMock = jest.spyOn(cacheUtils, 'createTempDirectory') + createTempDirectoryMock.mockImplementation(async () => { + return Promise.resolve(tempPath) + }) -// const cacheKey = await restoreCache(paths, key) + const archivePath = path.join(tempPath, CacheFilename.Zstd) + const downloadCacheFileMock = jest.spyOn( + downloadCacheModule, + 'downloadCacheFile' + ) + downloadCacheFileMock.mockReturnValue( + Promise.resolve({} as BlobDownloadResponseParsed) + ) -// 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)`) + const fileSize = 62915000 + const getArchiveFileSizeInBytesMock = jest + .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') + .mockReturnValue(fileSize) -// expect(extractTarMock).toHaveBeenCalledTimes(1) -// expect(extractTarMock).toHaveBeenCalledWith(archivePath, compression) -// expect(getCompressionMock).toHaveBeenCalledTimes(1) -// }) + const extractTarMock = jest.spyOn(tar, 'extractTar') + const unlinkFileMock = jest.spyOn(cacheUtils, 'unlinkFile') + + const cacheKey = await restoreCache(paths, key) + + expect(cacheKey).toBe(key) + expect(getCacheVersionMock).toHaveBeenCalledWith( + paths, + compressionMethod, + false + ) + expect(getCacheDownloadURLMock).toHaveBeenCalledWith({ + key, + restoreKeys: [], + version: cacheVersion + }) + expect(createTempDirectoryMock).toHaveBeenCalledTimes(1) + expect(downloadCacheFileMock).toHaveBeenCalledWith( + signedDownloadUrl, + archivePath + ) + expect(getArchiveFileSizeInBytesMock).toHaveBeenCalledWith(archivePath) + expect(logInfoMock).toHaveBeenCalledWith(`Cache Size: ~60 MB (62915000 B)`) + + expect(extractTarMock).toHaveBeenCalledTimes(1) + expect(extractTarMock).toHaveBeenCalledWith(archivePath, compressionMethod) + + expect(unlinkFileMock).toHaveBeenCalledTimes(1) + expect(unlinkFileMock).toHaveBeenCalledWith(archivePath) + + expect(compressionMethodMock).toHaveBeenCalledTimes(1) +}) // test('restore with cache found for restore key', async () => { // const paths = ['node_modules'] diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index f9863b14..07d6c7ce 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) @@ -405,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( diff --git a/packages/cache/src/internal/blob/download-cache.ts b/packages/cache/src/internal/blob/download-cache.ts index 807c73a4..e974cb2f 100644 --- a/packages/cache/src/internal/blob/download-cache.ts +++ b/packages/cache/src/internal/blob/download-cache.ts @@ -3,13 +3,14 @@ import * as core from '@actions/core' import { BlobClient, BlockBlobClient, - BlobDownloadOptions + BlobDownloadOptions, + BlobDownloadResponseParsed } from '@azure/storage-blob' export async function downloadCacheFile( signedUploadURL: string, archivePath: string -): Promise<{}> { +): Promise { const downloadOptions: BlobDownloadOptions = { maxRetryRequests: 5 }