From b66097fa3d3a7927eb13771786c3832ae5621b17 Mon Sep 17 00:00:00 2001 From: apainintheneck Date: Sat, 16 Mar 2024 13:14:12 -0700 Subject: [PATCH 1/3] spec_helper: add check for unexpected network calls Any test that is not tagged as :needs_network and that makes a call to an unapproved method in the `Utils::Curl` module will raise an error unless that method gets mocked somehow. tests: add exceptions for tests that use curl to download local files These are acceptable ways to use curl in local, non-network tests. For those edge cases we allow you to bypass the check with :needs_utils_curl. --- Library/Homebrew/test/cask/cask_spec.rb | 6 ++++-- Library/Homebrew/test/formulary_spec.rb | 3 ++- Library/Homebrew/test/spec_helper.rb | 9 +++++++++ 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/test/cask/cask_spec.rb b/Library/Homebrew/test/cask/cask_spec.rb index fea5589a37..6558078ee0 100644 --- a/Library/Homebrew/test/cask/cask_spec.rb +++ b/Library/Homebrew/test/cask/cask_spec.rb @@ -46,13 +46,15 @@ RSpec.describe Cask::Cask, :cask do expect(c.token).to eq("caffeine") end - it "returns an instance of the Cask from a URL" do + it "returns an instance of the Cask from a URL", :needs_utils_curl do + expect(Homebrew::API).not_to receive(:fetch_json_api_file) c = Cask::CaskLoader.load("file://#{tap_path}/Casks/local-caffeine.rb") expect(c).to be_a(described_class) expect(c.token).to eq("local-caffeine") end - it "raises an error when failing to download a Cask from a URL" do + it "raises an error when failing to download a Cask from a URL", :needs_utils_curl do + expect(Homebrew::API).not_to receive(:fetch_json_api_file) expect do Cask::CaskLoader.load("file://#{tap_path}/Casks/notacask.rb") end.to raise_error(Cask::CaskUnavailableError) diff --git a/Library/Homebrew/test/formulary_spec.rb b/Library/Homebrew/test/formulary_spec.rb index 86631a3a66..3bc5c5ee05 100644 --- a/Library/Homebrew/test/formulary_spec.rb +++ b/Library/Homebrew/test/formulary_spec.rb @@ -119,7 +119,8 @@ RSpec.describe Formulary do expect(described_class.factory(formula_path)).to be_a(Formula) end - it "returns a Formula when given a URL" do + it "returns a Formula when given a URL", :needs_utils_curl do + expect(Homebrew::API).not_to receive(:fetch_json_api_file) formula = described_class.factory("file://#{formula_path}") expect(formula).to be_a(Formula) end diff --git a/Library/Homebrew/test/spec_helper.rb b/Library/Homebrew/test/spec_helper.rb index bb08dd6e5a..47e5eff401 100644 --- a/Library/Homebrew/test/spec_helper.rb +++ b/Library/Homebrew/test/spec_helper.rb @@ -165,6 +165,15 @@ RSpec.configure do |config| skip "Requires network connection." unless ENV["HOMEBREW_TEST_ONLINE"] end + config.before do |example| + next if example.metadata.key?(:needs_network) + next if example.metadata.key?(:needs_utils_curl) + + allow(Utils::Curl).to receive(:curl_executable).and_raise(<<~ERROR) + Unexpected call to Utils::Curl.curl_executable without setting :needs_network or :needs_utils_curl. + ERROR + end + config.before(:each, :needs_svn) do svn_shim = HOMEBREW_SHIMS_PATH/"shared/svn" skip "Subversion is not installed." unless quiet_system svn_shim, "--version" From ad35db4b24997c7999e9c1c0a37ff66d2e985ccd Mon Sep 17 00:00:00 2001 From: apainintheneck Date: Sat, 16 Mar 2024 13:17:54 -0700 Subject: [PATCH 2/3] tests: fix tests that make unexpected network calls These were found with the Utils::Curl check and just turning off the network on my computer and running the entire test suite. --- Library/Homebrew/cli/named_args.rb | 8 ++++++-- Library/Homebrew/test/cask/info_spec.rb | 5 +++++ Library/Homebrew/test/formulary_spec.rb | 4 ++++ Library/Homebrew/test/tap_spec.rb | 2 +- 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/cli/named_args.rb b/Library/Homebrew/cli/named_args.rb index 1b30ef7d6b..284c18aa9b 100644 --- a/Library/Homebrew/cli/named_args.rb +++ b/Library/Homebrew/cli/named_args.rb @@ -256,11 +256,15 @@ module Homebrew paths = [] if formula_path.exist? || - (!CoreTap.instance.installed? && Homebrew::API::Formula.all_formulae.key?(path.basename.to_s)) + (!Homebrew::EnvConfig.no_install_from_api? && + !CoreTap.instance.installed? && + Homebrew::API::Formula.all_formulae.key?(path.basename.to_s)) paths << formula_path end if cask_path.exist? || - (!CoreCaskTap.instance.installed? && Homebrew::API::Cask.all_casks.key?(path.basename.to_s)) + (!Homebrew::EnvConfig.no_install_from_api? && + !CoreCaskTap.instance.installed? && + Homebrew::API::Cask.all_casks.key?(path.basename.to_s)) paths << cask_path end diff --git a/Library/Homebrew/test/cask/info_spec.rb b/Library/Homebrew/test/cask/info_spec.rb index 2c5a5aefc0..8710a8ba70 100644 --- a/Library/Homebrew/test/cask/info_spec.rb +++ b/Library/Homebrew/test/cask/info_spec.rb @@ -3,6 +3,11 @@ require "utils" RSpec.describe Cask::Info, :cask do + before do + # Prevent unnecessary network requests in `Utils::Analytics.cask_output` + ENV["HOMEBREW_NO_ANALYTICS"] = "1" + end + it "displays some nice info about the specified Cask" do expect do described_class.info(Cask::CaskLoader.load("local-transmission")) diff --git a/Library/Homebrew/test/formulary_spec.rb b/Library/Homebrew/test/formulary_spec.rb index 3bc5c5ee05..8e34f0fb42 100644 --- a/Library/Homebrew/test/formulary_spec.rb +++ b/Library/Homebrew/test/formulary_spec.rb @@ -384,6 +384,10 @@ RSpec.describe Formulary do before do ENV.delete("HOMEBREW_NO_INSTALL_FROM_API") + # avoid unnecessary network calls + allow(Homebrew::API::Formula).to receive_messages(all_aliases: {}, all_renames: {}) + allow(CoreTap.instance).to receive(:tap_migrations).and_return({}) + # don't try to load/fetch gcc/glibc allow(DevelopmentTools).to receive_messages(needs_libc_formula?: false, needs_compiler_formula?: false) end diff --git a/Library/Homebrew/test/tap_spec.rb b/Library/Homebrew/test/tap_spec.rb index 8ea9116037..44a771fc31 100644 --- a/Library/Homebrew/test/tap_spec.rb +++ b/Library/Homebrew/test/tap_spec.rb @@ -192,7 +192,7 @@ RSpec.describe Tap do end describe "#remote" do - it "returns the remote URL" do + it "returns the remote URL", :needs_network do setup_git_repo expect(homebrew_foo_tap.remote).to eq("https://github.com/Homebrew/homebrew-foo") From 74aea8e92d78201db65b6eedfd9ec60954264fc1 Mon Sep 17 00:00:00 2001 From: apainintheneck Date: Tue, 19 Mar 2024 22:18:02 -0700 Subject: [PATCH 3/3] spec_helper: add :no_api test scope This sets the HOMEBREW_NO_INSTALL_FROM_API environment variable to prevent the selected tests from using the API. We will need this as we transition to having the API be enabled by default when running the tests but it's also nice as a sanity check with the :needs_utils_curl scope in a few places. --- .../test/cask/cask_loader/from_api_loader_spec.rb | 6 +----- Library/Homebrew/test/cask/cask_loader_spec.rb | 12 ++---------- Library/Homebrew/test/cask/cask_spec.rb | 6 ++---- Library/Homebrew/test/formulary_spec.rb | 3 +-- Library/Homebrew/test/spec_helper.rb | 4 ++++ Library/Homebrew/test/tap_spec.rb | 3 +-- 6 files changed, 11 insertions(+), 23 deletions(-) diff --git a/Library/Homebrew/test/cask/cask_loader/from_api_loader_spec.rb b/Library/Homebrew/test/cask/cask_loader/from_api_loader_spec.rb index 7e567e95fe..eb21387245 100644 --- a/Library/Homebrew/test/cask/cask_loader/from_api_loader_spec.rb +++ b/Library/Homebrew/test/cask/cask_loader/from_api_loader_spec.rb @@ -22,11 +22,7 @@ RSpec.describe Cask::CaskLoader::FromAPILoader, :cask do describe ".can_load?" do include_context "with API setup", "test-opera" - context "when not using the API" do - before do - ENV["HOMEBREW_NO_INSTALL_FROM_API"] = "1" - end - + context "when not using the API", :no_api do it "returns false" do expect(described_class.try_new(token)).to be_nil end diff --git a/Library/Homebrew/test/cask/cask_loader_spec.rb b/Library/Homebrew/test/cask/cask_loader_spec.rb index e7fcc9696e..62be102cea 100644 --- a/Library/Homebrew/test/cask/cask_loader_spec.rb +++ b/Library/Homebrew/test/cask/cask_loader_spec.rb @@ -30,11 +30,7 @@ RSpec.describe Cask::CaskLoader, :cask do .and_return(cask_renames) end - context "when not using the API" do - before do - ENV["HOMEBREW_NO_INSTALL_FROM_API"] = "1" - end - + context "when not using the API", :no_api do it "warns when using the short token" do expect do expect(described_class.for("version-newest")).to be_a Cask::CaskLoader::FromPathLoader @@ -67,11 +63,7 @@ RSpec.describe Cask::CaskLoader, :cask do end end - context "when not using the API" do - before do - ENV["HOMEBREW_NO_INSTALL_FROM_API"] = "1" - end - + context "when not using the API", :no_api do context "when a cask is migrated" do let(:token) { "local-caffeine" } diff --git a/Library/Homebrew/test/cask/cask_spec.rb b/Library/Homebrew/test/cask/cask_spec.rb index 6558078ee0..cadba6516e 100644 --- a/Library/Homebrew/test/cask/cask_spec.rb +++ b/Library/Homebrew/test/cask/cask_spec.rb @@ -46,15 +46,13 @@ RSpec.describe Cask::Cask, :cask do expect(c.token).to eq("caffeine") end - it "returns an instance of the Cask from a URL", :needs_utils_curl do - expect(Homebrew::API).not_to receive(:fetch_json_api_file) + it "returns an instance of the Cask from a URL", :needs_utils_curl, :no_api do c = Cask::CaskLoader.load("file://#{tap_path}/Casks/local-caffeine.rb") expect(c).to be_a(described_class) expect(c.token).to eq("local-caffeine") end - it "raises an error when failing to download a Cask from a URL", :needs_utils_curl do - expect(Homebrew::API).not_to receive(:fetch_json_api_file) + it "raises an error when failing to download a Cask from a URL", :needs_utils_curl, :no_api do expect do Cask::CaskLoader.load("file://#{tap_path}/Casks/notacask.rb") end.to raise_error(Cask::CaskUnavailableError) diff --git a/Library/Homebrew/test/formulary_spec.rb b/Library/Homebrew/test/formulary_spec.rb index 8e34f0fb42..3bb7491bdf 100644 --- a/Library/Homebrew/test/formulary_spec.rb +++ b/Library/Homebrew/test/formulary_spec.rb @@ -119,8 +119,7 @@ RSpec.describe Formulary do expect(described_class.factory(formula_path)).to be_a(Formula) end - it "returns a Formula when given a URL", :needs_utils_curl do - expect(Homebrew::API).not_to receive(:fetch_json_api_file) + it "returns a Formula when given a URL", :needs_utils_curl, :no_api do formula = described_class.factory("file://#{formula_path}") expect(formula).to be_a(Formula) end diff --git a/Library/Homebrew/test/spec_helper.rb b/Library/Homebrew/test/spec_helper.rb index 47e5eff401..70fa46df38 100644 --- a/Library/Homebrew/test/spec_helper.rb +++ b/Library/Homebrew/test/spec_helper.rb @@ -174,6 +174,10 @@ RSpec.configure do |config| ERROR end + config.before(:each, :no_api) do + ENV["HOMEBREW_NO_INSTALL_FROM_API"] = "1" + end + config.before(:each, :needs_svn) do svn_shim = HOMEBREW_SHIMS_PATH/"shared/svn" skip "Subversion is not installed." unless quiet_system svn_shim, "--version" diff --git a/Library/Homebrew/test/tap_spec.rb b/Library/Homebrew/test/tap_spec.rb index 44a771fc31..ecfa2b8996 100644 --- a/Library/Homebrew/test/tap_spec.rb +++ b/Library/Homebrew/test/tap_spec.rb @@ -488,8 +488,7 @@ RSpec.describe Tap do expect(described_class.to_a).to include(CoreTap.instance) end - it "omits the core tap without the api" do - ENV["HOMEBREW_NO_INSTALL_FROM_API"] = "1" + it "omits the core tap without the api", :no_api do expect(described_class.to_a).not_to include(CoreTap.instance) end end