Fan scoped mutation runs out across shards for large windows (#370, #371) - #378
Fan scoped mutation runs out across shards for large windows (#370, #371)#378leynos wants to merge 1 commit into
Conversation
…370) Scheduled (scoped) runs pinned the root target to a single shard regardless of shard-count. When window-hours caught a large batch of changed files that one job outgrew timeout-minutes and was killed, truncating coverage (wildside 308 mutants over 384 min; rstest-bdd cancelled at 42%). Size the scoped root fan-out from the changed-file count: small windows stay single-shard (each shard would re-pay the baseline build for a handful of mutants), while large windows fan out across up to shard-count shards. Every shard carries the same scoped file list and cargo mutants --shard k/N partitions the mutants among them, reusing the existing sharding path with no new caller input. Extra crates stay single-shard, matching full runs. Cover scoped_shard_count and the fan-out matrix with unit tests; update the script, workflow input, and caller-guide docs.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Summary
WalkthroughUpdate scoped mutation detection to fan out large root-file windows across dynamically calculated shards, retain single-shard extra-crate targets, test the boundaries, and document the full versus scoped ChangesScoped mutation sharding
Sequence Diagram(s)sequenceDiagram
participant MutationDetectChanges
participant GitHubActions
participant MutationJobs
MutationDetectChanges->>MutationDetectChanges: Calculate scoped shard count from changed root files
MutationDetectChanges->>GitHubActions: Emit root and extra-crate matrix entries
GitHubActions->>MutationJobs: Start matrix jobs with shard metadata
Possibly related issues
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 16 | ❌ 4❌ Failed checks (3 warnings, 1 inconclusive)
✅ Passed checks (16 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@workflow_scripts/tests/test_mutation_detect_changes.py`:
- Around line 150-209: Add descriptive failure messages to every assertion
introduced in the listed tests: test_scoped_shard_count_thresholds,
test_small_scoped_root_stays_single_shard, test_large_scoped_root_fans_out,
test_large_scoped_root_caps_at_shard_count, and
test_scoped_extra_crate_stays_single_shard_when_root_fans_out. Preserve each
assertion’s existing condition while supplying a useful message that identifies
the expected scoped shard or matrix behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: cd1724dc-d882-483e-9bd7-3d5ba2725eec
📒 Files selected for processing (4)
.github/workflows/mutation-cargo.ymldocs/mutation-cargo-workflow.mdworkflow_scripts/mutation_detect_changes.pyworkflow_scripts/tests/test_mutation_detect_changes.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/polythene(auto-detected)
| @pytest.mark.parametrize( | ||
| ("file_count", "shard_count", "expected"), | ||
| [ | ||
| (0, 6, 1), | ||
| (1, 6, 1), | ||
| (detect.SCOPED_FILES_PER_SHARD, 6, 1), | ||
| (detect.SCOPED_FILES_PER_SHARD + 1, 6, 2), | ||
| (detect.SCOPED_FILES_PER_SHARD * 3, 6, 3), | ||
| (detect.SCOPED_FILES_PER_SHARD * 100, 6, 6), | ||
| (detect.SCOPED_FILES_PER_SHARD * 100, 1, 1), | ||
| ], | ||
| ) | ||
| def test_scoped_shard_count_thresholds( | ||
| self, file_count: int, shard_count: int, expected: int | ||
| ) -> None: | ||
| """Scoped fan-out grows with the batch, capped at ``shard_count``.""" | ||
| assert detect.scoped_shard_count(file_count, shard_count) == expected | ||
|
|
||
| def test_small_scoped_root_stays_single_shard(self) -> None: | ||
| """A handful of changed files runs the root target as one shard.""" | ||
| config = _config(shard_count=6) | ||
| files = [f"src/f{i}.rs" for i in range(detect.SCOPED_FILES_PER_SHARD)] | ||
| entries = detect.scoped_run_matrix({".": files}, config) | ||
| assert len(entries) == 1 | ||
| assert entries[0].shard == 0 | ||
| assert entries[0].shard_count == 1 | ||
|
|
||
| def test_large_scoped_root_fans_out(self) -> None: | ||
| """A large root batch fans out across shards sharing the file list.""" | ||
| config = _config(shard_count=6) | ||
| files = [f"src/f{i}.rs" for i in range(detect.SCOPED_FILES_PER_SHARD * 3)] | ||
| entries = detect.scoped_run_matrix({".": files}, config) | ||
| assert len(entries) == 3 | ||
| assert [e.shard for e in entries] == [0, 1, 2] | ||
| assert all(e.shard_count == 3 for e in entries) | ||
| # Every shard carries the same scoped file list; cargo-mutants' | ||
| # --shard k/N partitions the mutant set among them. | ||
| assert all(e.files == " ".join(files) for e in entries) | ||
| assert all(e.slug == "root" for e in entries) | ||
|
|
||
| def test_large_scoped_root_caps_at_shard_count(self) -> None: | ||
| """Fan-out never exceeds the configured shard-count ceiling.""" | ||
| config = _config(shard_count=4) | ||
| files = [f"src/f{i}.rs" for i in range(detect.SCOPED_FILES_PER_SHARD * 20)] | ||
| entries = detect.scoped_run_matrix({".": files}, config) | ||
| assert len(entries) == 4 | ||
| assert all(e.shard_count == 4 for e in entries) | ||
|
|
||
| def test_scoped_extra_crate_stays_single_shard_when_root_fans_out(self) -> None: | ||
| """Only the root target fans out; extra crates stay single-shard.""" | ||
| config = _config(shard_count=6, extra_crate_dirs=("testkit",)) | ||
| root_files = [f"src/f{i}.rs" for i in range(detect.SCOPED_FILES_PER_SHARD * 2)] | ||
| buckets = {".": root_files, "testkit": ["testkit/src/lib.rs"]} | ||
| entries = detect.scoped_run_matrix(buckets, config) | ||
| root = [e for e in entries if e.dir == "."] | ||
| extra = [e for e in entries if e.dir == "testkit"] | ||
| assert len(root) == 2 | ||
| assert len(extra) == 1 | ||
| assert extra[0].shard == 0 | ||
| assert extra[0].shard_count == 1 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add failure messages to the new bare asserts.
Every new assertion here (test_scoped_shard_count_thresholds, test_small_scoped_root_stays_single_shard, test_large_scoped_root_fans_out, test_large_scoped_root_caps_at_shard_count, test_scoped_extra_crate_stays_single_shard_when_root_fans_out) omits a failure message. As per path instructions, **/*.py files must "Use assert …, "message" over bare asserts".
🧪 Example fix for one case (apply the same pattern to the rest)
- assert detect.scoped_shard_count(file_count, shard_count) == expected
+ assert (
+ detect.scoped_shard_count(file_count, shard_count) == expected
+ ), f"expected {expected} shards for {file_count} files/{shard_count} cap"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workflow_scripts/tests/test_mutation_detect_changes.py` around lines 150 -
209, Add descriptive failure messages to every assertion introduced in the
listed tests: test_scoped_shard_count_thresholds,
test_small_scoped_root_stays_single_shard, test_large_scoped_root_fans_out,
test_large_scoped_root_caps_at_shard_count, and
test_scoped_extra_crate_stays_single_shard_when_root_fans_out. Preserve each
assertion’s existing condition while supplying a useful message that identifies
the expected scoped shard or matrix behavior.
Source: Path instructions
Summary
Scheduled (scoped) mutation runs pinned the root target to a single shard
regardless of
shard-count. When thewindow-hourschange-detection windowcaught a large batch of changed files, that one job outgrew
timeout-minutesand was killed, silently truncating coverage — wildside ran 308 mutants for
384 minutes before the 300-minute step timeout (#370); rstest-bdd was
cancelled at 42% of 661 mutants (#371).
This sizes the scoped root fan-out from the changed-file count:
build-and-test cost, not worth it for a handful of mutants.
shard-countshards.Every shard carries the same scoped
--filelist andcargo mutants --shard k/Npartitions the mutant set among them, reusing the existingsharding path.
The fan-out reuses the existing
shard-countinput (default 6) as an upperbound, so callers benefit with no new input and no configuration change. Extra
crates stay single-shard, matching
full_run_matrix. The threshold(
SCOPED_FILES_PER_SHARD = 15) is sized conservatively so a shard's changedfiles stay well under the ~200-250-mutant budget observed at the 300-minute
ceiling (~60-65 s/mutant).
Closes #370. Closes #371.
Review walkthrough
workflow_scripts/mutation_detect_changes.py— newscoped_shard_counthelper and
SCOPED_FILES_PER_SHARD;scoped_run_matrixfans the roottarget out across shards for large batches; docstrings updated.
.github/workflows/mutation-cargo.yml—shard-countinput descriptionupdated to describe scoped fan-out.
docs/mutation-cargo-workflow.md— behaviour and input table updated.workflow_scripts/tests/test_mutation_detect_changes.py— unit tests forscoped_shard_countthresholds, single-shard small batches, fan-out,the
shard-countcap, and extra crates staying single-shard.Validation
pytest workflow_scripts/tests/test_mutation_detect_changes.py— 25 passed.pytest workflow_scripts/tests/test_mutation_properties.py— 6 passed.ruff format --check/ruff check— clean;markdownlint/typoson thechanged doc — clean.