feat(test): support sharding tests across CI runners by peter-trost · Pull Request #1707 · VeryGoodOpenSource/very_good_cli

peter-trost · 2026-08-28T15:32:13Z

Description

Adds --shard-index and --total-shards to very_good test and very_good dart test, so a test suite can be split across multiple CI runners with a strategy.matrix.

strategy:
  matrix:
    shard: [1, 2, 3]
steps:
  - run: very_good test --shard-index ${{ matrix.shard }} --total-shards 3

Opening as a draft because @ryzizub is assigned to #1538 — happy to hand this over, close it, or adapt to a different design if work is already underway or a different approach is preferred.

Why this approach

The test optimizer already discovers every test file, so sharding is a partition of that list rather than new machinery.

Sorted, then round-robin. Directory.listSync order is filesystem dependent, so without sorting two runners could disagree on the partition and either skip or duplicate tests. Sorting first makes the partition deterministic across machines; dealing out round-robin keeps shards balanced by file count.
Non-optimized tests are sharded too. Files tagged skip_very_good_optimization run as standalone files alongside the optimizer entrypoint. Leaving them unsharded would re-run all of them on every runner, partly defeating the purpose.
No shard-specific filenames. The issue proposed generating .test_optimizer_1_of_3.dart. That turned out to be unnecessary — each runner generates its own .test_optimizer.dart containing only its slice, and the file is cleaned up afterwards anyway. Same result, no filename plumbing.
Empty shards succeed. A shard with no test files (more shards than test files) would otherwise fail with No tests were found, breaking builds on oversized matrices. It now reports success, matching the existing "no test folder" behaviour.

Interaction with coverage

--min-coverage is rejected with a usage error when sharding. Each shard only exercises a subset of the codebase, so its coverage is not representative of the whole suite and would fail the build spuriously. The error explains the alternative: collect per-shard coverage with --coverage, merge the lcov reports, and enforce the threshold once in a separate job.

Sharding also requires the optimizer, so it is rejected with --no-optimization and with --platform (which disables the optimizer). Both cases exit with ExitCode.usage and an actionable message.

This is the part I'd most like a maintainer opinion on — a documented merge-then-check workflow is the other reasonable option, and #804 may change what's possible here.

Testing

Hook unit tests (11 new): complete/disjoint partition, non-optimized sharding, determinism, balance, empty shards, and the pass-through case.
Command tests (15 new): all validation paths and shard pass-through, for both test and dart test.
Runner tests (2 new): shard vars reach the generator; empty shard short-circuits without invoking the test runner.
Manual end-to-end against a real package (7 optimizable + 2 tagged test files): verified all tests run exactly once across 3 and 12 shards, that shards are stable across repeated runs, and that the Flutter path shards correctly. Also exhaustively verified the partition is complete, disjoint, and balanced for suite sizes 0–40 across 1–8 shards.

dart analyze --fatal-infos --fatal-warnings . is clean, and the touched lib/ files are at 100% line coverage.

Note

The full suite passes with -j 1 (529 tests). At -j 8 a handful of unrelated tests fail due to a pre-existing working-directory race between test files — I confirmed the same failures on a clean main checkout, so it is not introduced here.

Closes #1538

🤖 Generated with Claude Code

Add `--shard-index` and `--total-shards` to `very_good test` and `very_good dart test`, so a test suite can be split across multiple CI runners with a `strategy.matrix`. The test optimizer already discovers every test file, so sharding is a partition of that list rather than new machinery. Files are sorted and dealt out round-robin, which keeps shards balanced by file count and makes the partition deterministic across machines — `Directory.listSync` order is filesystem dependent, so without sorting two runners could disagree and either skip or duplicate tests. Tests tagged `skip_very_good_optimization` are sharded as well. They run as standalone files alongside the optimizer entrypoint, so leaving them unsharded would re-run all of them on every runner. Each runner generates its own `.test_optimizer.dart` containing only its slice, so no shard-specific filenames are needed. A shard with no test files succeeds instead of failing with "No tests were found", so an oversized matrix does not break the build. Sharding is rejected with a usage error when combined with `--min-coverage`, since each shard only exercises a subset of the codebase and its coverage is not representative of the whole suite, and when the optimizer is disabled, which sharding depends on. Closes VeryGoodOpenSource#1538 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Normalize path separators in getNotOptimizedTests so tagged tests in subdirectories are excluded from the optimized set on Windows. Without this the relative paths compare unequal to the forward-slash normalized paths built in run(), causing those tests to be both inlined into the optimizer entrypoint and run standalone. Parse the shard values inside validateSharding instead of passing both the raw and parsed forms, and shorten the --min-coverage error. Also cover the shard that only contains non optimized tests, and document how sharding interacts with --recursive. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

ryzizub · 2026-08-31T07:54:27Z

Thanks for the contribution @peter-trost ! ❤️ You can undraft it, since I actually havent started much on the sharding tests and this actually on first glance does it how i would do it.

I will give it a test and bigger review later today/tomorrow but so far it looks good to me

peter-trost · 2026-08-31T08:01:47Z

@ryzizub nice, glad to hear! Let me know if I can support in any way :)

peter-trost · 2026-08-31T10:18:29Z

the flame e2e failing seems unrelated 🤔

ryzizub · 2026-08-31T11:32:40Z

the flame e2e failing seems unrelated 🤔

Correct, there seems to be issue unrelated to this PR

Sharding the optimized and skip_very_good_optimization lists separately restarted the round-robin at shard 1 for each list, so the first shards received a file from both while later shards could stay empty. Deal out the single sorted list once and split it afterwards, which also drops one shardOf call. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

peter-trost · 2026-09-02T19:37:53Z

a505315 adds a fix to put optimized and skip_very_good_optimization files into one sorted list instead of two. Sharding them separately restarted at shard 1 for each list, so early shards got a file from both while later ones sat empty.

The bundle also embeds the hook test file, which gained a case in the previous commit after the bundle had last been generated, so the verify_bundle check failed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

ryzizub

Overall LGTM, great job! Just a small findings

Route the raw --shard-index and --total-shards values through the test options classes instead of reading argResults in run(), make shardOf private now that the sharding behaviour is covered through run(), and name the flag values (<index>, <count>) while stating the default in their help text. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

peter-trost · 2026-09-03T09:32:31Z

Thanks for the review @ryzizub! Latest commit addresses the feedback.

# Conflicts: # lib/src/cli/dart_cli.dart # lib/src/cli/flutter_cli.dart # lib/src/cli/templates/test_optimizer_bundle.dart # lib/src/cli/test_cli_runner.dart # test/src/commands/dart/commands/dart_test_test.dart Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

peter-trost · 2026-09-07T19:29:31Z

@ryzizub merge conflicts fixed.

ryzizub · 2026-09-08T13:25:50Z

Adding @marcossevilla as additional reviewer for this PR

Validate sharding against the effective optimizer setting, so --platform, --update-goldens and explicit test paths are rejected instead of silently running the whole suite on every shard. Reject only an explicit --min-coverage and ignore a threshold inherited from very_good.yaml while sharding, since a single shard cannot meet it. Write an empty lcov.info for an empty shard so the merge step still finds a report per shard. Guard the hook against out of range shard values reached through `mason make`, look non optimized tests up in a set, keep the emitted test paths native, and move validateSharding next to TestCLIRunner so the dart command no longer depends on the flutter one. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

peter-trost · 2026-09-08T19:47:57Z

Thanks for the thorough review @marcossevilla. Addressed all comments. 🙂

# Conflicts: # lib/src/cli/dart_cli.dart # lib/src/cli/test_cli_runner.dart # lib/src/commands/dart/commands/dart_test_command.dart # lib/src/commands/test/test.dart # test/src/commands/dart/commands/dart_test_test.dart # test/src/commands/test/test_test.dart Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

peter-trost · 2026-09-16T18:10:13Z

@marcossevilla resolved conflicts. Ready for your re-review :)

marcossevilla · 2026-09-17T09:49:27Z

@peter-trost holding a bit on reviewing this since we had large changes for how optimization is handled inside the CLI, we addressed the issue on preserving test annotations and that should be the last.

sorry to cause so many changes on this PR, please lmk when the conflicts are solved and I can give a fresh review 😄

# Conflicts: # bricks/test_optimizer/hooks/lib/pre_gen.dart # lib/src/cli/templates/test_optimizer_bundle.dart Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

peter-trost · 2026-09-17T12:17:49Z

@marcossevilla no worries! :D The changes are integrated, please have another look :)

TestOptimization.testTargets joins the hook's POSIX paths natively, so on Windows the expected value is test\integration\login_test.dart rather than the mixed test\integration/login_test.dart these assertions built with p.join. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

peter-trost · 2026-09-17T15:00:50Z

@marcossevilla attempted a fix for the failing workflow on windows. Should pass now. 🤞

peter-trost and others added 2 commits August 28, 2026 17:31

ryzizub assigned peter-trost and ryzizub Aug 31, 2026

peter-trost marked this pull request as ready for review August 31, 2026 08:00

peter-trost requested a review from a team as a code owner August 31, 2026 08:00

peter-trost and others added 2 commits September 2, 2026 21:39

Merge branch 'main' into feat/test-sharding

f5b39fc

chore(test): regenerate test optimizer bundle

26357ed

The bundle also embeds the hook test file, which gained a case in the previous commit after the bundle had last been generated, so the verify_bundle check failed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

ryzizub reviewed Sep 3, 2026

View reviewed changes

ryzizub requested a review from marcossevilla September 8, 2026 13:25

marcossevilla reviewed Sep 8, 2026

View reviewed changes

marcossevilla and others added 2 commits September 8, 2026 16:14

Merge branch 'main' into feat/test-sharding

1c52e95

peter-trost requested a review from marcossevilla September 10, 2026 11:36

Merge remote-tracking branch 'origin/main' into pr-1707

9752788

# Conflicts: # bricks/test_optimizer/hooks/lib/pre_gen.dart # lib/src/cli/templates/test_optimizer_bundle.dart Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Conversation

peter-trost commented Aug 28, 2026

Description

Why this approach

Interaction with coverage

Testing

Uh oh!

ryzizub commented Aug 31, 2026

Uh oh!

peter-trost commented Aug 31, 2026

Uh oh!

peter-trost commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryzizub commented Aug 31, 2026

Uh oh!

peter-trost commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryzizub left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

peter-trost commented Sep 3, 2026

Uh oh!

peter-trost commented Sep 7, 2026

Uh oh!

ryzizub commented Sep 8, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

peter-trost commented Sep 8, 2026

Uh oh!

peter-trost commented Sep 16, 2026

Uh oh!

marcossevilla commented Sep 17, 2026

Uh oh!

peter-trost commented Sep 17, 2026

Uh oh!

peter-trost commented Sep 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

peter-trost commented Aug 31, 2026 •

edited

Loading

peter-trost commented Sep 2, 2026 •

edited

Loading