sorbet: Handle before blocks using let values from within tests#22480
Merged
issyl0 merged 1 commit intoMay 31, 2026
Merged
Conversation
f1624e2 to
ce6866a
Compare
MikeMcQuaid
reviewed
May 31, 2026
Member
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Thanks, looks good when 🟢!
Code Coverage OverviewLanguages: Ruby Ruby / code-coverage/simplecovThe overall coverage in the branch is 75%. The coverage in the branch is 78%. Show a code coverage summary of the most impacted files.
Updated |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to improve Sorbet typechecking for RSpec tests where before/subject blocks reference values that are only defined via let in nested examples (which Sorbet can’t always resolve statically).
Changes:
- Switch a few test files from
# typed: falseto# typed: true. - Add top-level placeholder
letvalues (e.g.,who_output,string,required_ruby_version,languages) so Sorbet can resolve references frombefore/subject. - Add
old_tap/new_tapandtapletvalues in some specs to make referenced methods visible to Sorbet.
Show a summary per file
| File | Description |
|---|---|
| Library/Homebrew/test/utils/user_spec.rb | Enables typechecking (typed: true) and adds a placeholder let(:who_output) referenced by a before stub. |
| Library/Homebrew/test/utils/string_inreplace_extension_spec.rb | Enables typechecking (typed: true) and adds a placeholder let(:string) used by subject. |
| Library/Homebrew/test/utils/ruby_check_version_script_spec.rb | Enables typechecking (typed: true) and adds a placeholder let(:required_ruby_version) used by subject. |
| Library/Homebrew/test/formulary_spec.rb | Adds top-level let(:old_tap)/let(:new_tap) for tap-migration setup used in an outer before. |
| Library/Homebrew/test/cmd/untap_spec.rb | Adds let(:tap) inside shared examples for installed formula/cask tests. |
| Library/Homebrew/test/cask/dsl_spec.rb | Adds a placeholder let(:languages) referenced by a before that configures cask.config.languages. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 6/6 changed files
- Comments generated: 2
- Enable typechecking of tests where a top-level `before` or `subject` block references a value that is only defined via `let` in tests, which Sorbet cannot resolve statically. - The actual value of the `let` that matters for the test and its setup is set in the individual tests, which will always override the top-level `let` value that we set.
ce6866a to
8862004
Compare
MikeMcQuaid
approved these changes
May 31, 2026
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?OpenAI GPT-5.3 Codex for finding all occurrences of this pattern that upset Sorbet, with local review and testing.
beforeorsubjectblock references a value that is only defined vialetin tests, which Sorbet cannot resolve statically.letthat matters for the test and its setup is set in the individual tests, which will always override the top-levelletvalue that we set.