Merge pull request #218 from borsothy/GithubDownloadErrorHandling

Improved error handling in 'download_from_github'

lib/moonshot/artifact_repository/s3_bucket_via_github_releases.rb

@@ -4,6 +4,7 @@
 require 'securerandom'
 require 'semantic'
 require 'tmpdir'
+require 'retriable'
 
 module Moonshot::ArtifactRepository
   # S3 Bucket repository backed by GitHub releases.
@@ -115,25 +116,26 @@ def upload_to_s3(file, key)
     # @raise [RuntimeError] If the file fails to download correctly after 3
     #                       attempts.
     def download_from_github(version)
+      file_pattern = "*#{version}*.tar.gz"
       attempts = 0
-      begin
+
+      Retriable.retriable on: RuntimeError do
         # Make sure the directory is empty before downloading the release.
         FileUtils.rm(Dir.glob('*'))
 
         # Download the release and find the actual build file.
         sh_out("hub release download #{version}")
-        file = Dir.glob("*#{version}*.tar.gz").fetch(0)
 
+        raise "File '#{file_pattern}' not found." if Dir.glob(file_pattern).empty?
+
+        file = Dir.glob(file_pattern).fetch(0)
         unless (checksum = checksum_file(file)).nil?
           verify_download_checksum(file, checksum, attempt: attempts)
         end
-      rescue RuntimeError => e
         attempts += 1
-        retry unless attempts > 3
-        raise e
-      end
 
-      file
+        file
+      end
     end
 
     # Find the checksum file for a release, if there is one.

moonshot.gemspec

@@ -19,6 +19,7 @@ Gem::Specification.new do |s|
   s.add_dependency('interactive-logger', '~> 0.1.2')
   s.add_dependency('rotp', '~> 2.1.1')
   s.add_dependency('ruby-duration', '~> 3.2.3')
+  s.add_dependency('retriable')
   # Pin back activesupport (ruby-duration dependency) until we only support
   # Ruby >= 2.2.2.
   s.add_dependency('activesupport', '< 5.0.0')

spec/moonshot/artifact_repository/s3_bucket_via_github_releases_spec.rb

@@ -0,0 +1,71 @@
+require 'fakefs/spec_helpers'
+
+module Moonshot
+  describe ArtifactRepository::S3BucketViaGithubReleases do
+    let(:version) { '0.0.0'}
+    let(:bucket_name) { 'moonshot-test-bucket' }
+    let(:artifact_name) { "moonshot-#{version}-beta.tar.gz" }
+    let(:hub_command) { "hub release download #{version}" }
+
+    def fake_artifact
+      FakeFS  { FileUtils.touch(artifact_name) }
+    end
+
+    def fake_checksum_file
+      FakeFS do 
+        base = File.basename(artifact_name, '.tar.gz')
+        FileUtils.touch("#{base}.md5")
+      end
+    end
+
+    subject { described_class.new(bucket_name) }
+
+    describe '#download_from_github' do
+      include FakeFS::SpecHelpers
+
+      it 'should use hub to download the latest version' do
+        obj = subject
+        expect(obj).to receive(:sh_out) { fake_artifact }.with(hub_command)
+
+        subject.send(:download_from_github, version)
+      end
+
+      it 'should raise RuntimeError when download has failed' do
+        obj = subject
+        expect(obj).to receive(:sh_out).at_least(:once)
+
+        expect do
+          obj.send(:download_from_github, version)
+        end.to raise_error(RuntimeError)
+      end
+
+      it 'should retry if the download has failed' do
+        obj = subject
+        expect(obj).to receive(:sh_out).with(hub_command).exactly(3).times
+
+        expect { obj.send(:download_from_github, version) }.to raise_error(RuntimeError)
+      end
+
+      it 'should verify the checksum of the downloaded artifact' do
+        obj = subject
+
+        expect(obj).to receive(:sh_out) do
+          fake_artifact
+          fake_checksum_file
+        end.with(hub_command)
+
+        expect(obj).to receive(:verify_download_checksum)
+        obj.send(:download_from_github, version)
+      end
+
+      it 'should return the name of the downloaded artifact' do
+        obj = subject
+
+        allow(obj).to receive(:sh_out) { fake_artifact }.with(hub_command)
+
+        expect(obj.send(:download_from_github, version))\
+          .to eql("/#{artifact_name}")
+      end
+    end
+  end
+end