Add tests for upload-sarif

[mbg]

This PR takes parts of #3157:

• Moves logic from upload-sarif-action.ts to upload-sarif.ts
• Adds unit test coverage for the functions in upload-sarif.ts
• Fixes sarif-ids output format to match documentation
• Throws a ConfigurationError if no SARIF files were uploaded, matching the behaviour of upload-sarif prior to Fix upload-sarif Action failing if there are no Code Scanning SARIF files #3123

This doesn't make any further changes to the actual upload logic beyond the hotfix we added in #3160. I am planning a follow-up PR to improve on that after having come up with an idea for a good approach over the weekend, but want to the tests, sarif-ids fix, and error for the case where no files were uploaded merged first.

Risk assessment

For internal use only. Please select the risk level of this change:

• Low risk: Changes are fully under feature flags, or have been fully tested and validated in pre-production environments and are highly observable, or are documentation or test only.

Merge / deployment checklist

• Confirm this change is backwards compatible with existing workflows.
• Consider adding a changelog entry for this change.
• Confirm the readme and docs have been updated if necessary.

[Copilot]

Pull Request Overview

This PR refactors the upload-sarif functionality by extracting upload logic from the action file into a separate module and adding comprehensive unit tests. The changes also fix the output format for sarif-ids and add error handling for cases where no SARIF files are found.

Key changes:

• Moves core upload logic from upload-sarif-action.ts to upload-sarif.ts for better separation of concerns
• Adds extensive unit tests covering various SARIF file scenarios and upload combinations
• Changes sarif-ids output format from array to object structure for easier consumption

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/upload-sarif.ts	New module containing extracted upload logic with findAndUpload and uploadSarif functions
src/upload-sarif.test.ts	Comprehensive unit tests for the new upload-sarif module functions
src/upload-sarif-action.ts	Refactored to use the new upload-sarif module and simplified upload flow
pr-checks/checks/upload-quality-sarif.yml	Updated test to use new sarif-ids object format
lib/upload-sarif-action.js	Generated JavaScript reflecting the TypeScript changes
.github/workflows/__upload-quality-sarif.yml	Updated workflow to use new sarif-ids object format

[esbena]

LGTM.

It's not blocking, but I'm getting a bit concerned about the complexity of the tests, especially
2adc894. It's the classic tradeoff between what tests current behaviour/API and what is maintainable.

[esbena]

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

[mbg]

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.

[esbena]

right. My eyes missed the AnalysisKind. prefix.

[mbg]

I'm getting a bit concerned about the complexity of the tests

I could modify UploadResult to include the array of files, but then we'd probably have to mock a bunch of other functions that uploadSpecifiedFiles calls or have more things for the tests to set up.

I'm inclined to accept the tests as they are for now, since I am planning to make further changes anyway and they might affect these tests and allow us to simplify this again later.