From 32549e8197e1d742177b585f89c55a947773c92d Mon Sep 17 00:00:00 2001 From: Rob Herley Date: Fri, 1 Dec 2023 01:32:45 +0000 Subject: [PATCH] update download-artifact tests for public and internal impl --- .../__tests__/artifact-http-client.test.ts | 2 +- .../__tests__/{common.test.ts => common.ts} | 2 +- .../__tests__/download-artifact.test.ts | 260 +++++++++++++++--- .../path-and-artifact-name-validation.test.ts | 2 +- .../__tests__/upload-artifact.test.ts | 9 +- .../upload-zip-specification.test.ts | 2 +- 6 files changed, 224 insertions(+), 53 deletions(-) rename packages/artifact/__tests__/{common.test.ts => common.ts} (85%) diff --git a/packages/artifact/__tests__/artifact-http-client.test.ts b/packages/artifact/__tests__/artifact-http-client.test.ts index 31bd24c3..2dd6c5c4 100644 --- a/packages/artifact/__tests__/artifact-http-client.test.ts +++ b/packages/artifact/__tests__/artifact-http-client.test.ts @@ -3,7 +3,7 @@ import * as net from 'net' import {HttpClient} from '@actions/http-client' import * as config from '../src/internal/shared/config' import {internalArtifactTwirpClient} from '../src/internal/shared/artifact-twirp-client' -import {noopLogs} from './common.test' +import {noopLogs} from './common' jest.mock('@actions/http-client') diff --git a/packages/artifact/__tests__/common.test.ts b/packages/artifact/__tests__/common.ts similarity index 85% rename from packages/artifact/__tests__/common.test.ts rename to packages/artifact/__tests__/common.ts index 02955f10..4fae850c 100644 --- a/packages/artifact/__tests__/common.test.ts +++ b/packages/artifact/__tests__/common.ts @@ -2,7 +2,7 @@ import * as core from '@actions/core' // noopLogs mocks the console.log and core.* functions to prevent output in the console while testing export const noopLogs = (): void => { - // jest.spyOn(console, 'log').mockImplementation(() => {}) + jest.spyOn(console, 'log').mockImplementation(() => {}) jest.spyOn(core, 'debug').mockImplementation(() => {}) jest.spyOn(core, 'info').mockImplementation(() => {}) jest.spyOn(core, 'warning').mockImplementation(() => {}) diff --git a/packages/artifact/__tests__/download-artifact.test.ts b/packages/artifact/__tests__/download-artifact.test.ts index 8f80ff76..207a9d01 100644 --- a/packages/artifact/__tests__/download-artifact.test.ts +++ b/packages/artifact/__tests__/download-artifact.test.ts @@ -7,9 +7,15 @@ import {HttpClient} from '@actions/http-client' import type {RestEndpointMethods} from '@octokit/plugin-rest-endpoint-methods/dist-types/generated/method-types' import archiver from 'archiver' -import {downloadArtifactPublic} from '../src/internal/download/download-artifact' +import { + downloadArtifactInternal, + downloadArtifactPublic +} from '../src/internal/download/download-artifact' import {getUserAgentString} from '../src/internal/shared/user-agent' -import {noopLogs} from './common.test' +import {noopLogs} from './common' +import * as config from '../src/internal/shared/config' +import {ArtifactServiceClientJSON} from '../src/generated' +import * as util from '../src/internal/shared/util' type MockedDownloadArtifact = jest.MockedFunction< RestEndpointMethods['actions']['downloadArtifact'] @@ -32,10 +38,16 @@ const fixtures = { ] }, artifactID: 1234, + artifactName: 'my-artifact', + artifactSize: 123456, repositoryOwner: 'actions', repositoryName: 'toolkit', token: 'ghp_1234567890', - blobStorageUrl: 'https://blob-storage.local?signed=true' + blobStorageUrl: 'https://blob-storage.local?signed=true', + backendIds: { + workflowRunBackendId: 'c4d7c21f-ba3f-4ddc-a8c8-6f2f626f8422', + workflowJobRunBackendId: '760803a1-f890-4d25-9a6e-a3fc01a0c7cf' + } } jest.mock('@actions/github', () => ({ @@ -88,6 +100,24 @@ const cleanup = async (): Promise => { delete process.env['GITHUB_WORKSPACE'] } +const mockGetArtifactSuccess = jest.fn(() => { + const message = new http.IncomingMessage(new net.Socket()) + message.statusCode = 200 + message.push(fs.readFileSync(fixtures.exampleArtifact.path)) + return { + message + } +}) + +const mockGetArtifactFailure = jest.fn(() => { + const message = new http.IncomingMessage(new net.Socket()) + message.statusCode = 500 + message.push('Internal Server Error') + return { + message + } +}) + describe('download-artifact', () => { describe('public', () => { beforeEach(setup) @@ -105,18 +135,10 @@ describe('download-artifact', () => { data: Buffer.from('') }) - const getMock = jest.fn(() => { - const message = new http.IncomingMessage(new net.Socket()) - message.statusCode = 200 - message.push(fs.readFileSync(fixtures.exampleArtifact.path)) - return { - message - } - }) - const httpClientMock = (HttpClient as jest.Mock).mockImplementation( + const mockHttpClient = (HttpClient as jest.Mock).mockImplementation( () => { return { - get: getMock + get: mockGetArtifactSuccess } } ) @@ -137,11 +159,11 @@ describe('download-artifact', () => { redirect: 'manual' } }) - expect(httpClientMock).toHaveBeenCalledWith(getUserAgentString()) - expect(getMock).toHaveBeenCalledWith(fixtures.blobStorageUrl) - + expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString()) + expect(mockGetArtifactSuccess).toHaveBeenCalledWith( + fixtures.blobStorageUrl + ) expectExtractedArchive(fixtures.workspaceDir) - expect(response.success).toBe(true) expect(response.downloadPath).toBe(fixtures.workspaceDir) }) @@ -160,18 +182,10 @@ describe('download-artifact', () => { data: Buffer.from('') }) - const getMock = jest.fn(() => { - const message = new http.IncomingMessage(new net.Socket()) - message.statusCode = 200 - message.push(fs.readFileSync(fixtures.exampleArtifact.path)) - return { - message - } - }) - const httpClientMock = (HttpClient as jest.Mock).mockImplementation( + const mockHttpClient = (HttpClient as jest.Mock).mockImplementation( () => { return { - get: getMock + get: mockGetArtifactSuccess } } ) @@ -195,11 +209,11 @@ describe('download-artifact', () => { redirect: 'manual' } }) - expect(httpClientMock).toHaveBeenCalledWith(getUserAgentString()) - expect(getMock).toHaveBeenCalledWith(fixtures.blobStorageUrl) - + expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString()) + expect(mockGetArtifactSuccess).toHaveBeenCalledWith( + fixtures.blobStorageUrl + ) expectExtractedArchive(customPath) - expect(response.success).toBe(true) expect(response.downloadPath).toBe(customPath) }) @@ -246,18 +260,10 @@ describe('download-artifact', () => { data: Buffer.from('') }) - const getMock = jest.fn(() => { - const message = new http.IncomingMessage(new net.Socket()) - message.statusCode = 500 - message.push('Internal Server Error') - return { - message - } - }) - const httpClientMock = (HttpClient as jest.Mock).mockImplementation( + const mockHttpClient = (HttpClient as jest.Mock).mockImplementation( () => { return { - get: getMock + get: mockGetArtifactFailure } } ) @@ -280,8 +286,176 @@ describe('download-artifact', () => { redirect: 'manual' } }) - expect(httpClientMock).toHaveBeenCalledWith(getUserAgentString()) - expect(getMock).toHaveBeenCalledWith(fixtures.blobStorageUrl) + expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString()) + expect(mockGetArtifactFailure).toHaveBeenCalledWith( + fixtures.blobStorageUrl + ) + }) + }) + + describe('internal', () => { + beforeEach(async () => { + await setup() + + jest.spyOn(config, 'getRuntimeToken').mockReturnValue('test-token') + + jest + .spyOn(util, 'getBackendIdsFromToken') + .mockReturnValue(fixtures.backendIds) + + jest + .spyOn(config, 'getResultsServiceUrl') + .mockReturnValue('https://results.local') + }) + afterEach(async () => { + await cleanup() + }) + + it('should successfully download an artifact to $GITHUB_WORKSPACE', async () => { + const mockListArtifacts = jest + .spyOn(ArtifactServiceClientJSON.prototype, 'ListArtifacts') + .mockResolvedValue({ + artifacts: [ + { + ...fixtures.backendIds, + databaseId: fixtures.artifactID.toString(), + name: fixtures.artifactName, + size: fixtures.artifactSize.toString() + } + ] + }) + + const mockGetSignedArtifactURL = jest + .spyOn(ArtifactServiceClientJSON.prototype, 'GetSignedArtifactURL') + .mockReturnValue( + Promise.resolve({ + signedUrl: fixtures.blobStorageUrl + }) + ) + + const mockHttpClient = (HttpClient as jest.Mock).mockImplementation( + () => { + return { + get: mockGetArtifactSuccess + } + } + ) + + const response = await downloadArtifactInternal(fixtures.artifactID) + + expectExtractedArchive(fixtures.workspaceDir) + expect(response.success).toBe(true) + expect(response.downloadPath).toBe(fixtures.workspaceDir) + expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString()) + expect(mockListArtifacts).toHaveBeenCalledWith({ + ...fixtures.backendIds + }) + expect(mockGetSignedArtifactURL).toHaveBeenCalledWith({ + ...fixtures.backendIds, + name: fixtures.artifactName + }) + }) + + it('should successfully download an artifact to user defined path', async () => { + const customPath = path.join(testDir, 'custom') + + const mockListArtifacts = jest + .spyOn(ArtifactServiceClientJSON.prototype, 'ListArtifacts') + .mockResolvedValue({ + artifacts: [ + { + ...fixtures.backendIds, + databaseId: fixtures.artifactID.toString(), + name: fixtures.artifactName, + size: fixtures.artifactSize.toString() + } + ] + }) + + const mockGetSignedArtifactURL = jest + .spyOn(ArtifactServiceClientJSON.prototype, 'GetSignedArtifactURL') + .mockReturnValue( + Promise.resolve({ + signedUrl: fixtures.blobStorageUrl + }) + ) + + const mockHttpClient = (HttpClient as jest.Mock).mockImplementation( + () => { + return { + get: mockGetArtifactSuccess + } + } + ) + + const response = await downloadArtifactInternal(fixtures.artifactID, { + path: customPath + }) + + expectExtractedArchive(customPath) + expect(response.success).toBe(true) + expect(response.downloadPath).toBe(customPath) + expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString()) + expect(mockListArtifacts).toHaveBeenCalledWith({ + ...fixtures.backendIds + }) + expect(mockGetSignedArtifactURL).toHaveBeenCalledWith({ + ...fixtures.backendIds, + name: fixtures.artifactName + }) + }) + + it('should fail if download artifact API does not respond with location', async () => { + jest + .spyOn(ArtifactServiceClientJSON.prototype, 'ListArtifacts') + .mockRejectedValue(new Error('boom')) + + await expect( + downloadArtifactInternal(fixtures.artifactID) + ).rejects.toBeInstanceOf(Error) + }) + + it('should fail if blob storage response is non-200', async () => { + const mockListArtifacts = jest + .spyOn(ArtifactServiceClientJSON.prototype, 'ListArtifacts') + .mockResolvedValue({ + artifacts: [ + { + ...fixtures.backendIds, + databaseId: fixtures.artifactID.toString(), + name: fixtures.artifactName, + size: fixtures.artifactSize.toString() + } + ] + }) + + const mockGetSignedArtifactURL = jest + .spyOn(ArtifactServiceClientJSON.prototype, 'GetSignedArtifactURL') + .mockReturnValue( + Promise.resolve({ + signedUrl: fixtures.blobStorageUrl + }) + ) + + const mockHttpClient = (HttpClient as jest.Mock).mockImplementation( + () => { + return { + get: mockGetArtifactFailure + } + } + ) + + await expect( + downloadArtifactInternal(fixtures.artifactID) + ).rejects.toBeInstanceOf(Error) + expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString()) + expect(mockListArtifacts).toHaveBeenCalledWith({ + ...fixtures.backendIds + }) + expect(mockGetSignedArtifactURL).toHaveBeenCalledWith({ + ...fixtures.backendIds, + name: fixtures.artifactName + }) }) }) }) diff --git a/packages/artifact/__tests__/path-and-artifact-name-validation.test.ts b/packages/artifact/__tests__/path-and-artifact-name-validation.test.ts index cd3fcd52..3ecdde45 100644 --- a/packages/artifact/__tests__/path-and-artifact-name-validation.test.ts +++ b/packages/artifact/__tests__/path-and-artifact-name-validation.test.ts @@ -3,7 +3,7 @@ import { validateFilePath } from '../src/internal/upload/path-and-artifact-name-validation' -import {noopLogs} from './common.test' +import {noopLogs} from './common' describe('Path and artifact name validation', () => { beforeAll(() => { diff --git a/packages/artifact/__tests__/upload-artifact.test.ts b/packages/artifact/__tests__/upload-artifact.test.ts index fc7b79f5..439a96a3 100644 --- a/packages/artifact/__tests__/upload-artifact.test.ts +++ b/packages/artifact/__tests__/upload-artifact.test.ts @@ -6,7 +6,7 @@ import * as config from '../src/internal/shared/config' import {Timestamp, ArtifactServiceClientJSON} from '../src/generated' import * as blobUpload from '../src/internal/upload/blob-upload' import {uploadArtifact} from '../src/internal/upload/upload-artifact' -import {noopLogs} from './common.test' +import {noopLogs} from './common' describe('upload-artifact', () => { beforeEach(() => { @@ -127,7 +127,7 @@ describe('upload-artifact', () => { expect(uploadResp).resolves.toEqual({success: false}) }) - it('should return false if no backend IDs are found', () => { + it('should reject if no backend IDs are found', () => { jest .spyOn(uploadZipSpecification, 'validateRootDirectory') .mockReturnValue() @@ -151,9 +151,6 @@ describe('upload-artifact', () => { jest .spyOn(zip, 'createZipUploadStream') .mockReturnValue(Promise.resolve(new zip.ZipUploadStream(1))) - jest - .spyOn(util, 'getBackendIdsFromToken') - .mockReturnValue({workflowRunBackendId: '', workflowJobRunBackendId: ''}) const uploadResp = uploadArtifact( 'test-artifact', @@ -165,7 +162,7 @@ describe('upload-artifact', () => { '/home/user/files/plz-upload' ) - expect(uploadResp).resolves.toEqual({success: false}) + expect(uploadResp).rejects.toThrow() }) it('should return false if the creation request fails', () => { diff --git a/packages/artifact/__tests__/upload-zip-specification.test.ts b/packages/artifact/__tests__/upload-zip-specification.test.ts index cf606d31..0b59bff7 100644 --- a/packages/artifact/__tests__/upload-zip-specification.test.ts +++ b/packages/artifact/__tests__/upload-zip-specification.test.ts @@ -5,7 +5,7 @@ import { getUploadZipSpecification, validateRootDirectory } from '../src/internal/upload/upload-zip-specification' -import {noopLogs} from './common.test' +import {noopLogs} from './common' const root = path.join(__dirname, '_temp', 'upload-specification') const goodItem1Path = path.join(