Skip to content

Use test_each for table-driven specs, and update spoom to 1.8.6 - #23359

Merged
MikeMcQuaid merged 2 commits into
mainfrom
dug/spoom-1.8.6-test-each
Jul 30, 2026
Merged

Use test_each for table-driven specs, and update spoom to 1.8.6#23359
MikeMcQuaid merged 2 commits into
mainfrom
dug/spoom-1.8.6-test-each

Conversation

@dduugg

@dduugg dduugg commented Jul 30, 2026

Copy link
Copy Markdown
Member

Two commits, reviewable separately.

1. vendor-gems: update spoom to 1.8.6

spoom 1.8.4 → 1.8.6, plus rbi 0.4.0 → 0.4.1 which spoom 1.8.6 requires, and the regenerated RBIs for both.

sorbet is deliberately held at 0.6.13342. A plain brew vendor-gems --update=spoom also sweeps sorbet to 0.6.13371, but spoom only needs sorbet-static-and-runtime >= 0.5.10187, and bumping it rewrites the vendored sorbet-runtime copy that is committed in-tree (~13k lines). That belongs in its own PR.

2. test: use test_each for table-driven specs

Sorbet only looks for spec methods at the top level of a class body or describe block, so example groups generated inside a plain each are invisible to it: their bodies typecheck against the group class rather than an example, and every call in the loop fails to resolve. One loop cascades into an error per call, which is usually enough to pin the file at # typed: false.

This adds Test::Helper::TestEach and applies Shopify/rubocop-sorbet#391's new Sorbet/TestsDefinedWithEach cop, which corrected 22 loops across 16 files. The rewrites are semantics-preserving: each already yielded one row per iteration, so multi-parameter blocks were auto-splatting what is now an explicit destructure.

Six files come off # typed: false as a result, three of them all the way to # typed: strict:

file before after
test/formula_creator_spec.rb false strict
test/rubocops/presence_spec.rb false strict
test/vulns/cvss_spec.rb false strict
test/download_strategies/curl_spec.rb false true
test/mcp_server_spec.rb false true
test/sandbox_shared_spec.rb false true

Why both sigs are checked(:never)

A Hash reaches test_each rather than test_each_hash whenever the receiver is not a hash literal — a constant, a local, a shared-example parameter. Under HOMEBREW_SORBET_RECURSIVE, TypedEnumerable#valid? becomes recursively_valid? (standalone/sorbet.rb:33), which accepts a Hash only when the element type is a tuple. Plain array rows are not tuples, so no single element type satisfies both, and widening it does not help: only a literal tuple passes the Hash branch, so a T.any(...) element type fails the is_a?(FixedArray) guard.

A top-level union does not work either. T.any(T::Enumerable[T.type_parameter(:U)], T::Hash[T.anything, T.anything]) typechecks clean at the definition but leaves U unbound at hash call sites, which turns the corrected blocks into dead code:

test/mcp_server_spec.rb:133: This code is unreachable
test/sandbox_shared_spec.rb:267: This code is unreachable
test/sandbox_shared_spec.rb:285: This code is unreachable
test/support/helper/test_each.rb:18: Expected `T.proc.params(arg0: T.any([T.anything, T.anything], T.type_parameter(:U)))` ... https://srb.help/7002

Since these methods exist only to give Sorbet a static view of the loop, validating the pass-through at runtime buys nothing.

Verification

brew typecheck and brew style --changed are clean, and the 47 spec files covering all 16 changed files pass. Note that a repro outside the suite is misleading here: brew ruby disables runtime checks entirely, so the Hash failure only surfaces under brew tests, where rspec-sorbet reinstates a raising call_validation_error_handler.


  • 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?

  • I did not use AI/LLM to create this PR, or I disclosed the tool/model below and reviewed its output; I did not attribute commits to AI and will answer maintainer questions and review comments myself without AI/LLM.

AI/LLM disclosure: prepared with Claude Code (Opus 5) — it ran the gem update, ran the
new cop's autocorrection, and measured the sigil promotions with spoom srb bump.


dduugg added 2 commits July 29, 2026 21:52
Also bumps `rbi` to 0.4.1, which spoom 1.8.6 requires, and regenerates the RBI
files for both.

`sorbet` is held at 0.6.13342: spoom only needs `>= 0.5.10187`, and bumping it
would rewrite the vendored `sorbet-runtime` copy that is committed in-tree.
Sorbet only looks for spec methods at the top level of a class body or
`describe` block, so example groups generated inside a plain `each` are
invisible to it: their bodies typecheck against the group class rather than an
example, and every call in the loop fails to resolve. One loop cascades into an
error per call, which is usually enough to pin the file at `# typed: false`.

Adds `Test::Helper::TestEach`, which Sorbet can see through, and applies the new
`Sorbet/TestsDefinedWithEach` cop from rubocop-sorbet to the 22 loops that
qualify. The rewrites are semantics-preserving: `each` already yielded one row
per iteration, so multi-parameter blocks were auto-splatting what is now an
explicit destructure.

Six files come off `# typed: false` as a result, three of them all the way to
`# typed: strict`.

Both sigs are `checked(:never)`. A `Hash` reaches `test_each` rather than
`test_each_hash` whenever the receiver is not a hash literal, and under
`HOMEBREW_SORBET_RECURSIVE` a `Hash` only validates when the element type is a
tuple, which plain array rows are not. No element type satisfies both, and a
top-level union leaves the block parameter type unbound.
@dduugg
dduugg force-pushed the dug/spoom-1.8.6-test-each branch from 4a8c5a0 to d65b9c0 Compare July 30, 2026 04:55
@dduugg dduugg changed the title Use test_each for table-driven specs (and update spoom to 1.8.6) Use test_each for table-driven specs, and update spoom to 1.8.6 Jul 30, 2026
@dduugg
dduugg marked this pull request as ready for review July 30, 2026 04:57
Copilot AI review requested due to automatic review settings July 30, 2026 04:57

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 updates vendored Sorbet tooling (spoom/rbi) and introduces an RSpec helper (test_each/test_each_hash) to replace table-driven specs written with plain .each, improving Sorbet’s ability to typecheck spec DSL inside those generated example groups.

Changes:

  • Bump vendored gems: spoom 1.8.4 → 1.8.6 and rbi 0.4.0 → 0.4.1, including updated load paths and regenerated gem RBIs.
  • Add Test::Helper::TestEach and wire it into the test harness (RSpec config + RSpec RBI shim) so Sorbet can typecheck table-driven spec blocks.
  • Rewrite multiple specs to use test_each/test_each_hash, enabling stricter Sorbet sigils in several test files.

Reviewed changes

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

Show a summary per file
File Description
Library/Homebrew/vendor/bundle/bundler/setup.rb Updates vendored gem load paths for rbi and spoom new versions.
Library/Homebrew/Gemfile.lock Records spoom/rbi version bumps and dependency constraint change.
Library/Homebrew/sorbet/rbi/gems/spoom@1.8.6.rbi Regenerated RBI for spoom 1.8.6.
Library/Homebrew/sorbet/rbi/gems/rbi@0.4.1.rbi Regenerated RBI for rbi 0.4.1.
Library/Homebrew/test/support/helper/test_each.rb Adds the test_each/test_each_hash helper methods for table-driven specs.
Library/Homebrew/test/spec_helper.rb Requires the new helper and extends it into RSpec example groups.
Library/Homebrew/sorbet/rbi/shims/rspec.rbi Exposes test_each methods to Sorbet’s RSpec example-group type.
Library/Homebrew/test/vulns/cvss_spec.rb Switches hash-table spec loop to test_each_hash and tightens sigil to strict.
Library/Homebrew/test/utils/linkage_spec.rb Replaces OS-metadata .each loop with test_each.
Library/Homebrew/test/support/helper/spec/shared_examples/formulae_exist.rb Replaces .each loop with test_each in shared examples.
Library/Homebrew/test/sandbox_shared_spec.rb Replaces multiple .each loops with test_each and updates destructuring; sigil to true.
Library/Homebrew/test/rubocops/presence_spec.rb Replaces operator .each loop with test_each; sigil to strict.
Library/Homebrew/test/mcp_server_spec.rb Replaces .each over tools with test_each; sigil to true.
Library/Homebrew/test/livecheck_spec.rb Replaces metadata hash .each loops with test_each_hash.
Library/Homebrew/test/formula_spec.rb Replaces actions .each loop with test_each.
Library/Homebrew/test/formula_creator_spec.rb Replaces .each loop with test_each + explicit destructure; sigil to strict.
Library/Homebrew/test/download_strategies/curl_spec.rb Replaces invalid header value .each loop with test_each; sigil to true.
Library/Homebrew/test/cmd/link_spec.rb Replaces formula-type hash .each loop with test_each_hash.
Library/Homebrew/test/cask/dsl/version_spec.rb Replaces multiple .each loops with test_each and updates destructuring.
Library/Homebrew/test/cask/cask_spec.rb Replaces shared-example hash .each loops with test_each and updates destructuring.
Library/Homebrew/test/cask/audit_spec.rb Replaces stanza .each loop with test_each.
Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb Replaces .each loop with test_each.
Library/Homebrew/test/cask/artifact/abstract_uninstall_spec.rb Replaces array .each loop with test_each + explicit destructure.
Files not reviewed (1)
  • Library/Homebrew/sorbet/rbi/shims/rspec.rbi: File type not supported

💡 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.

Nice, good catch!

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