Skip to content

Add tests for upload-sarif #3166

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mbg merged 8 commits into from
Sep 29, 2025
Merged

Add tests for upload-sarif #3166

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5fc9e66
Move `findAndUpload` to a new module
mbg Sep 29, 2025
9f452fa
Move core `upload-sarif` logic to `upload-sarif` module
mbg Sep 29, 2025
6e0b087
Add some tests for `findAndUpload` and `uploadSarif`
mbg Sep 25, 2025
5b3f0de
Test that uploaded files match expectations for each analysis kind
mbg Sep 26, 2025
2adc894
Tests: ensure `uploadSpecifiedFiles` wasn't called if we don't expect…
mbg Sep 26, 2025
5fd2cfe
Fail if no SARIF files were uploaded
mbg Sep 29, 2025
73fbfb0
Update src/upload-sarif.test.ts
mbg Sep 29, 2025
9715962
Fix condition in test workflow
mbg Sep 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 63 additions & 18 deletions src/upload-sarif.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { getRunnerLogger } from "./logging";
import { createFeatures, setupTests } from "./testing-utils";
import { UploadResult } from "./upload-lib";
import * as uploadLib from "./upload-lib";
import { findAndUpload, uploadSarif, UploadSarifResults } from "./upload-sarif";
import { findAndUpload, uploadSarif } from "./upload-sarif";
import * as util from "./util";

setupTests(test);
Expand Down Expand Up @@ -85,17 +85,25 @@ test(
},
);

interface UploadSarifExpectedResult {
uploadResult?: UploadResult;
expectedFiles?: string[];
}

const uploadSarifMacro = test.macro({
exec: async (
t: ExecutionContext<unknown>,
sarifFiles: string[],
sarifPath: (tempDir: string) => string = (tempDir) => tempDir,
expectedResult: UploadSarifResults,
expectedResult: Partial<Record<AnalysisKind, UploadSarifExpectedResult>>,
) => {
await util.withTmpDir(async (tempDir) => {
const logger = getRunnerLogger(true);
const testPath = sarifPath(tempDir);
const features = createFeatures([]);

const toFullPath = (filename: string) => path.join(tempDir, filename);

const uploadSpecifiedFiles = sinon.stub(
uploadLib,
"uploadSpecifiedFiles",
Expand All @@ -113,16 +121,38 @@ const uploadSarifMacro = test.macro({
? CodeScanning
: CodeQuality,
)
.resolves(expectedResult[analysisKind as AnalysisKind]);
.resolves(expectedResult[analysisKind as AnalysisKind]?.uploadResult);
}

for (const sarifFile of sarifFiles) {
fs.writeFileSync(path.join(tempDir, sarifFile), "");
const fullSarifPaths = sarifFiles.map(toFullPath);
for (const sarifFile of fullSarifPaths) {
fs.writeFileSync(sarifFile, "");
}

const actual = await uploadSarif(logger, features, "", testPath);

t.deepEqual(actual, expectedResult);
for (const analysisKind of Object.values(AnalysisKind)) {
const analyisKindResult = expectedResult[analysisKind];
if (analyisKindResult) {
t.deepEqual(actual[analysisKind], analyisKindResult.uploadResult);

t.assert(
uploadSpecifiedFiles.calledWith(
analyisKindResult.expectedFiles?.map(toFullPath) ??
fullSarifPaths,
sinon.match.any,
sinon.match.any,
features,
logger,
analysisKind === AnalysisKind.CodeScanning
? CodeScanning
: CodeQuality,
Comment on lines +149 to +151
Copy link
Contributor

@esbena esbena Sep 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could this just be analysisKind? Or do we prefer the explicit casing?

Copy link
Member Author

@mbg mbg Sep 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

analysisKind: AnalysisKind while CodeScanning: AnalysisConfig and CodeQuality: AnalysisConfig. It maps the kind to the matching configuration. We could have this as a function for reusability instead of this ad-hoc mechanism, but we can't just use analysisKind here since we need an AnalysisConfig object for uploadSpecifiedFiles.

Copy link
Contributor

@esbena esbena Sep 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right. My eyes missed the AnalysisKind. prefix.

),
);
} else {
t.is(actual[analysisKind], undefined);
}
}
});
},
title: (providedTitle = "") => `uploadSarif - ${providedTitle}`,
Expand All @@ -135,8 +165,10 @@ test(
(tempDir) => path.join(tempDir, "test.sarif"),
{
"code-scanning": {
statusReport: {},
sarifID: "code-scanning-sarif",
uploadResult: {
statusReport: {},
sarifID: "code-scanning-sarif",
},
},
},
);
Expand All @@ -148,8 +180,10 @@ test(
(tempDir) => path.join(tempDir, "test.json"),
{
"code-scanning": {
statusReport: {},
sarifID: "code-scanning-sarif",
uploadResult: {
statusReport: {},
sarifID: "code-scanning-sarif",
},
},
},
);
Expand All @@ -161,8 +195,11 @@ test(
undefined,
{
"code-scanning": {
statusReport: {},
sarifID: "code-scanning-sarif",
uploadResult: {
statusReport: {},
sarifID: "code-scanning-sarif",
},
expectedFiles: ["test.sarif"],
},
},
);
Expand All @@ -174,8 +211,10 @@ test(
(tempDir) => path.join(tempDir, "test.quality.sarif"),
{
"code-quality": {
statusReport: {},
sarifID: "code-quality-sarif",
uploadResult: {
statusReport: {},
sarifID: "code-quality-sarif",
},
},
},
);
Expand All @@ -187,12 +226,18 @@ test(
undefined,
{
"code-scanning": {
statusReport: {},
sarifID: "code-scanning-sarif",
uploadResult: {
statusReport: {},
sarifID: "code-scanning-sarif",
},
expectedFiles: ["test.sarif"],
},
"code-quality": {
statusReport: {},
sarifID: "code-quality-sarif",
uploadResult: {
statusReport: {},
sarifID: "code-quality-sarif",
},
expectedFiles: ["test.quality.sarif"],
},
},
);