test-bot: shard dependent testing across CI matrix runners#21593
Draft
GunniBusch wants to merge 1 commit intoHomebrew:mainfrom
Draft
test-bot: shard dependent testing across CI matrix runners#21593GunniBusch wants to merge 1 commit intoHomebrew:mainfrom
GunniBusch wants to merge 1 commit intoHomebrew:mainfrom
Conversation
826e7e2 to
c213d65
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds deterministic sharding for brew test-bot’s “formulae dependents” testing and updates the GitHub Actions runner matrix generation so dependent testing can be split across multiple CI jobs per runner.
Changes:
- Introduces
DependentShardAssigner(deterministic assignment) and applies it inFormulaeDependentsto shard the final runnable dependent set. - Adds
DependentShardMatrixand wires dependent shard metadata (dependent_shard_count,dependent_shard_index) intoGitHubRunnerMatrixrows, with sizing controls exposed viadetermine-test-runners. - Adds CLI validations + specs, and updates CI workflow steps to avoid restoring a cached prefix in jobs where it can overwrite the PR checkout.
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Library/Homebrew/test_bot/formulae_dependents.rb | Builds dependency “feature” sets and filters dependents based on the shard assignment. |
| Library/Homebrew/test_bot/dependent_shard_assigner.rb | Implements deterministic dependent→shard assignment with locality/balance heuristics. |
| Library/Homebrew/test/test_bot/formulae_dependents_spec.rb | Specs for FormulaeDependents sharding behavior (determinism/disjointness/etc.). |
| Library/Homebrew/test/test_bot/dependent_shard_assigner_spec.rb | Specs covering assigner determinism, locality, balance, and argument validation. |
| Library/Homebrew/test/github_runner_matrix_spec.rb | Specs for dependent matrix row expansion/clamping and non-dependent behavior. |
| Library/Homebrew/test/dev-cmd/test-bot_spec.rb | Integration specs for test-bot dependent shard flag validation. |
| Library/Homebrew/test/dev-cmd/determine-test-runners_spec.rb | Integration specs for determine-test-runners new shard sizing flags. |
| Library/Homebrew/test/dependent_shard_matrix_spec.rb | Unit specs for shard row expansion and validation in DependentShardMatrix. |
| Library/Homebrew/sorbet/rbi/dsl/homebrew/dev_cmd/determine_test_runners.rbi | Adds RBI accessors for new determine-test-runners args. |
| Library/Homebrew/sorbet/rbi/dsl/homebrew/cmd/test_bot_cmd.rbi | Adds RBI accessors for new test-bot shard args. |
| Library/Homebrew/github_runner_matrix.rb | Adds dependent shard defaults, computes per-runner dependent counts, and expands matrix rows via DependentShardMatrix. |
| Library/Homebrew/dev-cmd/test-bot.rb | Adds dependent sharding flags and validates shard argument combinations/ranges. |
| Library/Homebrew/dev-cmd/determine-test-runners.rb | Adds shard sizing flags and parsing/validation; passes values into GitHubRunnerMatrix. |
| Library/Homebrew/dependent_shard_matrix.rb | New helper to expand dependent runner spec rows by computed shard count per runner. |
| .github/workflows/tests.yml | Avoids restoring cached Homebrew prefix in jobs where it can override the PR checkout; installs tools directly. |
Files not reviewed (2)
- Library/Homebrew/sorbet/rbi/dsl/homebrew/cmd/test_bot_cmd.rbi: Language not supported
- Library/Homebrew/sorbet/rbi/dsl/homebrew/dev_cmd/determine_test_runners.rbi: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Library/Homebrew/test/test_bot/dependent_shard_assigner_spec.rb
Outdated
Show resolved
Hide resolved
2c44a3a to
5f46806
Compare
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
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.
brew lgtm(style, typechecking and tests) with your changes locally?This implements a way to parallelise dependency testing using shards. This was inspired by how bazel does it. This implements the idea of Homebrew/homebrew-core#206400. But it does not resolve this issue because my feature is not the default behaviour meaning, one merged (or working), I will open a pr in core.
Meanwhile during developing, I made a testing repo to test if this is working.
The latest run is: https://github.com/GunniBusch/brew-dependent-shard-e2e/actions/runs/22170548663
Summary
This PR adds deterministic dependent-test sharding to
brew test-bot, with dynamic shard sizing indetermine-test-runners, and wires shard metadata into runner matrix rows.What changed
brew determine-test-runnersflags:--dependent-shard-max-runners=<n>--dependent-shard-min-dependents-per-runner=<n>brew test-botflags:--dependent-shard-count=<m>--dependent-shard-index=<n>DependentShardMatrixto expand dependent matrix rows per active runner and compute shard counts from dependent cardinality.GitHubRunnerMatrixto delegate dependent row expansion and include:dependent_shard_countdependent_shard_indexDependentShardAssignerfor deterministic shard assignment in dependent testing.FormulaeDependentsto shard the final runnable dependent set (after existing filtering/defer logic) via the assigner.tests.yml:syntaxjob now installsshellcheck/shfmtdirectly instead of restoring cached Homebrew prefix, to avoid stale repo content overriding PR checkout.Behavioral notes
4), behavior is changed but not breakingTest coverage