From 610c7c68e37196cc9fa91a01b93112e77c0a82b8 Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Thu, 9 Oct 2025 15:24:02 +0200 Subject: [PATCH] Address review --- src/upload-lib.test.ts | 172 +++++++++++++++++------------------------ 1 file changed, 70 insertions(+), 102 deletions(-) diff --git a/src/upload-lib.test.ts b/src/upload-lib.test.ts index b5a4da758..7a5be6382 100644 --- a/src/upload-lib.test.ts +++ b/src/upload-lib.test.ts @@ -3,7 +3,7 @@ import * as path from "path"; import * as github from "@actions/github"; import { HTTPError } from "@actions/tool-cache"; -import test, { ExecutionContext } from "ava"; +import test from "ava"; import * as sinon from "sinon"; import * as analyses from "./analyses"; @@ -873,122 +873,90 @@ function createMockSarif(id?: string, tool?: string) { }; } -const uploadPayloadMacro = test.macro({ - exec: async ( - t: ExecutionContext, - options: { - analysis: analyses.AnalysisConfig; - body: ( - t: ExecutionContext, - upload: () => Promise, - requestStub: sinon.SinonStub, - mockData: { - payload: { sarif: string; commit_sha: string }; - owner: string; - repo: string; - response: { - status: number; - data: { id: string }; - headers: any; - url: string; - }; - }, - ) => void | Promise; +function uploadPayloadFixtures(analysis: analyses.AnalysisConfig) { + const mockData = { + payload: { sarif: "base64data", commit_sha: "abc123" }, + owner: "test-owner", + repo: "test-repo", + response: { + status: 200, + data: { id: "uploaded-sarif-id" }, + headers: {}, + url: analysis.target, }, - ) => { - const mockData = { - payload: { sarif: "base64data", commit_sha: "abc123" }, - owner: "test-owner", - repo: "test-repo", - response: { - status: 200, - data: { id: "uploaded-sarif-id" }, - headers: {}, - url: options.analysis.target, + }; + const client = github.getOctokit("123"); + sinon.stub(api, "getApiClient").value(() => client); + const requestStub = sinon.stub(client, "request"); + + const upload = async () => + uploadLib.uploadPayload( + mockData.payload, + { + owner: mockData.owner, + repo: mockData.repo, }, - }; + getRunnerLogger(true), + analysis, + ); - const client = github.getOctokit("123"); - sinon.stub(api, "getApiClient").value(() => client); - const requestStub = sinon.stub(client, "request"); - - const upload = async () => - uploadLib.uploadPayload( - mockData.payload, - { - owner: mockData.owner, - repo: mockData.repo, - }, - getRunnerLogger(true), - options.analysis, - ); - - await options.body(t, upload, requestStub, mockData); - }, - title: (providedTitle = "", options: { analysis: analyses.AnalysisConfig }) => - `uploadPayload - ${options.analysis.name} - ${providedTitle}`, -}); + return { + upload, + requestStub, + mockData, + }; +} for (const analysis of [CodeScanning, CodeQuality]) { - test("uploads successfully", uploadPayloadMacro, { - analysis, - body: async (t, upload, requestStub, mockData) => { - requestStub - .withArgs(analysis.target, { - owner: mockData.owner, - repo: mockData.repo, - data: mockData.payload, - }) - .onFirstCall() - .returns(Promise.resolve(mockData.response)); - const result = await upload(); - t.is(result, mockData.response.data.id); - t.true(requestStub.calledOnce); - }, + test(`uploadPayload on ${analysis.name} uploads successfully`, async (t) => { + const { upload, requestStub, mockData } = uploadPayloadFixtures(analysis); + requestStub + .withArgs(analysis.target, { + owner: mockData.owner, + repo: mockData.repo, + data: mockData.payload, + }) + .onFirstCall() + .returns(Promise.resolve(mockData.response)); + const result = await upload(); + t.is(result, mockData.response.data.id); + t.true(requestStub.calledOnce); }); for (const envVar of [ "CODEQL_ACTION_SKIP_SARIF_UPLOAD", "CODEQL_ACTION_TEST_MODE", ]) { - test(`skips upload when ${envVar} is set`, uploadPayloadMacro, { - analysis, - body: async (t, upload, requestStub, mockData) => - withTmpDir(async (tmpDir) => { - process.env.RUNNER_TEMP = tmpDir; - process.env[envVar] = "true"; - const result = await upload(); - t.is(result, "dummy-sarif-id"); - t.false(requestStub.called); + test(`uploadPayload on ${analysis.name} skips upload when ${envVar} is set`, async (t) => { + const { upload, requestStub, mockData } = uploadPayloadFixtures(analysis); + await withTmpDir(async (tmpDir) => { + process.env.RUNNER_TEMP = tmpDir; + process.env[envVar] = "true"; + const result = await upload(); + t.is(result, "dummy-sarif-id"); + t.false(requestStub.called); - const payloadFile = path.join( - tmpDir, - `payload-${analysis.kind}.json`, - ); - t.true(fs.existsSync(payloadFile)); + const payloadFile = path.join(tmpDir, `payload-${analysis.kind}.json`); + t.true(fs.existsSync(payloadFile)); - const savedPayload = JSON.parse(fs.readFileSync(payloadFile, "utf8")); - t.deepEqual(savedPayload, mockData.payload); - }), + const savedPayload = JSON.parse(fs.readFileSync(payloadFile, "utf8")); + t.deepEqual(savedPayload, mockData.payload); + }); }); } - test("handles error", uploadPayloadMacro, { - analysis, - body: async (t, upload, requestStub) => { - const wrapApiConfigurationErrorStub = sinon.stub( - api, - "wrapApiConfigurationError", - ); - const originalError = new HTTPError(404); - const wrappedError = new Error("Wrapped error message"); - requestStub.rejects(originalError); - wrapApiConfigurationErrorStub - .withArgs(originalError) - .returns(wrappedError); - await t.throwsAsync(upload, { - is: wrappedError, - }); - }, + test(`uploadPayload on ${analysis.name} wraps request errors using wrapApiConfigurationError`, async (t) => { + const { upload, requestStub } = uploadPayloadFixtures(analysis); + const wrapApiConfigurationErrorStub = sinon.stub( + api, + "wrapApiConfigurationError", + ); + const originalError = new HTTPError(404); + const wrappedError = new Error("Wrapped error message"); + requestStub.rejects(originalError); + wrapApiConfigurationErrorStub.withArgs(originalError).returns(wrappedError); + await t.throwsAsync(upload, { + is: wrappedError, + }); }); }