some test updates

packages/artifact/__tests__/artifact-http-client.test.ts

@@ -2,18 +2,14 @@ import * as http from 'http'
 import * as net from 'net'
 import {HttpClient} from '@actions/http-client'
 import * as config from '../src/internal/shared/config'
-import {createArtifactTwirpClient} from '../src/internal/shared/artifact-twirp-client'
-import * as core from '@actions/core'
+import {internalArtifactTwirpClient} from '../src/internal/shared/artifact-twirp-client'
+import {noopLogs} from './common.test'
 
 jest.mock('@actions/http-client')
 
 describe('artifact-http-client', () => {
   beforeAll(() => {
-    // mock all output so that there is less noise when running tests
-    jest.spyOn(console, 'log').mockImplementation(() => {})
-    jest.spyOn(core, 'debug').mockImplementation(() => {})
-    jest.spyOn(core, 'info').mockImplementation(() => {})
-    jest.spyOn(core, 'warning').mockImplementation(() => {})
+    noopLogs()
     jest
       .spyOn(config, 'getResultsServiceUrl')
       .mockReturnValue('http://localhost:8080')
@@ -25,7 +21,7 @@ describe('artifact-http-client', () => {
   })
 
   it('should successfully create a client', () => {
-    const client = createArtifactTwirpClient('upload')
+    const client = internalArtifactTwirpClient()
     expect(client).toBeDefined()
   })
 
@@ -50,7 +46,7 @@ describe('artifact-http-client', () => {
       }
     })
 
-    const client = createArtifactTwirpClient('upload')
+    const client = internalArtifactTwirpClient()
     const artifact = await client.CreateArtifact({
       workflowRunBackendId: '1234',
       workflowJobRunBackendId: '5678',
@@ -98,12 +94,11 @@ describe('artifact-http-client', () => {
       }
     })
 
-    const client = createArtifactTwirpClient(
-      'upload',
-      5, // retry 5 times
-      1, // wait 1 ms
-      1.5 // backoff factor
-    )
+    const client = internalArtifactTwirpClient({
+      maxAttempts: 5,
+      retryIntervalMs: 1,
+      retryMultiplier: 1.5
+    })
     const artifact = await client.CreateArtifact({
       workflowRunBackendId: '1234',
       workflowJobRunBackendId: '5678',
@@ -138,12 +133,11 @@ describe('artifact-http-client', () => {
         post: mockPost
       }
     })
-    const client = createArtifactTwirpClient(
-      'upload',
-      5, // retry 5 times
-      1, // wait 1 ms
-      1.5 // backoff factor
-    )
+    const client = internalArtifactTwirpClient({
+      maxAttempts: 5,
+      retryIntervalMs: 1,
+      retryMultiplier: 1.5
+    })
     await expect(async () => {
       await client.CreateArtifact({
         workflowRunBackendId: '1234',
@@ -178,12 +172,11 @@ describe('artifact-http-client', () => {
         post: mockPost
       }
     })
-    const client = createArtifactTwirpClient(
-      'upload',
-      5, // retry 5 times
-      1, // wait 1 ms
-      1.5 // backoff factor
-    )
+    const client = internalArtifactTwirpClient({
+      maxAttempts: 5,
+      retryIntervalMs: 1,
+      retryMultiplier: 1.5
+    })
     await expect(async () => {
       await client.CreateArtifact({
         workflowRunBackendId: '1234',

packages/artifact/__tests__/common.test.ts

@@ -0,0 +1,9 @@
+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(core, 'debug').mockImplementation(() => {})
+  jest.spyOn(core, 'info').mockImplementation(() => {})
+  jest.spyOn(core, 'warning').mockImplementation(() => {})
+}

packages/artifact/__tests__/download-artifact.test.ts

@@ -2,14 +2,14 @@ import fs from 'fs'
 import * as http from 'http'
 import * as net from 'net'
 import * as path from 'path'
-import * as core from '@actions/core'
 import * as github from '@actions/github'
 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 {downloadArtifact} from '../src/internal/download/download-artifact'
+import {downloadArtifactPublic} from '../src/internal/download/download-artifact'
 import {getUserAgentString} from '../src/internal/shared/user-agent'
+import {noopLogs} from './common.test'
 
 type MockedDownloadArtifact = jest.MockedFunction<
   RestEndpointMethods['actions']['downloadArtifact']
@@ -74,206 +74,214 @@ const expectExtractedArchive = async (dir: string): Promise<void> => {
   }
 }
 
+const setup = async (): Promise<void> => {
+  noopLogs()
+  await fs.promises.mkdir(testDir, {recursive: true})
+  await createTestArchive()
+
+  process.env['GITHUB_WORKSPACE'] = fixtures.workspaceDir
+}
+
+const cleanup = async (): Promise<void> => {
+  jest.restoreAllMocks()
+  await fs.promises.rm(testDir, {recursive: true})
+  delete process.env['GITHUB_WORKSPACE']
+}
+
 describe('download-artifact', () => {
-  beforeEach(async () => {
-    jest.spyOn(core, 'debug').mockImplementation(() => {})
-    jest.spyOn(core, 'info').mockImplementation(() => {})
-    jest.spyOn(core, 'warning').mockImplementation(() => {})
+  describe('public', () => {
+    beforeEach(setup)
+    afterEach(cleanup)
 
-    await fs.promises.mkdir(testDir, {recursive: true})
-    await createTestArchive()
+    it('should successfully download an artifact to $GITHUB_WORKSPACE', async () => {
+      const downloadArtifactMock = github.getOctokit(fixtures.token).rest
+        .actions.downloadArtifact as MockedDownloadArtifact
+      downloadArtifactMock.mockResolvedValueOnce({
+        headers: {
+          location: fixtures.blobStorageUrl
+        },
+        status: 302,
+        url: '',
+        data: Buffer.from('')
+      })
 
-    process.env['GITHUB_WORKSPACE'] = fixtures.workspaceDir
-  })
+      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(
+        () => {
+          return {
+            get: getMock
+          }
+        }
+      )
 
-  afterEach(async () => {
-    jest.restoreAllMocks()
-    await fs.promises.rm(testDir, {recursive: true})
-    delete process.env['GITHUB_WORKSPACE']
-  })
-
-  it('should successfully download an artifact to $GITHUB_WORKSPACE', async () => {
-    const downloadArtifactMock = github.getOctokit(fixtures.token).rest.actions
-      .downloadArtifact as MockedDownloadArtifact
-    downloadArtifactMock.mockResolvedValueOnce({
-      headers: {
-        location: fixtures.blobStorageUrl
-      },
-      status: 302,
-      url: '',
-      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(() => {
-      return {
-        get: getMock
-      }
-    })
-
-    const response = await downloadArtifact(
-      fixtures.artifactID,
-      fixtures.repositoryOwner,
-      fixtures.repositoryName,
-      fixtures.token
-    )
-
-    expect(downloadArtifactMock).toHaveBeenCalledWith({
-      owner: fixtures.repositoryOwner,
-      repo: fixtures.repositoryName,
-      artifact_id: fixtures.artifactID,
-      archive_format: 'zip',
-      request: {
-        redirect: 'manual'
-      }
-    })
-    expect(httpClientMock).toHaveBeenCalledWith(getUserAgentString())
-    expect(getMock).toHaveBeenCalledWith(fixtures.blobStorageUrl)
-
-    expectExtractedArchive(fixtures.workspaceDir)
-
-    expect(response.success).toBe(true)
-    expect(response.downloadPath).toBe(fixtures.workspaceDir)
-  })
-
-  it('should successfully download an artifact to user defined path', async () => {
-    const customPath = path.join(testDir, 'custom')
-
-    const downloadArtifactMock = github.getOctokit(fixtures.token).rest.actions
-      .downloadArtifact as MockedDownloadArtifact
-    downloadArtifactMock.mockResolvedValueOnce({
-      headers: {
-        location: fixtures.blobStorageUrl
-      },
-      status: 302,
-      url: '',
-      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(() => {
-      return {
-        get: getMock
-      }
-    })
-
-    const response = await downloadArtifact(
-      fixtures.artifactID,
-      fixtures.repositoryOwner,
-      fixtures.repositoryName,
-      fixtures.token,
-      {
-        path: customPath
-      }
-    )
-
-    expect(downloadArtifactMock).toHaveBeenCalledWith({
-      owner: fixtures.repositoryOwner,
-      repo: fixtures.repositoryName,
-      artifact_id: fixtures.artifactID,
-      archive_format: 'zip',
-      request: {
-        redirect: 'manual'
-      }
-    })
-    expect(httpClientMock).toHaveBeenCalledWith(getUserAgentString())
-    expect(getMock).toHaveBeenCalledWith(fixtures.blobStorageUrl)
-
-    expectExtractedArchive(customPath)
-
-    expect(response.success).toBe(true)
-    expect(response.downloadPath).toBe(customPath)
-  })
-
-  it('should fail if download artifact API does not respond with location', async () => {
-    const downloadArtifactMock = github.getOctokit(fixtures.token).rest.actions
-      .downloadArtifact as MockedDownloadArtifact
-    downloadArtifactMock.mockResolvedValueOnce({
-      headers: {},
-      status: 302,
-      url: '',
-      data: Buffer.from('')
-    })
-
-    await expect(
-      downloadArtifact(
+      const response = await downloadArtifactPublic(
         fixtures.artifactID,
         fixtures.repositoryOwner,
         fixtures.repositoryName,
         fixtures.token
       )
-    ).rejects.toBeInstanceOf(Error)
 
-    expect(downloadArtifactMock).toHaveBeenCalledWith({
-      owner: fixtures.repositoryOwner,
-      repo: fixtures.repositoryName,
-      artifact_id: fixtures.artifactID,
-      archive_format: 'zip',
-      request: {
-        redirect: 'manual'
-      }
-    })
-  })
+      expect(downloadArtifactMock).toHaveBeenCalledWith({
+        owner: fixtures.repositoryOwner,
+        repo: fixtures.repositoryName,
+        artifact_id: fixtures.artifactID,
+        archive_format: 'zip',
+        request: {
+          redirect: 'manual'
+        }
+      })
+      expect(httpClientMock).toHaveBeenCalledWith(getUserAgentString())
+      expect(getMock).toHaveBeenCalledWith(fixtures.blobStorageUrl)
 
-  it('should fail if blob storage response is non-200', async () => {
-    const downloadArtifactMock = github.getOctokit(fixtures.token).rest.actions
-      .downloadArtifact as MockedDownloadArtifact
-    downloadArtifactMock.mockResolvedValueOnce({
-      headers: {
-        location: fixtures.blobStorageUrl
-      },
-      status: 302,
-      url: '',
-      data: Buffer.from('')
+      expectExtractedArchive(fixtures.workspaceDir)
+
+      expect(response.success).toBe(true)
+      expect(response.downloadPath).toBe(fixtures.workspaceDir)
     })
 
-    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(() => {
-      return {
-        get: getMock
-      }
-    })
+    it('should successfully download an artifact to user defined path', async () => {
+      const customPath = path.join(testDir, 'custom')
 
-    await expect(
-      downloadArtifact(
+      const downloadArtifactMock = github.getOctokit(fixtures.token).rest
+        .actions.downloadArtifact as MockedDownloadArtifact
+      downloadArtifactMock.mockResolvedValueOnce({
+        headers: {
+          location: fixtures.blobStorageUrl
+        },
+        status: 302,
+        url: '',
+        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(
+        () => {
+          return {
+            get: getMock
+          }
+        }
+      )
+
+      const response = await downloadArtifactPublic(
         fixtures.artifactID,
         fixtures.repositoryOwner,
         fixtures.repositoryName,
-        fixtures.token
+        fixtures.token,
+        {
+          path: customPath
+        }
       )
-    ).rejects.toBeInstanceOf(Error)
 
-    expect(downloadArtifactMock).toHaveBeenCalledWith({
-      owner: fixtures.repositoryOwner,
-      repo: fixtures.repositoryName,
-      artifact_id: fixtures.artifactID,
-      archive_format: 'zip',
-      request: {
-        redirect: 'manual'
-      }
+      expect(downloadArtifactMock).toHaveBeenCalledWith({
+        owner: fixtures.repositoryOwner,
+        repo: fixtures.repositoryName,
+        artifact_id: fixtures.artifactID,
+        archive_format: 'zip',
+        request: {
+          redirect: 'manual'
+        }
+      })
+      expect(httpClientMock).toHaveBeenCalledWith(getUserAgentString())
+      expect(getMock).toHaveBeenCalledWith(fixtures.blobStorageUrl)
+
+      expectExtractedArchive(customPath)
+
+      expect(response.success).toBe(true)
+      expect(response.downloadPath).toBe(customPath)
+    })
+
+    it('should fail if download artifact API does not respond with location', async () => {
+      const downloadArtifactMock = github.getOctokit(fixtures.token).rest
+        .actions.downloadArtifact as MockedDownloadArtifact
+      downloadArtifactMock.mockResolvedValueOnce({
+        headers: {},
+        status: 302,
+        url: '',
+        data: Buffer.from('')
+      })
+
+      await expect(
+        downloadArtifactPublic(
+          fixtures.artifactID,
+          fixtures.repositoryOwner,
+          fixtures.repositoryName,
+          fixtures.token
+        )
+      ).rejects.toBeInstanceOf(Error)
+
+      expect(downloadArtifactMock).toHaveBeenCalledWith({
+        owner: fixtures.repositoryOwner,
+        repo: fixtures.repositoryName,
+        artifact_id: fixtures.artifactID,
+        archive_format: 'zip',
+        request: {
+          redirect: 'manual'
+        }
+      })
+    })
+
+    it('should fail if blob storage response is non-200', async () => {
+      const downloadArtifactMock = github.getOctokit(fixtures.token).rest
+        .actions.downloadArtifact as MockedDownloadArtifact
+      downloadArtifactMock.mockResolvedValueOnce({
+        headers: {
+          location: fixtures.blobStorageUrl
+        },
+        status: 302,
+        url: '',
+        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(
+        () => {
+          return {
+            get: getMock
+          }
+        }
+      )
+
+      await expect(
+        downloadArtifactPublic(
+          fixtures.artifactID,
+          fixtures.repositoryOwner,
+          fixtures.repositoryName,
+          fixtures.token
+        )
+      ).rejects.toBeInstanceOf(Error)
+
+      expect(downloadArtifactMock).toHaveBeenCalledWith({
+        owner: fixtures.repositoryOwner,
+        repo: fixtures.repositoryName,
+        artifact_id: fixtures.artifactID,
+        archive_format: 'zip',
+        request: {
+          redirect: 'manual'
+        }
+      })
+      expect(httpClientMock).toHaveBeenCalledWith(getUserAgentString())
+      expect(getMock).toHaveBeenCalledWith(fixtures.blobStorageUrl)
     })
-    expect(httpClientMock).toHaveBeenCalledWith(getUserAgentString())
-    expect(getMock).toHaveBeenCalledWith(fixtures.blobStorageUrl)
   })
 })

packages/artifact/__tests__/path-and-artifact-name-validation.test.ts

@@ -3,15 +3,11 @@ import {
   validateFilePath
 } from '../src/internal/upload/path-and-artifact-name-validation'
 
-import * as core from '@actions/core'
+import {noopLogs} from './common.test'
 
 describe('Path and artifact name validation', () => {
   beforeAll(() => {
-    // mock all output so that there is less noise when running tests
-    jest.spyOn(console, 'log').mockImplementation(() => {})
-    jest.spyOn(core, 'debug').mockImplementation(() => {})
-    jest.spyOn(core, 'info').mockImplementation(() => {})
-    jest.spyOn(core, 'warning').mockImplementation(() => {})
+    noopLogs()
   })
 
   it('Check Artifact Name for any invalid characters', () => {

packages/artifact/__tests__/upload-artifact.test.ts

@@ -1,4 +1,3 @@
-import * as core from '@actions/core'
 import * as uploadZipSpecification from '../src/internal/upload/upload-zip-specification'
 import * as zip from '../src/internal/upload/zip'
 import * as util from '../src/internal/shared/util'
@@ -7,14 +6,11 @@ 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'
 
 describe('upload-artifact', () => {
   beforeEach(() => {
-    // mock all output so that there is less noise when running tests
-    jest.spyOn(console, 'log').mockImplementation(() => {})
-    jest.spyOn(core, 'debug').mockImplementation(() => {})
-    jest.spyOn(core, 'info').mockImplementation(() => {})
-    jest.spyOn(core, 'warning').mockImplementation(() => {})
+    noopLogs()
   })
 
   afterEach(() => {

packages/artifact/__tests__/upload-zip-specification.test.ts

@@ -1,11 +1,11 @@
 import * as io from '../../io/src/io'
 import * as path from 'path'
 import {promises as fs} from 'fs'
-import * as core from '@actions/core'
 import {
   getUploadZipSpecification,
   validateRootDirectory
 } from '../src/internal/upload/upload-zip-specification'
+import {noopLogs} from './common.test'
 
 const root = path.join(__dirname, '_temp', 'upload-specification')
 const goodItem1Path = path.join(
@@ -51,11 +51,7 @@ const artifactFilesToUpload = [
 
 describe('Search', () => {
   beforeAll(async () => {
-    // mock all output so that there is less noise when running tests
-    jest.spyOn(console, 'log').mockImplementation(() => {})
-    jest.spyOn(core, 'debug').mockImplementation(() => {})
-    jest.spyOn(core, 'info').mockImplementation(() => {})
-    jest.spyOn(core, 'warning').mockImplementation(() => {})
+    noopLogs()
 
     // clear temp directory
     await io.rmRF(root)

packages/artifact/__tests__/util.test.ts

@@ -1,13 +1,12 @@
 import * as config from '../src/internal/shared/config'
 import * as util from '../src/internal/shared/util'
 
+export const testRuntimeToken =
+  'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwic2NwIjoiQWN0aW9ucy5FeGFtcGxlIEFjdGlvbnMuQW5vdGhlckV4YW1wbGU6dGVzdCBBY3Rpb25zLlJlc3VsdHM6Y2U3ZjU0YzctNjFjNy00YWFlLTg4N2YtMzBkYTQ3NWY1ZjFhOmNhMzk1MDg1LTA0MGEtNTI2Yi0yY2U4LWJkYzg1ZjY5Mjc3NCIsImlhdCI6MTUxNjIzOTAyMn0.XYnI_wHPBlUi1mqYveJnnkJhp4dlFjqxzRmISPsqfw8'
+
 describe('get-backend-ids-from-token', () => {
   it('should return backend ids when the token is valid', () => {
-    jest
-      .spyOn(config, 'getRuntimeToken')
-      .mockReturnValue(
-        'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwic2NwIjoiQWN0aW9ucy5FeGFtcGxlIEFjdGlvbnMuQW5vdGhlckV4YW1wbGU6dGVzdCBBY3Rpb25zLlJlc3VsdHM6Y2U3ZjU0YzctNjFjNy00YWFlLTg4N2YtMzBkYTQ3NWY1ZjFhOmNhMzk1MDg1LTA0MGEtNTI2Yi0yY2U4LWJkYzg1ZjY5Mjc3NCIsImlhdCI6MTUxNjIzOTAyMn0.XYnI_wHPBlUi1mqYveJnnkJhp4dlFjqxzRmISPsqfw8'
-      )
+    jest.spyOn(config, 'getRuntimeToken').mockReturnValue(testRuntimeToken)
 
     const backendIds = util.getBackendIdsFromToken()
     expect(backendIds.workflowRunBackendId).toBe(

packages/artifact/package.json

@@ -30,7 +30,7 @@
   },
   "scripts": {
     "audit-moderate": "npm install && npm audit --json --audit-level=moderate > audit.json",
-    "test": "echo \"Error: run tests from root\" && exit 1",
+    "test": "cd ../../ && npm run test ./packages/artifact",
     "bootstrap": "cd ../../ && npm run bootstrap",
     "tsc-run": "tsc",
     "tsc": "npm run bootstrap && npm run tsc-run"

packages/artifact/src/internal/download/download-artifact.ts

@@ -10,7 +10,10 @@ import {
 import {getUserAgentString} from '../shared/user-agent'
 import {getGitHubWorkspaceDir} from '../shared/config'
 import {internalArtifactTwirpClient} from '../shared/artifact-twirp-client'
-import {GetSignedArtifactURLRequest, ListArtifactsRequest} from 'src/generated'
+import {
+  GetSignedArtifactURLRequest,
+  ListArtifactsRequest
+} from '../../generated'
 import {getBackendIdsFromToken} from '../shared/util'
 
 const scrubQueryParameters = (url: string): string => {

packages/artifact/src/internal/shared/artifact-twirp-client.ts

@@ -160,13 +160,13 @@ class ArtifactHttpClient implements Rpc {
 
 export function internalArtifactTwirpClient(options?: {
   maxAttempts?: number
-  baseRetryIntervalMilliseconds?: number
+  retryIntervalMs?: number
   retryMultiplier?: number
 }): ArtifactServiceClientJSON {
   const client = new ArtifactHttpClient(
     getUserAgentString(),
     options?.maxAttempts,
-    options?.baseRetryIntervalMilliseconds,
+    options?.retryIntervalMs,
     options?.retryMultiplier
   )
   return new ArtifactServiceClientJSON(client)