Merge pull request #3018 from Homebrew/revert-2848-refactoring

Revert "Refactor SVN and cURL download strategies."

2 files changed, 54 insertions, 23 deletions

diff --git a/Library/Homebrew/test/cask/download_strategy_spec.rb b/Library/Homebrew/test/cask/download_strategy_spec.rb
index 60d101bdb..222352c07 100644
--- a/Library/Homebrew/test/cask/download_strategy_spec.rb
+++ b/Library/Homebrew/test/cask/download_strategy_spec.rb
@@ -27,11 +27,8 @@ describe "download strategies", :cask do
 
       expect(downloader).to have_received(:curl).with(
         cask.url.to_s,
-        "--location",
-        "--remote-time",
-        "--continue-at", "-",
-        "--output", kind_of(Pathname),
-        user_agent: :default
+        "-C", 0,
+        "-o", kind_of(Pathname)
       )
     end
 
@@ -39,25 +36,25 @@ describe "download strategies", :cask do
       let(:url_options) { { user_agent: "Mozilla/25.0.1" } }
 
       it "adds the appropriate curl args" do
-        expect(downloader).to receive(:safe_system) { |*args|
-          expect(args.each_cons(2)).to include(["--user-agent", "Mozilla/25.0.1"])
-        }
+        curl_args = []
+        allow(downloader).to receive(:curl) { |*args| curl_args = args }
 
         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
-        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/)])
-        }
+        curl_args = []
+        allow(downloader).to receive(:curl) { |*args| curl_args = args }
 
         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
 
@@ -141,7 +138,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, command: fake_system_command) }
+    let(:downloader) { Hbc::SubversionDownloadStrategy.new(cask, fake_system_command) }
     before do
       allow(fake_system_command).to receive(:run!)
     end
@@ -150,7 +147,7 @@ describe "download strategies", :cask do
       allow(downloader).to receive(:compress)
       allow(downloader).to receive(:fetch_repo)
 
-      expect(downloader.fetch).to equal(downloader.cached_location)
+      expect(downloader.fetch).to equal(downloader.tarball_path)
     end
 
     it "calls fetch_repo with default arguments for a simple Cask" do
@@ -240,5 +237,44 @@ 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 ccc6a4633..b8903b1be 100644
--- a/Library/Homebrew/test/cask/dsl/appcast_spec.rb
+++ b/Library/Homebrew/test/cask/dsl/appcast_spec.rb
@@ -33,18 +33,13 @@ describe Hbc::DSL::Appcast do
 
   describe "#calculate_checkpoint" do
     before do
-      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
+      expect(Hbc::SystemCommand).to receive(:run).with(*cmd_args).and_return(cmd_result)
       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) { [HOMEBREW_USER_AGENT_FAKE_SAFARI, "--compressed", "--location", "--fail", uri] }
+      let(:cmd_args) { ["/usr/bin/curl", args: ["--compressed", "--location", "--user-agent", Hbc::URL::FAKE_USER_AGENT, "--fail", uri], print_stderr: false] }
       let(:cmd_result) { double("Hbc::SystemCommand::Result") }
       let(:cmd_success) { true }
       let(:cmd_stdout) { "hello world" }
@@ -61,7 +56,7 @@ describe Hbc::DSL::Appcast do
     end
 
     context "when server returns a non-successful HTTP status" do
-      let(:cmd_args) { [HOMEBREW_USER_AGENT_FAKE_SAFARI, "--compressed", "--location", "--fail", uri] }
+      let(:cmd_args) { ["/usr/bin/curl", args: ["--compressed", "--location", "--user-agent", Hbc::URL::FAKE_USER_AGENT, "--fail", uri], print_stderr: false] }
       let(:cmd_result) { double("Hbc::SystemCommand::Result") }
       let(:cmd_success) { false }
       let(:cmd_stdout) { "some error message from the server" }