Testing: Provide default value for more environment variables in setupActionsVars#3540
Testing: Provide default value for more environment variables in setupActionsVars#3540henrymercer merged 4 commits intomainfrom
setupActionsVars#3540Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the unit-test harness by providing a centralized set of default GitHub Actions environment variables and allowing per-test overrides, reducing repetitive process.env setup across tests.
Changes:
- Added
DEFAULT_ACTIONS_VARSand extendedsetupActionsVarsto apply defaults plus optional overrides. - Updated several tests to rely on
setupActionsVars(..., overrides)instead of manually setting many env vars. - Adjusted a few test assertions to reference the shared defaults where appropriate.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/testing-utils.ts | Introduces DEFAULT_ACTIONS_VARS and adds an overrides parameter to setupActionsVars to standardize Actions env setup in tests. |
| src/status-report.test.ts | Switches to using setupActionsVars overrides for event/run metadata instead of ad-hoc env mutation. |
| src/setup-codeql.test.ts | Uses setupActionsVars(..., { GITHUB_EVENT_NAME: "dynamic" }) in relevant tests to reduce repeated env setup. |
| src/init-action-post-helper.test.ts | Replaces repeated env setup with setupActionsVars and aligns some expectations with shared defaults. |
| src/analyze-action-input.test.ts | Removes redundant manual env setup and relies on setupActionsVars for standard Actions vars. |
| src/analyze-action-env.test.ts | Same as above: simplifies env initialization via setupActionsVars. |
mbg
left a comment
mbg
left a comment
There was a problem hiding this comment.
Thank you for following-up on this suggestion! 💜
Looks good. I just have one suggestion for how we might want to type this differently.
src/testing-utils.ts
Outdated
| export const DEFAULT_ACTIONS_VARS: Record<string, string> = { | ||
| GITHUB_ACTION_REPOSITORY: "github/codeql-action", | ||
| GITHUB_API_URL: "https://api.github.com", | ||
| GITHUB_EVENT_NAME: "push", | ||
| GITHUB_JOB: "test-job", | ||
| GITHUB_REF: "refs/heads/main", | ||
| GITHUB_REPOSITORY: "github/codeql-action-testing", | ||
| GITHUB_RUN_ATTEMPT: "1", | ||
| GITHUB_RUN_ID: "1", | ||
| GITHUB_SERVER_URL: "https://github.com", | ||
| GITHUB_SHA: "0".repeat(40), | ||
| GITHUB_WORKFLOW: "test-workflow", | ||
| RUNNER_OS: "Linux", | ||
| }; |
There was a problem hiding this comment.
Minor: Could we remove the Record<string, string> type annotation and add as const satisfies Record<string, string>? Then we could retrieve the keys from this as values/type of all Actions environment variables.
src/testing-utils.ts
Outdated
| export function setupActionsVars( | ||
| tempDir: string, | ||
| toolsDir: string, | ||
| overrides?: Partial<Record<string, string>>, |
There was a problem hiding this comment.
Following from my first suggestion, we could then type this as something like Partial<Record<keyof typeof DEFAULT_ACTIONS_VARS, string>> to limit it to just Actions variables so that this doesn't just turn into a generic setupEnv function?
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
As suggested in #3537. I don't claim that this is comprehensive, but it introduces a pattern we can make use of to hopefully simplify our tests.