Merge pull request #16166 from jawshooah/audit-enhancements

Audit: further spec enhancements and refactoring

lib/hbc/audit.rb

@@ -4,20 +4,25 @@ require 'hbc/download'
 class Hbc::Audit
   include Hbc::Checkable
 
-  attr_reader :cask
+  attr_reader :cask, :download
 
-  def initialize(cask)
+  def initialize(cask, download = false)
     @cask = cask
+    @download = download
   end
 
-  def run!(download = false)
+  def run!
     check_required_stanzas
     check_no_string_version_latest
     check_sha256
     check_appcast
     check_sourceforge_download_url_format
-    check_download(download) if download
-    return !(errors? or warnings?)
+    check_download if download
+    self
+  end
+
+  def success?
+    !(errors? || warnings?)
   end
 
   def summary_header
@@ -31,7 +36,7 @@ class Hbc::Audit
     %i{version sha256 url homepage}.each do |sym|
       add_error "a #{sym} stanza is required" unless cask.send(sym)
     end
-    add_error 'a license value is required (:unknown is OK)' unless cask.license
+    add_error 'a license stanza is required (:unknown is OK)' unless cask.license
     add_error 'at least one name stanza is required' if cask.full_name.empty?
     # todo: specific DSL knowledge should not be spread around in various files like this
     # todo: nested_container should not still be a pseudo-artifact at this point
@@ -91,7 +96,7 @@ class Hbc::Audit
     add_error 'a sha256 is required for appcast' unless cask.appcast.sha256
   end
 
-  def check_download(download)
+  def check_download
     odebug "Auditing download"
     downloaded_path = download.perform
     Hbc::Verify.all(cask, downloaded_path)

lib/hbc/auditor.rb

@@ -1,14 +1,9 @@
 class Hbc::Auditor
   def self.audit(cask, options = {})
-    audit = Hbc::Audit.new(cask)
-
-    retval = if options.fetch(:audit_download, false)
-      audit.run!(Hbc::Download.new(cask))
-    else
-      audit.run!
-    end
-
+    download = options.fetch(:audit_download, false)
+    audit = Hbc::Audit.new(cask, download)
+    audit.run!
     puts audit.summary
-    retval
+    audit.success?
   end
 end

spec/cask/audit_spec.rb

@@ -1,117 +1,127 @@
 require 'spec_helper'
 
 describe Hbc::Audit do
-  describe "result" do
+  include AuditMatchers
+
+  let(:cask) { Hbc::Cask.new }
+  let(:download) { false }
+  let(:audit) { Hbc::Audit.new(cask, download) }
+
+  describe "#result" do
+    subject { audit.result }
+
     it "is 'failed' if there are have been any errors added" do
-      audit = Hbc::Audit.new(Hbc::Cask.new)
       audit.add_error 'bad'
       audit.add_warning 'eh'
-      expect(audit.result).to match(/failed/)
+      expect(subject).to match(/failed/)
     end
 
     it "is 'warning' if there are no errors, but there are warnings" do
-      audit = Hbc::Audit.new(Hbc::Cask.new)
       audit.add_warning 'eh'
-      expect(audit.result).to match(/warning/)
+      expect(subject).to match(/warning/)
     end
 
     it "is 'passed' if there are no errors or warning" do
-      audit = Hbc::Audit.new(Hbc::Cask.new)
-      expect(audit.result).to match(/passed/)
+      expect(subject).to match(/passed/)
     end
   end
 
-  describe "run!" do
-    describe "required fields" do
-      %w[version sha256 url homepage].each do |stanza|
-        it "adds an error if #{stanza} is missing" do
-          audit = Hbc::Audit.new(Hbc.load("missing-#{stanza}"))
-          audit.run!
-          expect(audit.errors).to include("a #{stanza} stanza is required")
+  describe "#run!" do
+    let(:cask) { Hbc.load(cask_token) }
+    subject { audit.run! }
+
+    describe "required stanzas" do
+      %w[version sha256 url name homepage license].each do |stanza|
+        context "missing #{stanza}" do
+          let(:cask_token) { "missing-#{stanza}" }
+          it { should fail_with(/#{stanza} stanza is required/) }
         end
       end
-
-      it "adds an error if license is missing" do
-        audit = Hbc::Audit.new(Hbc.load('missing-license'))
-        audit.run!
-        expect(audit.errors).to include('a license value is required (:unknown is OK)')
-      end
-
-      it "adds an error if name is missing" do
-        audit = Hbc::Audit.new(Hbc.load('missing-name'))
-        audit.run!
-        expect(audit.errors).to include('at least one name stanza is required')
-      end
     end
 
     describe "sha256 checks" do
-      it "adds an error if version is :latest and sha256 is not :no_check" do
-        audit = Hbc::Audit.new(Hbc.load('version-latest-with-checksum'))
-        audit.run!
-        expect(audit.errors).to include('you should use sha256 :no_check when version is :latest')
+      context "version is :latest and sha256 is not :no_check" do
+        let(:cask_token) { 'version-latest-with-checksum' }
+        it { should fail_with('you should use sha256 :no_check when version is :latest') }
       end
 
-      it "adds an error if sha256 is not a legal SHA-256 digest" do
-        audit = Hbc::Audit.new(Hbc.load('invalid-sha256'))
-        audit.run!
-        expect(audit.errors).to include('sha256 string must be of 64 hexadecimal characters')
+      context "sha256 is not a legal SHA-256 digest" do
+        let(:cask_token) { 'invalid-sha256' }
+        it { should fail_with('sha256 string must be of 64 hexadecimal characters') }
       end
 
-      it "adds an error if sha256 is sha256 for empty string" do
-        audit = Hbc::Audit.new(Hbc.load('sha256-for-empty-string'))
-        audit.run!
-        expect(audit.errors).to include('cannot use the sha256 for an empty string: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855')
+      context "sha256 is sha256 for empty string" do
+        let(:cask_token) { 'sha256-for-empty-string' }
+        it { should fail_with(/cannot use the sha256 for an empty string/) }
       end
     end
 
     describe "appcast checks" do
-      it "adds an error if appcast has no sha256" do
-        audit = Hbc::Audit.new(Hbc.load('appcast-missing-sha256'))
-        audit.run!
-        expect(audit.errors).to include('a sha256 is required for appcast')
+      context "appcast has no sha256" do
+        let(:cask_token) { 'appcast-missing-sha256' }
+        it { should fail_with('a sha256 is required for appcast') }
       end
 
-      it "adds an error if appcast sha256 is not a string of 64 hexadecimal characters" do
-        audit = Hbc::Audit.new(Hbc.load('appcast-invalid-sha256'))
-        audit.run!
-        expect(audit.errors).to include('sha256 string must be of 64 hexadecimal characters')
+      context "appcast sha256 is not a string of 64 hexadecimal characters" do
+        let(:cask_token) { 'appcast-invalid-sha256' }
+        it { should fail_with('sha256 string must be of 64 hexadecimal characters') }
       end
 
-      it "adds an error if appcast sha256 is sha256 for empty string" do
-        audit = Hbc::Audit.new(Hbc.load('appcast-sha256-for-empty-string'))
-        audit.run!
-        expect(audit.errors).to include('cannot use the sha256 for an empty string: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855')
+      context "appcast sha256 is sha256 for empty string" do
+        let(:cask_token) { 'appcast-sha256-for-empty-string' }
+        it { should fail_with(/cannot use the sha256 for an empty string/) }
       end
     end
 
     describe "preferred download URL formats" do
-      it "adds a warning if SourceForge doesn't use download subdomain" do
-        warning_msg = 'SourceForge URL format incorrect. See https://github.com/caskroom/homebrew-cask/blob/master/CONTRIBUTING.md#sourceforge-urls'
+      let(:warning_msg) { /SourceForge URL format incorrect/ }
 
+      context "incorrect SourceForge URL format" do
+        let(:cask_token) { 'sourceforge-incorrect-url-format' }
+        it { should warn_with(warning_msg) }
+      end
 
-        audit = Hbc::Audit.new(Hbc.load('sourceforge-incorrect-url-format'))
-        audit.run!
-        expect(audit.warnings).to include(warning_msg)
+      context "correct SourceForge URL format" do
+        let(:cask_token) { 'sourceforge-correct-url-format' }
+        it { should_not warn_with(warning_msg) }
+      end
 
-        audit = Hbc::Audit.new(Hbc.load('sourceforge-correct-url-format'))
-        audit.run!
-        expect(audit.warnings).to_not include(warning_msg)
-
-        audit = Hbc::Audit.new(Hbc.load('sourceforge-other-correct-url-format'))
-        audit.run!
-        expect(audit.warnings).to_not include(warning_msg)
+      context "correct SourceForge URL format for version :latest" do
+        let(:cask_token) { 'sourceforge-other-correct-url-format' }
+        it { should_not warn_with(warning_msg) }
       end
     end
 
     describe "audit of downloads" do
-      it "creates an error if the download fails" do
-        error_message = "Download Failed"
-        download = double()
-        download.expects(:perform).raises(StandardError.new(error_message))
+      let(:cask) { Hbc::Cask.new }
+      let(:download) { instance_double(Hbc::Download) }
+      let(:verify) { class_double(Hbc::Verify).as_stubbed_const }
+      let(:error_msg) { "Download Failed" }
 
-        audit = Hbc::Audit.new(Hbc::Cask.new)
-        audit.run!(download)
-        expect(audit.errors).to include(/#{error_message}/)
+      context "download and verification succeed" do
+        before do
+          download.expects(:perform)
+          verify.expects(:all)
+        end
+
+        it { should_not fail_with(/#{error_msg}/) }
+      end
+
+      context "download fails" do
+        before do
+          download.expects(:perform).raises(StandardError.new(error_msg))
+        end
+
+        it { should fail_with(/#{error_msg}/) }
+      end
+
+      context "verification fails" do
+        before do
+          download.expects(:perform)
+          verify.expects(:all).raises(StandardError.new(error_msg))
+        end
+
+        it { should fail_with(/#{error_msg}/) }
       end
     end
   end

spec/support/audit_matchers.rb

@@ -0,0 +1,41 @@
+module AuditMatchers
+  extend RSpec::Matchers::DSL
+
+  matcher :pass do
+    match do |audit|
+      !audit.errors? && !audit.warnings?
+    end
+  end
+
+  matcher :fail do
+    match do |audit|
+      audit.errors?
+    end
+  end
+
+  matcher :warn do
+    match do |audit|
+      audit.warnings? && !audit.errors?
+    end
+  end
+
+  matcher :fail_with do |error_msg|
+    match do |audit|
+      include_msg?(audit.errors, error_msg)
+    end
+  end
+
+  matcher :warn_with do |warning_msg|
+    match do |audit|
+      include_msg?(audit.warnings, warning_msg)
+    end
+  end
+
+  def include_msg?(messages, msg)
+    if msg.is_a?(Regexp)
+      Array(messages).any? { |m| m =~ msg }
+    else
+      Array(messages).include?(msg)
+    end
+  end
+end