From 01bf918aa54471eb872114e2283b70517994706e Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Thu, 24 Oct 2024 06:09:23 -0700 Subject: [PATCH] Refactoring & cleanup --- .../cache/__tests__/cacheHttpClient.test.ts | 9 +- .../cache/__tests__/restoreCachev2.test.ts | 346 ------------------ packages/cache/__tests__/saveCache.test.ts | 75 ---- packages/cache/src/cache.ts | 2 +- 4 files changed, 6 insertions(+), 426 deletions(-) delete mode 100644 packages/cache/__tests__/restoreCachev2.test.ts diff --git a/packages/cache/__tests__/cacheHttpClient.test.ts b/packages/cache/__tests__/cacheHttpClient.test.ts index 21c5ae86..b8176ba6 100644 --- a/packages/cache/__tests__/cacheHttpClient.test.ts +++ b/packages/cache/__tests__/cacheHttpClient.test.ts @@ -1,7 +1,8 @@ -import {downloadCache, getCacheVersion} from '../src/internal/cacheHttpClient' -import {CompressionMethod} from '../src/internal/constants' +import { getCacheVersion } from '../src/internal/cacheUtils' +import { downloadCache } from '../src/internal/cacheHttpClient' +import { CompressionMethod } from '../src/internal/constants' import * as downloadUtils from '../src/internal/downloadUtils' -import {DownloadOptions, getDownloadOptions} from '../src/options' +import { DownloadOptions, getDownloadOptions } from '../src/options' jest.mock('../src/internal/downloadUtils') @@ -128,7 +129,7 @@ test('downloadCache passes options to download methods', async () => { const archiveLocation = 'http://foo.blob.core.windows.net/bar/baz' const archivePath = '/foo/bar' - const options: DownloadOptions = {downloadConcurrency: 4} + const options: DownloadOptions = { downloadConcurrency: 4 } await downloadCache(archiveLocation, archivePath, options) diff --git a/packages/cache/__tests__/restoreCachev2.test.ts b/packages/cache/__tests__/restoreCachev2.test.ts deleted file mode 100644 index 73f42bfa..00000000 --- a/packages/cache/__tests__/restoreCachev2.test.ts +++ /dev/null @@ -1,346 +0,0 @@ -import * as core from '@actions/core' -import * as path from 'path' -import { restoreCache } from '../src/cache' -import { CacheServiceClientJSON } from '../src/generated/results/api/v1/cache.twirp' -import * as cacheUtils from '../src/internal/cacheUtils' -import * as config from '../src/internal/config' -import { CacheFilename, CompressionMethod } from '../src/internal/constants' -import * as util from '@actions/artifact/lib/internal/shared/util' -import { ArtifactCacheEntry } from '../src/internal/contracts' -import * as tar from '../src/internal/tar' - -jest.mock('../src/internal/cacheTwirpClient') -jest.mock('../src/internal/cacheUtils') -jest.mock('../src/internal/tar') - -const fixtures = { - testRuntimeToken: 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwic2NwIjoiQWN0aW9ucy5FeGFtcGxlIEFjdGlvbnMuQW5vdGhlckV4YW1wbGU6dGVzdCBBY3Rpb25zLlJlc3VsdHM6Y2U3ZjU0YzctNjFjNy00YWFlLTg4N2YtMzBkYTQ3NWY1ZjFhOmNhMzk1MDg1LTA0MGEtNTI2Yi0yY2U4LWJkYzg1ZjY5Mjc3NCIsImlhdCI6MTUxNjIzOTAyMn0.XYnI_wHPBlUi1mqYveJnnkJhp4dlFjqxzRmISPsqfw8', - backendIds: { - workflowRunBackendId: 'c4d7c21f-ba3f-4ddc-a8c8-6f2f626f8422', - workflowJobRunBackendId: '760803a1-f890-4d25-9a6e-a3fc01a0c7cf' - }, - cacheServiceURL: 'http://results.local', -} - -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) - }) - - jest.spyOn(config, 'getCacheServiceVersion').mockImplementation(() => { - return "v2" - }) - - jest.spyOn(config, 'getRuntimeToken').mockImplementation(() => { - return fixtures.testRuntimeToken - }) - - jest.spyOn(util, 'getBackendIdsFromToken').mockImplementation(() => { - return fixtures.backendIds - }) - - jest.spyOn(config, 'getCacheServiceURL').mockReturnValue( - fixtures.cacheServiceURL - ) -}) - -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(cacheHttpClient, 'getCacheEntry').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(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) -}) - **/ \ No newline at end of file diff --git a/packages/cache/__tests__/saveCache.test.ts b/packages/cache/__tests__/saveCache.test.ts index 7597ba8d..4d0027be 100644 --- a/packages/cache/__tests__/saveCache.test.ts +++ b/packages/cache/__tests__/saveCache.test.ts @@ -2,14 +2,10 @@ import * as core from '@actions/core' import * as path from 'path' import {saveCache} from '../src/cache' import * as cacheHttpClient from '../src/internal/cacheHttpClient' -import * as cacheTwirpClient from '../src/internal/cacheTwirpClient' -import {GetCacheBlobUploadURLResponse} from '../src/generated/results/api/v1/blobcache' -import {BlobCacheServiceClientJSON} from '../src/generated/results/api/v1/blobcache.twirp' import * as cacheUtils from '../src/internal/cacheUtils' import {CacheFilename, CompressionMethod} from '../src/internal/constants' import * as tar from '../src/internal/tar' import {TypedResponse} from '@actions/http-client/lib/interfaces' -import * as uploadCache from '../src/internal/v2/upload-cache' import { ReserveCacheResponse, ITypedResponseWithError @@ -331,74 +327,3 @@ test('save with non existing path should not save cache', async () => { `Path Validation Error: Path(s) specified in the action for caching do(es) not exist, hence no cache is being saved.` ) }) - -test('throwaway test', async () => { - const filePath = 'node_modules' - const primaryKey = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' - const cachePaths = [path.resolve(filePath)] - - const cacheSignedURL = 'https://container.blob.core.windows.net/cache/${primaryKey}?sig=1234' - const getCacheBlobUploadURL: GetCacheBlobUploadURLResponse = { - urls: [ - { - key: primaryKey, - url: cacheSignedURL, - }, - ] - } - - const cacheId = 4 - const reserveCacheMock = jest - .spyOn(cacheHttpClient, 'reserveCache') - .mockImplementation(async () => { - const response: TypedResponse = { - statusCode: 500, - result: {cacheId}, - headers: {} - } - return response - }) - - const getCacheBlobUploadURLMock = jest - .spyOn(BlobCacheServiceClientJSON.prototype, 'GetCacheBlobUploadURL') - .mockResolvedValue(getCacheBlobUploadURL) - - const uploadCacheMock = jest - .spyOn(uploadCache, 'UploadCacheFile') - .mockImplementation(async () => { - return { - status: 200 - } - }) - - const createTarMock = jest.spyOn(tar, 'createTar') - - const saveCacheMock = jest.spyOn(cacheHttpClient, 'saveCache') - const compression = CompressionMethod.Zstd - const getCompressionMock = jest - .spyOn(cacheUtils, 'getCompressionMethod') - .mockReturnValue(Promise.resolve(compression)) - - await uploadCache.UploadCacheFile(getCacheBlobUploadURL, cachePaths[0]) - await saveCache([filePath], primaryKey) - - expect(reserveCacheMock).toHaveBeenCalledTimes(1) - expect(reserveCacheMock).toHaveBeenCalledWith(primaryKey, [filePath], { - cacheSize: undefined, - compressionMethod: compression, - enableCrossOsArchive: false - }) - expect (getCacheBlobUploadURLMock).toHaveBeenCalledTimes(1) - const archiveFolder = '/foo/bar' - const archiveFile = path.join(archiveFolder, CacheFilename.Zstd) - expect(createTarMock).toHaveBeenCalledTimes(1) - expect(createTarMock).toHaveBeenCalledWith( - archiveFolder, - cachePaths, - compression - ) - expect(uploadCacheMock).toHaveBeenCalledTimes(2) - expect(saveCacheMock).toHaveBeenCalledTimes(1) - expect(saveCacheMock).toHaveBeenCalledWith(cacheId, archiveFile, undefined) - expect(getCompressionMock).toHaveBeenCalledTimes(1) -}) \ No newline at end of file diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index 7e4200f8..2659b848 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -64,7 +64,7 @@ function checkKey(key: string): void { */ export function isFeatureAvailable(): boolean { - return !!config.getCacheServiceVersion + return !!process.env['ACTIONS_CACHE_URL'] } /**