-
-
Notifications
You must be signed in to change notification settings - Fork 10.9k
test/cmd: use bottle pour for faster test run #21177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
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 optimizes integration test performance by switching from source builds to bottle pours for faster test execution. The change reduces test execution time significantly (e.g., reinstall tests from ~8.5s to ~4.6s) by avoiding build environment setup and additional Ruby process overhead.
Key Changes
- Added automatic bottle block generation for
testball_bottleformula in integration test helper - Converted upgrade, reinstall, and install tests to use
testball_bottleinstead of source-built formulas - Added
HOMEBREW_TEMP_CELLARto test cleanup directories to handle bottle extraction artifacts
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Library/Homebrew/test/support/helper/spec/shared_context/integration_test.rb | Added automatic bottle block generation for testball_bottle formula with SHA256 hash and root URL pointing to test fixtures |
| Library/Homebrew/test/support/fixtures/bottles/testball_bottle-0.1.all.bottle.tar.gz | Added bottle fixture file for cross-platform testing |
| Library/Homebrew/test/spec_helper.rb | Added HOMEBREW_TEMP_CELLAR to TEST_DIRECTORIES for proper cleanup |
| Library/Homebrew/test/cmd/upgrade_spec.rb | Refactored to use testball_bottle instead of testball, updated all references to use consistent formula_name variable |
| Library/Homebrew/test/cmd/reinstall_spec.rb | Refactored to use testball_bottle, added explicit link call after setup, updated path references |
| Library/Homebrew/test/cmd/install_spec.rb | Split into two separate tests: one for bottle installation and one for source installation, with appropriate expectations for each |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, thanks!
3386050 to
59036c1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
59036c1 to
9b2f3bf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work again, thanks!
brew lgtm(style, typechecking and tests) with your changes locally?Bottle pour is faster to test than source-build as it avoids setting up build environment and starting another Ruby process.
Example result for reinstall:
8.56 seconds ./test/cmd/reinstall_spec.rb:104.65 seconds ./test/cmd/reinstall_spec.rb:10cmd/install_spec only partly benefits as it still needs to run source build tests for options, HEAD, etc.