Skip to content

Fix spurious test-bot missing :all bottle warning#23019

Merged
MikeMcQuaid merged 1 commit into
mainfrom
test-bot-missing-all-bottle
Jul 9, 2026
Merged

Fix spurious test-bot missing :all bottle warning#23019
MikeMcQuaid merged 1 commit into
mainfrom
test-bot-missing-all-bottle

Conversation

@p-linnane

Copy link
Copy Markdown
Member

The test-bot "missing :all bottle" warning misfired on every per-platform job of Homebrew/homebrew-core#291991 (webfont 12.5.0): each job saw only its own platform's bottle JSON and warned, even though the final brew bottle --merge then created a new :all bottle because the generated checksums and cellars matched.

annotate_missing_all_bottle now mirrors the gates brew bottle --merge actually uses to decide all_bottle, so it only warns when the final merge genuinely cannot recreate one. It stays quiet when the formula already has an all: stanza, when a local all bottle JSON is present, when only one platform JSON exists (a single runner cannot know what the multi-platform merge will produce), and when every local tag hash shares one cellar and checksum. A new local_bottle_tag_hashes helper collects the local JSONs and normalizes the top-level "cellar" into each per-tag hash to match the shape the merge builds.

The annotation is informational and never blocks merge, and the authoritative "should have had an :all bottle" check still runs in the final brew bottle --merge, so this only trims false positives from the per-runner heads-up.


  • Have you followed our Contributing guidelines?
  • Have you checked for other open Pull Requests for the same change?
  • Have you explained what your changes do? Performance claims (e.g. "this is faster") must include Hyperfine benchmarks.
  • Have you explained why you'd like these changes included, not just what they do?
  • For bug fixes, have you given step-by-step brew commands to reproduce the bug?
  • Have you written new tests (excluding integration tests)? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) locally?

  • AI was used to generate or assist with generating this PR.

Claude Code (Opus 4.8) developed the fix and tests; I reviewed the diff and verified with brew lgtm plus targeted test_bot/formulae specs.


Signed-off-by: Patrick Linnane <patrick@linnane.io>
Copilot AI review requested due to automatic review settings July 9, 2026 01:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts test-bot’s annotate_missing_all_bottle warning logic to avoid false positives on per-platform CI jobs by aligning the predictor’s gating conditions with the ones used when brew bottle --merge determines whether an :all bottle can be (re)created.

Changes:

  • Update annotate_missing_all_bottle to short-circuit when an all tag is already present locally, when there’s only a single local bottle JSON, or when local tag hashes share the same (cellar, sha256) pair (indicating an :all bottle can be formed).
  • Add local_bottle_tag_hashes to aggregate local bottle JSONs and normalize top-level "cellar" into per-tag hashes.
  • Expand unit tests to cover the new gating behavior (local all present, single-platform JSON, differing cellars, and convergent platform bottles).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
Library/Homebrew/test_bot/formulae.rb Refines the “missing :all bottle” annotation predictor to use merge-equivalent gates and adds a helper to aggregate/normalize local bottle JSON tag data.
Library/Homebrew/test/test_bot/formulae_spec.rb Refactors test setup and adds coverage for the new annotation gating scenarios to prevent per-runner false positives.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@MikeMcQuaid MikeMcQuaid left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fix!

@MikeMcQuaid MikeMcQuaid added this pull request to the merge queue Jul 9, 2026
Merged via the queue into main with commit 53ee764 Jul 9, 2026
43 checks passed
@MikeMcQuaid MikeMcQuaid deleted the test-bot-missing-all-bottle branch July 9, 2026 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants