ci: Make jest matrix config dynamic for PR runs#116662
Conversation
📊 Type Coverage Diff✅ no issues found |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 12c4119. Configure here.
| frontend_all: ${{ steps.changes.outputs.frontend_all }} | ||
| merge_base: ${{ steps.merge_base.outputs.merge_base }} | ||
| merge_base_strategy: ${{ steps.merge_base.outputs.merge_base_strategy }} | ||
| jest_test_matrix: ${{ steps.merge_base.outputs.jest_test_matrix }} |
There was a problem hiding this comment.
Bug: The workflow references steps.merge_base.outputs.jest_test_matrix, but the jest_test_matrix output is generated by a different, unnamed step, causing the reference to fail.
Severity: HIGH
Suggested Fix
Assign an id to the "Jest Matrix Config" step (e.g., id: jest_matrix_config). Then, update the output reference at line 34 to use the correct step ID, for example: jest_test_matrix: ${{ steps.jest_matrix_config.outputs.jest_test_matrix }}.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: .github/workflows/frontend.yml#L34
Potential issue: The "Jest Matrix Config" step generates the `jest_test_matrix` output
but lacks an `id`. The workflow incorrectly attempts to reference this output from the
`merge_base` step at line 34, using `steps.merge_base.outputs.jest_test_matrix`. Since
the `merge_base` step does not produce this output, the expression resolves to an empty
string. This empty string is then passed to `fromJson()` in the downstream
`frontend-jest-tests` job, which will cause the workflow to fail at runtime.
12c4119 to
5fed63b
Compare

Make the matrix config for jest dynamic: only when it's using the
changedSincestrategy, which only happens with a PRThis helps fix the problem where we have 8 runners being used when we actually intended to only run tests related to the changes at hand.
It doesn't fully improve the PR run though: there are more internal conditions like
JEST_TESTSandJEST_LIST_TESTS_INNERwhich could load up a long list of tests, or the same list of tests in each matrix instance. What this does though it simplify the runtime env, so we're just running everything on one worker node.We could endup with slower test runs for PRs that codemod/change a lot of files at once, but this is something we can overcome in a followup if we move the call to
jest --listTestsfrom inside jest.config.ts, to frontend.yml instead.