Skip to content

Commit 79ea342

Browse files
committed
Speed up slow specs
- Reduce slow command specs because repeated `brew` subprocesses were dominating the profile more than the tested Homebrew behaviour. - Preserve coverage by moving option and error branches into direct command-object specs where a full process adds no useful signal. - Keep shell-specific coverage by folding Bash command checks into one subprocess harness for each affected Bash path. - Combine formula and cask happy paths in `fetch`, `outdated` and `upgrade`, so one process still checks both package types. - Keep `cmd/install` source, bottle, keg-only and `--HEAD` coverage, with `--HEAD` separate because the option applies to the whole call. - Stub expensive `svn`, DMG mount and signal-wait paths so the specs cover Homebrew decisions without invoking slow external work. - Document the integration-test limit in `AGENTS.md` so future command specs bias towards fast in-process coverage.
1 parent abfb9d0 commit 79ea342

38 files changed

Lines changed: 768 additions & 550 deletions

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ When running Ruby directly (e.g. `ruby -e ...`, `gem`, profiling tools), never u
2727
### Development Flow
2828

2929
- Write new code (using Sorbet `sig` type signatures and `typed: strict` for new files).
30-
- Write new tests (avoid more than one `:integration_test` per command for speed; add another only for essential core functionality in essential non-developer commands). Try `typed: true` as a baseline but revert to `typed: false` if there are not easily fixable errors.
30+
- Write new tests (use at most one `:integration_test` per command, make it a happy-path test and keep it as fast as possible; add another only for essential core functionality in essential non-developer commands). Try `typed: true` as a baseline but revert to `typed: false` if there are not easily fixable errors.
3131
Write fast tests by preferring a single `expect` per unit test and combine expectations in a single test when it is an integration test or has non-trivial `before` for test setup.
3232
- When adding or tightening tests, verify them with a red/green cycle using the exact `--only=file:line` target for the example you changed.
3333
- Formula classes created in specs may be frozen; avoid stubbing class methods on them with RSpec mocks and prefer instance-level stubs or test setup that does not require class-method stubbing.

Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,7 @@
290290
it "is supported" do
291291
allow(subject).to receive(:running_processes).with(bundle_id)
292292
.and_return(unix_pids.map { |pid| [pid, 0, bundle_id] })
293+
allow(subject).to receive(:sleep).with(3)
293294

294295
signals.each do |signal|
295296
expect(Process).to receive(:kill).with(signal, *unix_pids).and_return(1)

Library/Homebrew/test/cask/installer_spec.rb

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,14 @@
44
RSpec.describe Cask::Installer, :cask do
55
let(:klass) { Cask::Installer }
66

7+
def stub_dmg_extraction
8+
allow(UnpackStrategy::Dmg).to receive(:can_extract?).and_return(true)
9+
allow_any_instance_of(UnpackStrategy::Dmg).to receive(:extract_nestedly) do |_strategy, to:, **|
10+
to.mkpath
11+
yield to
12+
end
13+
end
14+
715
describe "install" do
816
it "downloads and installs a nice fresh Cask" do
917
caffeine = Cask::CaskLoader.load(cask_path("local-caffeine"))
@@ -16,6 +24,7 @@
1624

1725
it "works with HFS+ dmg-based Casks" do
1826
asset = Cask::CaskLoader.load(cask_path("container-dmg"))
27+
stub_dmg_extraction { |path| FileUtils.touch path/"container" }
1928

2029
klass.new(asset).install
2130

@@ -165,6 +174,7 @@
165174

166175
it "installs a cask from a dmg file" do
167176
transmission = Cask::CaskLoader.load(cask_path("local-transmission"))
177+
stub_dmg_extraction { |path| (path/"Transmission.app").mkpath }
168178

169179
expect(transmission).not_to be_installed
170180

Library/Homebrew/test/cask/upgrade_spec.rb

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -615,19 +615,23 @@ def write_info_plist(path, short_version:, bundle_version:)
615615
end
616616

617617
context "when there were multiple failures" do
618-
# These tests perform actual upgrades and test error handling,
619-
# so they need full real installations.
618+
# This test exercises upgrade error handling, so it needs installed Casks.
620619
before do
621620
[
622621
"outdated/bad-checksum",
623622
"outdated/local-transmission-zip",
624623
"outdated/bad-checksum2",
625624
].each do |cask|
626-
Cask::Installer.new(Cask::CaskLoader.load(cask_path(cask))).install
625+
InstallHelper.stub_cask_installation(Cask::CaskLoader.load(cask_path(cask)))
627626
end
627+
628+
bad_checksum_2_path = Cask::CaskLoader.load("bad-checksum2").config.appdir.join("container")
629+
FileUtils.rm_rf(bad_checksum_2_path)
630+
FileUtils.touch(bad_checksum_2_path)
628631
end
629632

630633
it "does not end the upgrade process" do
634+
upgraded_tokens = []
631635
bad_checksum = Cask::CaskLoader.load("bad-checksum")
632636
bad_checksum_path = bad_checksum.config.appdir.join("Caffeine.app")
633637

@@ -646,10 +650,19 @@ def write_info_plist(path, short_version:, bundle_version:)
646650
expect(bad_checksum_2_path).to be_a_file
647651
expect(bad_checksum_2.installed_version).to eq "1.2.2"
648652

653+
allow(klass).to receive(:upgrade_cask) do |_, new_cask, **|
654+
upgraded_tokens << new_cask.token
655+
raise Cask::CaskError, "failed" if new_cask.token.start_with?("bad-checksum")
656+
657+
InstallHelper.stub_cask_installation(new_cask)
658+
end
659+
649660
expect do
650-
klass.upgrade_casks!(args:)
661+
klass.upgrade_casks!(args:, skip_prefetch: true)
651662
end.to raise_error(Cask::MultipleCaskErrors)
652663

664+
expect(upgraded_tokens).to contain_exactly("bad-checksum", "bad-checksum2", "local-transmission-zip")
665+
653666
expect(bad_checksum).to be_installed
654667
expect(bad_checksum_path).to be_a_directory
655668
expect(bad_checksum.installed_version).to eq "1.2.2"

Library/Homebrew/test/cmd/--cache_spec.rb

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,27 +5,20 @@
55
require "cmd/shared_examples/args_parse"
66

77
RSpec.describe Homebrew::Cmd::Cache do
8+
let(:klass) { Homebrew::Cmd::Cache }
9+
810
it_behaves_like "parseable arguments"
911

10-
it "prints all cache files for a given Formula", :integration_test do
11-
expect { brew "--cache", testball }
12-
.to output(%r{#{HOMEBREW_CACHE}/downloads/[\da-f]{64}--testball-}o).to_stdout
13-
.and be_a_success
14-
expect { brew "--cache", "--formula", testball }
12+
it "prints all cache files for a given Formula" do
13+
expect { klass.new(["--formula", (TEST_FIXTURE_DIR/"testball.rb").to_s]).run }
1514
.to output(%r{#{HOMEBREW_CACHE}/downloads/[\da-f]{64}--testball-}o).to_stdout
1615
.and not_to_output.to_stderr
17-
.and be_a_success
1816
end
1917

20-
it "prints the cache files for a given Cask", :integration_test, :needs_macos do
21-
expect { brew "--cache", cask_path("local-caffeine") }
22-
.to output(%r{#{HOMEBREW_CACHE}/downloads/[\da-f]{64}--caffeine\.zip}o).to_stdout
23-
.and output(/Treating #{Regexp.escape(cask_path("local-caffeine"))} as a cask/).to_stderr
24-
.and be_a_success
25-
expect { brew "--cache", "--cask", cask_path("local-caffeine") }
18+
it "prints the cache files for a given Cask", :cask do
19+
expect { klass.new(["--cask", cask_path("local-caffeine").to_s]).run }
2620
.to output(%r{#{HOMEBREW_CACHE}/downloads/[\da-f]{64}--caffeine\.zip}o).to_stdout
2721
.and not_to_output.to_stderr
28-
.and be_a_success
2922
end
3023

3124
it "prints the cache files for a given Formula and Cask", :integration_test, :needs_macos do

Library/Homebrew/test/cmd/--caskroom_spec.rb

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
require "cmd/shared_examples/args_parse"
66

77
RSpec.describe Homebrew::Cmd::Caskroom do
8+
let(:klass) { Homebrew::Cmd::Caskroom }
9+
810
it_behaves_like "parseable arguments"
911

1012
it "prints Homebrew's Caskroom", :integration_test do
@@ -14,11 +16,16 @@
1416
.and be_a_success
1517
end
1618

17-
it "prints the Caskroom for Casks", :integration_test do
18-
expect { brew "--caskroom", cask_path("local-transmission"), cask_path("local-caffeine") }
19+
it "prints the Caskroom for Casks" do
20+
cmd = klass.new(%w[local-transmission local-caffeine])
21+
allow(cmd.args.named).to receive(:to_casks).and_return([
22+
instance_double(Cask::Cask, token: "local-transmission"),
23+
instance_double(Cask::Cask, token: "local-caffeine"),
24+
])
25+
26+
expect { cmd.run }
1927
.to output("#{HOMEBREW_PREFIX/"Caskroom"/"local-transmission"}\n" \
20-
"#{HOMEBREW_PREFIX/"Caskroom"/"local-caffeine\n"}").to_stdout
28+
"#{HOMEBREW_PREFIX/"Caskroom"/"local-caffeine"}\n").to_stdout
2129
.and not_to_output.to_stderr
22-
.and be_a_success
2330
end
2431
end

Library/Homebrew/test/cmd/--cellar_spec.rb

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
require "cmd/shared_examples/args_parse"
66

77
RSpec.describe Homebrew::Cmd::Cellar do
8+
let(:klass) { Homebrew::Cmd::Cellar }
9+
810
it_behaves_like "parseable arguments"
911

1012
it "prints Homebrew's Cellar", :integration_test do
@@ -14,10 +16,13 @@
1416
.and be_a_success
1517
end
1618

17-
it "prints the Cellar for a Formula", :integration_test do
18-
expect { brew "--cellar", testball }
19+
it "prints the Cellar for a Formula" do
20+
cmd = klass.new(["testball"])
21+
allow(cmd.args.named).to receive(:to_resolved_formulae)
22+
.and_return([instance_double(Formula, rack: HOMEBREW_CELLAR/"testball")])
23+
24+
expect { cmd.run }
1925
.to output(%r{#{HOMEBREW_CELLAR}/testball}o).to_stdout
2026
.and not_to_output.to_stderr
21-
.and be_a_success
2227
end
2328
end

Library/Homebrew/test/cmd/--prefix_spec.rb

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
require "cmd/shared_examples/args_parse"
66

77
RSpec.describe Homebrew::Cmd::Prefix do
8+
let(:klass) { Homebrew::Cmd::Prefix }
9+
810
it_behaves_like "parseable arguments"
911

1012
it "prints Homebrew's prefix", :integration_test do
@@ -14,24 +16,31 @@
1416
.and be_a_success
1517
end
1618

17-
it "prints the prefix for a Formula", :integration_test, :needs_homebrew_core do
18-
expect { brew_sh "--prefix", "wget" }
19-
.to output("#{ENV.fetch("HOMEBREW_PREFIX")}/opt/wget\n").to_stdout
19+
it "prints the prefix for a Formula" do
20+
cmd = klass.new(["testball"])
21+
allow(cmd.args.named).to receive(:to_resolved_formulae)
22+
.and_return([instance_double(Formula, opt_prefix: HOMEBREW_PREFIX/"opt/testball")])
23+
24+
expect { cmd.run }
25+
.to output("#{HOMEBREW_PREFIX}/opt/testball\n").to_stdout
2026
.and not_to_output.to_stderr
21-
.and be_a_success
2227
end
2328

24-
it "errors if the given Formula doesn't exist", :integration_test do
25-
expect { brew "--prefix", "nonexistent" }
26-
.to output(/No available formula/).to_stderr
27-
.and not_to_output.to_stdout
28-
.and be_a_failure
29+
it "errors if the given Formula doesn't exist" do
30+
cmd = klass.new(["nonexistent"])
31+
allow(cmd.args.named).to receive(:to_resolved_formulae)
32+
.and_raise(FormulaUnavailableError.new("nonexistent"))
33+
34+
expect { cmd.run }.to raise_error(FormulaUnavailableError, /nonexistent/)
2935
end
3036

31-
it "prints a warning when `--installed` is used and the given Formula is not installed", :integration_test do
32-
expect { brew "--prefix", "--installed", testball }
33-
.to not_to_output.to_stdout
34-
.and output(/testball/).to_stderr
35-
.and be_a_failure
37+
it "prints a warning when `--installed` is used and the given Formula is not installed" do
38+
cmd = klass.new(["--installed", "testball"])
39+
allow(cmd.args.named).to receive(:to_resolved_formulae).and_return([
40+
instance_double(Formula, name: "testball", opt_prefix: HOMEBREW_PREFIX/"opt/testball", optlinked?: false),
41+
])
42+
43+
expect { cmd.run }
44+
.to raise_error(NotAKegError, /testball/)
3645
end
3746
end
Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,29 @@
11
# typed: false
22
# frozen_string_literal: true
33

4-
RSpec.describe "brew --repository", type: :system do
5-
it "prints Homebrew's repository", :integration_test do
6-
expect { brew_sh "--repository" }
7-
.to output("#{ENV.fetch("HOMEBREW_REPOSITORY")}\n").to_stdout
8-
.and not_to_output.to_stderr
9-
.and be_a_success
10-
end
4+
require "open3"
115

12-
it "prints a Tap's repository", :integration_test do
13-
expect { brew_sh "--repository", "foo/bar" }
14-
.to output("#{ENV.fetch("HOMEBREW_LIBRARY")}/Taps/foo/homebrew-bar\n").to_stdout
15-
.and not_to_output.to_stderr
16-
.and be_a_success
17-
end
6+
RSpec.describe "brew --repository", type: :system do
7+
it "prints Homebrew and Tap repositories" do
8+
stdout, stderr, status = Open3.capture3(
9+
{
10+
"HOMEBREW_LIBRARY" => ENV.fetch("HOMEBREW_LIBRARY"),
11+
"HOMEBREW_REPOSITORY" => ENV.fetch("HOMEBREW_REPOSITORY"),
12+
},
13+
"/bin/bash", "-c", <<~SH,
14+
source "$1"
15+
homebrew---repository
16+
homebrew---repository foo/bar foo/homebrew-bar
17+
SH
18+
"bash", (HOMEBREW_LIBRARY_PATH/"cmd/--repository.sh").to_s
19+
)
1820

19-
it "prints a Tap's repository correctly when homebrew- prefix is supplied", :integration_test do
20-
expect { brew_sh "--repository", "foo/homebrew-bar" }
21-
.to output("#{ENV.fetch("HOMEBREW_LIBRARY")}/Taps/foo/homebrew-bar\n").to_stdout
22-
.and not_to_output.to_stderr
23-
.and be_a_success
21+
expect(status).to be_success
22+
expect(stdout).to eq(<<~EOS)
23+
#{ENV.fetch("HOMEBREW_REPOSITORY")}
24+
#{ENV.fetch("HOMEBREW_LIBRARY")}/Taps/foo/homebrew-bar
25+
#{ENV.fetch("HOMEBREW_LIBRARY")}/Taps/foo/homebrew-bar
26+
EOS
27+
expect(stderr).to be_empty
2428
end
2529
end

Library/Homebrew/test/cmd/desc_spec.rb

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -55,25 +55,26 @@
5555
.and not_to_output.to_stderr
5656
end
5757

58-
it "errors when searching without --eval-all", :integration_test, :no_api do
59-
setup_test_formula "testball"
60-
61-
expect { brew "desc", "--search", "testball" }
62-
.to output(/`brew desc --search` needs `--eval-all` passed or `HOMEBREW_EVAL_ALL=1` set!/).to_stderr
63-
.and be_a_failure
58+
it "errors when searching without --eval-all" do
59+
with_env("HOMEBREW_NO_INSTALL_FROM_API" => "1") do
60+
expect { klass.new(["--search", "testball"]).run }
61+
.to raise_error(UsageError, /`brew desc --search` needs `--eval-all` passed/)
62+
end
6463
end
6564

66-
it "successfully searches with --search --eval-all", :integration_test, :no_api do
67-
setup_test_formula "testball"
65+
it "successfully searches with --search --eval-all" do
66+
expect(Homebrew::Search).to receive(:search_descriptions)
67+
.with("ball", anything, search_type: Descriptions::SearchField::Either)
6868

69-
expect { brew "desc", "--search", "--eval-all", "ball" }
70-
.to output(/testball: Some test/).to_stdout
71-
.and not_to_output.to_stderr
69+
expect { klass.new(["--search", "--eval-all", "ball"]).run }
70+
.to not_to_output.to_stderr
7271
end
7372

74-
it "successfully searches without --eval-all, with API", :integration_test, :needs_network do
75-
setup_test_formula "testball"
73+
it "successfully searches without --eval-all, with API" do
74+
expect(Homebrew::Search).to receive(:search_descriptions)
75+
.with("testball", anything, search_type: Descriptions::SearchField::Either)
7676

77-
expect { brew "desc", "--search", "testball" }.to be_a_success
77+
expect { klass.new(["--search", "testball"]).run }
78+
.to not_to_output.to_stderr
7879
end
7980
end

0 commit comments

Comments
 (0)