ci: aggregate integration tests matrix into single check#2569
Merged
Conversation
Rename matrix job to 'run' and add a non-matrix 'integration_tests' aggregator that depends on it. This produces a single 'integration-tests / integration_tests' status check that can be used as the required check in branch protection, regardless of how many matrix shards exist.
Contributor
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
PR SummaryMedium Risk Overview Potential issue: renaming the matrix job changes the per-shard check names and could break existing branch protection rules or automations that reference the old Reviewed by Cursor Bugbot for commit 3d08aad. Bugbot is set up for automated code reviews on this repo. Configure here. |
❌ 108 Tests Failed:
View the top 3 failed test(s) by shortest run time
View the full list of 95 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — straightforward CI matrix aggregator pattern.
Extended reasoning...
Overview
Single-file change to .github/workflows/integration_tests.yml: renames the matrix job from integration_tests to run, and adds a new non-matrix integration_tests job that depends on it via needs: run and fails unless needs.run.result == 'success'. This produces a single stable status check usable as a required check in branch protection, independent of matrix shard count.
Security risks
None. This is a CI workflow change that only affects how status checks are reported. No secrets handling, no permissions changes, no production code paths touched.
Level of scrutiny
Low. This is a CI-only config change with a well-known pattern (matrix aggregator job). The aggregator logic is correct: if: always() ensures the job runs even when matrix shards fail, and the explicit needs.run.result check ensures it actually fails in those cases (since always() alone would let it pass).
Other factors
The cursor bot flagged a 'Medium Risk' concern about existing branch protection rules referencing the old shard names — but this is exactly the problem the PR is trying to solve, and the PR author would need to update branch protection as part of the rollout regardless. The codecov test failures appear to be pre-existing flaky tests in main (33-55% flake rate) unrelated to this change. No human reviewer comments are outstanding.
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Rename the matrix job from
integration_teststorun, and add a non-matrixintegration_testsaggregator job that depends on it.This produces a single
integration-tests / integration_testsstatus check that can be used as the required check in branch protection — independent of how many matrix shards there are. Individual shards still appear asintegration-tests / run (uncompressed),integration-tests / run (zstd1), etc.Why
Today the matrix job is named
integration_tests, so each shard renders asintegration-tests / integration_tests (<name>). There's no single aggregated check, which makes branch protection awkward (you have to list each shard explicitly, and adding/removing a shard requires updating branch protection).Test plan
integration-tests / run (...)).integration-tests / integration_testscheck appears at the end and reflects pass/fail of the matrix.needs.run.resultcheck, which is required becauseif: always()would otherwise let the aggregator pass on a skipped/failed matrix).