ci: decouple verify-groups from test matrix#562
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v0.42-dev #562 +/- ##
=============================================
+ Coverage 66.13% 66.35% +0.21%
=============================================
Files 311 311
Lines 64757 64976 +219
=============================================
+ Hits 42829 43116 +287
+ Misses 21928 21860 -68
|
|
@CodeRabbit review |
Move group listing into `build-and-test.yml` so test jobs start immediately instead of waiting for `verify-groups` to complete. `verify-groups` still runs in parallel as a validation guard.
xdustinface
force-pushed
the
ci/decouple-tests
branch
from
ac2f2c8 to
f9cedd1
Compare
📝 WalkthroughWalkthroughThe PR transitions CI configuration from dynamic matrix generation to a static hardcoded approach. The verification script now validates that the hardcoded matrix in the workflow matches the expected group keys. The workflow_call input for groups is removed, and downstream jobs no longer receive dynamically generated group matrices. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/rust.yml (1)
48-55: Re-check required GitHub checks after this rename/decoupling.This PR makes the verification job independent from the OS jobs and also changes the check GitHub reports for that guard. If branch protection or merge-queue rules still reference the previous verification check, merges can hang on a missing status; if they only require the OS jobs, a failing
Verify Test Groupsrun stops being a hard gate. Please update the required checks alongside this change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/rust.yml around lines 48 - 55, The GitHub workflow renamed/decoupled the verification job to "verify-groups" (name: "Verify Test Groups", step invoking python .github/scripts/ci_config.py verify-groups), so update any branch protection rules, required status checks, and merge-queue rules that referenced the old verification check to reference this new check name; locate references in repository/project settings and CI configuration dashboards and replace the old check identifier with "Verify Test Groups" (or the workflow identifier "verify-groups") so branch protection and merge queues correctly reflect the new guard..github/scripts/ci_config.py (1)
99-117: Parsebuild-and-test.ymlas YAML instead of regex.This works with the current inline list, but it will false-fail on valid YAML refactors like block-sequence syntax or on an earlier unrelated
group:key. Sincepyyamlis already in use here, readingjobs.test.strategy.matrix.groupdirectly is more robust.♻️ Proposed refactor
- try: - with open(workflow_file) as f: - content = f.read() - except FileNotFoundError: - github_error(f"Workflow file not found: {workflow_file}") - return 1 - - match = re.search(r'group:\s*\[([^\]]+)\]', content) - if not match: - github_error(f"No hardcoded group matrix found in {workflow_file}") - return 1 - - actual = sorted( - name.strip().strip('"').strip("'") for name in match.group(1).split(",") - ) + workflow = load_yaml(workflow_file) + try: + actual = sorted(workflow["jobs"]["test"]["strategy"]["matrix"]["group"]) + except (KeyError, TypeError): + github_error(f"No hardcoded group matrix found in {workflow_file}") + return 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/scripts/ci_config.py around lines 99 - 117, The current check reads .github/workflows/build-and-test.yml into content and uses re.search('group:\s*\[([^\]]+)\]') to extract the hardcoded matrix which will break on valid YAML refactors; instead parse the file with the project's YAML loader (pyyaml) and read jobs -> test -> strategy -> matrix -> group to get the actual list. Replace the regex block that computes actual (and the FileNotFoundError handling) with YAML load of workflow_file, validate the presence of jobs.test.strategy.matrix.group, coerce that node to a list of strings, and then sort/compare to the expected groups variable; preserve github_error(...) usage for missing or malformed nodes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/scripts/ci_config.py:
- Around line 99-117: The current check reads
.github/workflows/build-and-test.yml into content and uses
re.search('group:\s*\[([^\]]+)\]') to extract the hardcoded matrix which will
break on valid YAML refactors; instead parse the file with the project's YAML
loader (pyyaml) and read jobs -> test -> strategy -> matrix -> group to get the
actual list. Replace the regex block that computes actual (and the
FileNotFoundError handling) with YAML load of workflow_file, validate the
presence of jobs.test.strategy.matrix.group, coerce that node to a list of
strings, and then sort/compare to the expected groups variable; preserve
github_error(...) usage for missing or malformed nodes.
In @.github/workflows/rust.yml:
- Around line 48-55: The GitHub workflow renamed/decoupled the verification job
to "verify-groups" (name: "Verify Test Groups", step invoking python
.github/scripts/ci_config.py verify-groups), so update any branch protection
rules, required status checks, and merge-queue rules that referenced the old
verification check to reference this new check name; locate references in
repository/project settings and CI configuration dashboards and replace the old
check identifier with "Verify Test Groups" (or the workflow identifier
"verify-groups") so branch protection and merge queues correctly reflect the new
guard.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bdca7d52-5040-49a0-b15c-b3815b4d8962
📒 Files selected for processing (3)
.github/scripts/ci_config.py.github/workflows/build-and-test.yml.github/workflows/rust.yml
1 participant
Move group listing into
build-and-test.ymlso test jobs start immediately instead of waiting forverify-groupsto complete.verify-groupsstill runs in parallel as a validation guard.Summary by CodeRabbit