Merge pull request #3026 from reitermarkus/refactor-download-strategies

Refactor download strategies.

2 files changed, 23 insertions, 54 deletions

diff --git a/Library/Homebrew/test/cask/download_strategy_spec.rb b/Library/Homebrew/test/cask/download_strategy_spec.rb
index 222352c07..60d101bdb 100644
--- a/Library/Homebrew/test/cask/download_strategy_spec.rb
+++ b/Library/Homebrew/test/cask/download_strategy_spec.rb
@@ -27,8 +27,11 @@ describe "download strategies", :cask do
 
       expect(downloader).to have_received(:curl).with(
         cask.url.to_s,
-        "-C", 0,
-        "-o", kind_of(Pathname)
+        "--location",
+        "--remote-time",
+        "--continue-at", "-",
+        "--output", kind_of(Pathname),
+        user_agent: :default
       )
     end
 
@@ -36,25 +39,25 @@ describe "download strategies", :cask do
       let(:url_options) { { user_agent: "Mozilla/25.0.1" } }
 
       it "adds the appropriate curl args" do
-        curl_args = []
-        allow(downloader).to receive(:curl) { |*args| curl_args = args }
+        expect(downloader).to receive(:safe_system) { |*args|
+          expect(args.each_cons(2)).to include(["--user-agent", "Mozilla/25.0.1"])
+        }
 
         downloader.fetch
-
-        expect(curl_args.each_cons(2)).to include(["-A", "Mozilla/25.0.1"])
       end
     end
 
     context "with a generalized fake user agent" do
+      alias_matcher :a_string_matching, :match
+
       let(:url_options) { { user_agent: :fake } }
 
       it "adds the appropriate curl args" do
-        curl_args = []
-        allow(downloader).to receive(:curl) { |*args| curl_args = args }
+        expect(downloader).to receive(:safe_system) { |*args|
+          expect(args.each_cons(2).to_a).to include(["--user-agent", a_string_matching(/Mozilla.*Mac OS X 10.*AppleWebKit/)])
+        }
 
         downloader.fetch
-
-        expect(curl_args.each_cons(2)).to include(["-A", "Mozilla/5.0 (Macintosh; Intel Mac OS X 10) https://caskroom.github.io"])
       end
     end
 
@@ -138,7 +141,7 @@ describe "download strategies", :cask do
   describe Hbc::SubversionDownloadStrategy do
     let(:url_options) { { using: :svn } }
     let(:fake_system_command) { class_double(Hbc::SystemCommand) }
-    let(:downloader) { Hbc::SubversionDownloadStrategy.new(cask, fake_system_command) }
+    let(:downloader) { Hbc::SubversionDownloadStrategy.new(cask, command: fake_system_command) }
     before do
       allow(fake_system_command).to receive(:run!)
     end
@@ -147,7 +150,7 @@ describe "download strategies", :cask do
       allow(downloader).to receive(:compress)
       allow(downloader).to receive(:fetch_repo)
 
-      expect(downloader.fetch).to equal(downloader.tarball_path)
+      expect(downloader.fetch).to equal(downloader.cached_location)
     end
 
     it "calls fetch_repo with default arguments for a simple Cask" do
@@ -237,44 +240,5 @@ describe "download strategies", :cask do
         )
       end
     end
-
-    it "runs tar to serialize svn downloads" do
-      # sneaky stub to remake the directory, since homebrew code removes it
-      # before tar is called
-      allow(downloader).to receive(:fetch_repo) {
-        downloader.cached_location.mkdir
-      }
-
-      downloader.fetch
-
-      expect(fake_system_command).to have_received(:run!).with(
-        "/usr/bin/tar",
-        hash_including(args: [
-                         '-s/^\\.//',
-                         "--exclude",
-                         ".svn",
-                         "-cf",
-                         downloader.tarball_path,
-                         "--",
-                         ".",
-                       ]),
-      )
-    end
   end
-
-  # does not work yet, because (for unknown reasons), the tar command
-  # returns an error code when running under the test suite
-  # it 'creates a tarball matching the expected checksum' do
-  #   cask = Hbc::CaskLoader.load('svn-download-check-cask')
-  #   downloader = Hbc::SubversionDownloadStrategy.new(cask)
-  #   # special mocking required for tar to have something to work with
-  #   def downloader.fetch_repo(target, url, revision = nil, ignore_externals=false)
-  #     target.mkpath
-  #     FileUtils.touch(target.join('empty_file.txt'))
-  #     File.utime(1000,1000,target.join('empty_file.txt'))
-  #   end
-  #   expect(downloader.fetch).to equal(downloader.tarball_path)
-  #   d = Hbc::Download.new(cask)
-  #   d.send(:_check_sums, downloader.tarball_path, cask.sums)
-  # end
 end
diff --git a/Library/Homebrew/test/cask/dsl/appcast_spec.rb b/Library/Homebrew/test/cask/dsl/appcast_spec.rb
index b8903b1be..ccc6a4633 100644
--- a/Library/Homebrew/test/cask/dsl/appcast_spec.rb
+++ b/Library/Homebrew/test/cask/dsl/appcast_spec.rb
@@ -33,13 +33,18 @@ describe Hbc::DSL::Appcast do
 
   describe "#calculate_checkpoint" do
     before do
-      expect(Hbc::SystemCommand).to receive(:run).with(*cmd_args).and_return(cmd_result)
+      expect(Hbc::SystemCommand).to receive(:run) do |executable, **options|
+        expect(executable).to eq "/usr/bin/curl"
+        expect(options[:args]).to include(*cmd_args)
+        expect(options[:print_stderr]).to be false
+        cmd_result
+      end
       allow(cmd_result).to receive(:success?).and_return(cmd_success)
       allow(cmd_result).to receive(:stdout).and_return(cmd_stdout)
     end
 
     context "when server returns a successful HTTP status" do
-      let(:cmd_args) { ["/usr/bin/curl", args: ["--compressed", "--location", "--user-agent", Hbc::URL::FAKE_USER_AGENT, "--fail", uri], print_stderr: false] }
+      let(:cmd_args) { [HOMEBREW_USER_AGENT_FAKE_SAFARI, "--compressed", "--location", "--fail", uri] }
       let(:cmd_result) { double("Hbc::SystemCommand::Result") }
       let(:cmd_success) { true }
       let(:cmd_stdout) { "hello world" }
@@ -56,7 +61,7 @@ describe Hbc::DSL::Appcast do
     end
 
     context "when server returns a non-successful HTTP status" do
-      let(:cmd_args) { ["/usr/bin/curl", args: ["--compressed", "--location", "--user-agent", Hbc::URL::FAKE_USER_AGENT, "--fail", uri], print_stderr: false] }
+      let(:cmd_args) { [HOMEBREW_USER_AGENT_FAKE_SAFARI, "--compressed", "--location", "--fail", uri] }
       let(:cmd_result) { double("Hbc::SystemCommand::Result") }
       let(:cmd_success) { false }
       let(:cmd_stdout) { "some error message from the server" }