Prefer providing CodeQL via dependency injection #3011
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the CodeQL Action codebase to prefer dependency injection of CodeQL instances over using global cached instances. The change makes CodeQL usage more explicit and facilitates better testing by providing stub CodeQL objects directly to functions that need them.
- Replaces
setCodeQLwithcreateStubCodeQLto create stub instances without modifying global state - Updates functions to accept
CodeQLparameters instead of internally callinggetCodeQL - Modifies test files to pass stub CodeQL instances explicitly
Reviewed Changes
Copilot reviewed 28 out of 42 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/trap-caching.test.ts | Updates test to use createStubCodeQL instead of setCodeQL |
| src/testing-utils.ts | Changes test utility to use createStubCodeQL |
| src/init-action-post.ts | Adds CodeQL parameter to initActionPostHelper.run call |
| src/init-action-post-helper.ts | Updates function signatures to accept CodeQL via dependency injection |
| src/init-action-post-helper.test.ts | Passes stub CodeQL instances to test functions |
| src/debug-artifacts.ts | Updates functions to accept CodeQL parameter instead of calling getCodeQL internally |
| src/database-upload.test.ts | Changes test to use createStubCodeQL |
| src/config-utils.test.ts | Updates multiple test cases to use createStubCodeQL and removes unused getCachedCodeQL calls |
| src/codeql.ts | Refactors setCodeQL to call createStubCodeQL and removes getCachedCodeQL function |
| src/analyze.ts | Updates runQueries to accept CodeQL parameter |
| src/analyze.test.ts | Updates test to use createStubCodeQL and pass CodeQL instance to runQueries |
| src/analyze-action.ts | Passes CodeQL instance to runQueries call |
| src/analyze-action-input.test.ts | Adds assertions to verify function calls |
| src/analyze-action-env.test.ts | Adds assertions to verify function calls |
| lib/* files | Generated JavaScript equivalents of TypeScript changes |
| @@ -42,7 +42,7 @@ test("status report fields", async (t) => { | |||
| sinon.stub(uploadLib, "validateSarifFileSchema"); | |||
|
|
|||
| for (const language of Object.values(KnownLanguage)) { | |||
There was a problem hiding this comment.
Just an observation unrelated to this PR: KnownLanguage doesn't include "actions" anymore, so that doesn't get tested here.
Agreed, though this is trickier since we have a couple of tests that run the top-level Actions, namely |
This makes it more obvious when CodeQL is being used, and therefore when we might need to provide stubs in tests.
Merge / deployment checklist