Merge pull request #8115 from reitermarkus/cask-audit

Make `Cask::Auditor` return a `Hash` with warnings and errors.

Library/Homebrew/cask/all.rb

@@ -9,7 +9,6 @@ require "cask/cache"
 require "cask/cask"
 require "cask/cask_loader"
 require "cask/caskroom"
-require "cask/checkable"
 require "cask/cmd"
 require "cask/exceptions"
 require "cask/installer"

Library/Homebrew/cask/audit.rb

@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require "cask/denylist"
-require "cask/checkable"
 require "cask/download"
 require "digest"
 require "utils/curl"
@@ -10,7 +9,6 @@ require "utils/notability"
 
 module Cask
   class Audit
-    include Checkable
     extend Predicable
 
     attr_reader :cask, :commit_range, :download
@@ -62,12 +60,56 @@ module Cask
       self
     end
 
-    def success?
+    def errors
-      !(errors? || warnings?)
+      @errors ||= []
     end
 
-    def summary_header
+    def warnings
-      "audit for #{cask}"
+      @warnings ||= []
+    end
+
+    def add_error(message)
+      errors << message
+    end
+
+    def add_warning(message)
+      warnings << message
+    end
+
+    def errors?
+      errors.any?
+    end
+
+    def warnings?
+      warnings.any?
+    end
+
+    def result
+      if errors?
+        Formatter.error("failed")
+      elsif warnings?
+        Formatter.warning("warning")
+      else
+        Formatter.success("passed")
+      end
+    end
+
+    def summary
+      summary = ["audit for #{cask}: #{result}"]
+
+      errors.each do |error|
+        summary << " #{Formatter.error("-")} #{error}"
+      end
+
+      warnings.each do |warning|
+        summary << " #{Formatter.warning("-")} #{warning}"
+      end
+
+      summary.join("\n")
+    end
+
+    def success?
+      !(errors? || warnings?)
     end
 
     private

Library/Homebrew/cask/auditor.rb

@@ -2,7 +2,6 @@
 
 module Cask
   class Auditor
-    include Checkable
     extend Predicable
 
     def self.audit(cask, audit_download: false, audit_appcast: false,
@@ -39,19 +38,28 @@ module Cask
                    :audit_strict?, :audit_new_cask?, :audit_token_conflicts?, :quarantine?
 
     def audit
+      warnings = Set.new
+      errors = Set.new
+
       if !Homebrew.args.value("language") && language_blocks
-        audit_all_languages
+        language_blocks.each_key do |l|
+          audit = audit_languages(l)
+          puts audit.summary
+          warnings += audit.warnings
+          errors += audit.errors
+        end
       else
-        audit_cask_instance(cask)
+        audit = audit_cask_instance(cask)
+        puts audit.summary
+        warnings += audit.warnings
+        errors += audit.errors
       end
+
+      { warnings: warnings, errors: errors }
     end
 
     private
 
-    def audit_all_languages
-      language_blocks.keys.all?(&method(:audit_languages))
-    end
-
     def audit_languages(languages)
       ohai "Auditing language: #{languages.map { |lang| "'#{lang}'" }.to_sentence}"
       localized_cask = CaskLoader.load(cask.sourcefile_path)
@@ -71,8 +79,7 @@ module Cask
                               quarantine:      quarantine?,
                               commit_range:    commit_range)
       audit.run!
-      puts audit.summary
+      audit
-      audit.success?
     end
 
     def language_blocks

Library/Homebrew/cask/checkable.rb

@@ -1,53 +0,0 @@
-# frozen_string_literal: true
-
-module Cask
-  module Checkable
-    def errors
-      @errors ||= []
-    end
-
-    def warnings
-      @warnings ||= []
-    end
-
-    def add_error(message)
-      errors << message
-    end
-
-    def add_warning(message)
-      warnings << message
-    end
-
-    def errors?
-      errors.any?
-    end
-
-    def warnings?
-      warnings.any?
-    end
-
-    def result
-      if errors?
-        Formatter.error("failed")
-      elsif warnings?
-        Formatter.warning("warning")
-      else
-        Formatter.success("passed")
-      end
-    end
-
-    def summary
-      summary = ["#{summary_header}: #{result}"]
-
-      errors.each do |error|
-        summary << " #{Formatter.error("-")} #{error}"
-      end
-
-      warnings.each do |warning|
-        summary << " #{Formatter.warning("-")} #{warning}"
-      end
-
-      summary.join("\n")
-    end
-  end
-end

Library/Homebrew/cask/cmd/audit.rb

@@ -48,13 +48,15 @@ module Cask
         failed_casks = casks(alternative: -> { Cask.to_a })
                        .reject do |cask|
           odebug "Auditing Cask #{cask}"
-          Auditor.audit(cask, audit_download:        download,
+          result = Auditor.audit(cask, audit_download:        download,
-                              audit_appcast:         appcast,
+                                       audit_appcast:         appcast,
-                              audit_online:          online,
+                                       audit_online:          online,
-                              audit_strict:          strict,
+                                       audit_strict:          strict,
-                              audit_new_cask:        new_cask_arg?,
+                                       audit_new_cask:        new_cask_arg?,
-                              audit_token_conflicts: token_conflicts,
+                                       audit_token_conflicts: token_conflicts,
-                              quarantine:            quarantine?)
+                                       quarantine:            quarantine?)
+
+          result[:warnings].empty? && result[:errors].empty?
         end
 
         return if failed_casks.empty?

Library/Homebrew/cask/cmd/doctor.rb

@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require "system_config"
-require "cask/checkable"
 require "diagnostic"
 
 module Cask

Library/Homebrew/sorbet/files.yaml

@@ -39,7 +39,6 @@ false:
   - ./cask/cask.rb
   - ./cask/cask_loader.rb
   - ./cask/caskroom.rb
-  - ./cask/checkable.rb
   - ./cask/cmd.rb
   - ./cask/cmd/--cache.rb
   - ./cask/cmd/abstract_command.rb

Library/Homebrew/test/cask/cmd/audit_spec.rb

@@ -4,6 +4,7 @@ require_relative "shared_examples/invalid_option"
 
 describe Cask::Cmd::Audit, :cask do
   let(:cask) { Cask::Cask.new("cask") }
+  let(:result) { { warnings: Set.new, errors: Set.new } }
 
   it_behaves_like "a command that handles invalid options"
 
@@ -11,7 +12,7 @@ describe Cask::Cmd::Audit, :cask do
     it "audits all Casks if no tokens are given" do
       allow(Cask::Cask).to receive(:to_a).and_return([cask, cask])
 
-      expect(Cask::Auditor).to receive(:audit).twice.and_return(true)
+      expect(Cask::Auditor).to receive(:audit).twice.and_return(result)
 
       described_class.run
     end
@@ -28,7 +29,7 @@ describe Cask::Cmd::Audit, :cask do
                     audit_online:          false,
                     audit_strict:          false,
                     quarantine:            true)
-        .and_return(true)
+        .and_return(result)
 
       described_class.run(cask_token)
     end
@@ -45,7 +46,7 @@ describe Cask::Cmd::Audit, :cask do
                     audit_online:          false,
                     audit_strict:          false,
                     quarantine:            true)
-        .and_return(true)
+        .and_return(result)
 
       described_class.run("casktoken")
     end
@@ -60,7 +61,7 @@ describe Cask::Cmd::Audit, :cask do
                     audit_online:          false,
                     audit_strict:          false,
                     quarantine:            true)
-        .and_return(true)
+        .and_return(result)
 
       described_class.run("casktoken", "--download")
     end
@@ -77,7 +78,7 @@ describe Cask::Cmd::Audit, :cask do
                     audit_online:          false,
                     audit_strict:          false,
                     quarantine:            true)
-        .and_return(true)
+        .and_return(result)
 
       described_class.run("casktoken")
     end
@@ -92,7 +93,7 @@ describe Cask::Cmd::Audit, :cask do
                     audit_online:          false,
                     audit_strict:          false,
                     quarantine:            true)
-        .and_return(true)
+        .and_return(result)
 
       described_class.run("casktoken", "--token-conflicts")
     end
@@ -109,7 +110,7 @@ describe Cask::Cmd::Audit, :cask do
                     audit_online:          false,
                     audit_strict:          false,
                     quarantine:            true)
-        .and_return(true)
+        .and_return(result)
 
       described_class.run("casktoken")
     end
@@ -124,7 +125,7 @@ describe Cask::Cmd::Audit, :cask do
                     audit_online:          false,
                     audit_strict:          true,
                     quarantine:            true)
-        .and_return(true)
+        .and_return(result)
 
       described_class.run("casktoken", "--strict")
     end
@@ -141,7 +142,7 @@ describe Cask::Cmd::Audit, :cask do
                     audit_online:          false,
                     audit_strict:          false,
                     quarantine:            true)
-        .and_return(true)
+        .and_return(result)
 
       described_class.run("casktoken")
     end
@@ -156,7 +157,7 @@ describe Cask::Cmd::Audit, :cask do
                     audit_online:          true,
                     audit_strict:          false,
                     quarantine:            true)
-        .and_return(true)
+        .and_return(result)
 
       described_class.run("casktoken", "--online")
     end
@@ -173,7 +174,7 @@ describe Cask::Cmd::Audit, :cask do
                     audit_online:          false,
                     audit_strict:          false,
                     quarantine:            true)
-        .and_return(true)
+        .and_return(result)
 
       described_class.run("casktoken")
     end
@@ -188,7 +189,7 @@ describe Cask::Cmd::Audit, :cask do
                     audit_online:          true,
                     audit_strict:          true,
                     quarantine:            true)
-        .and_return(true)
+        .and_return(result)
 
       described_class.run("casktoken", "--new-cask")
     end

Library/Homebrew/test/cmd/reinstall_spec.rb

@@ -7,7 +7,7 @@ describe "Homebrew.reinstall_args" do
   it_behaves_like "parseable arguments"
 end
 
-describe "brew reinstall", :integration_test do
+describe "brew reinstall", :integration_test, timeout: 120 do
   it "reinstalls a Formula" do
     install_test_formula "testball"
     foo_dir = HOMEBREW_CELLAR/"testball/0.1/bin"

Library/Homebrew/test/dev-cmd/test_spec.rb

@@ -7,7 +7,7 @@ describe "Homebrew.test_args" do
 end
 
 # randomly segfaults on Linux with portable-ruby.
-describe "brew test", :integration_test, :needs_macos do
+describe "brew test", :integration_test, :needs_macos, timeout: 120 do
   it "tests a given Formula" do
     install_test_formula "testball", <<~'RUBY'
       test do