Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from Jul 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion Library/Homebrew/cask/all.rb
Expand Up @@ -9,7 +9,6 @@
require "cask/cask"
require "cask/cask_loader"
require "cask/caskroom"
require "cask/checkable"
require "cask/cmd"
require "cask/exceptions"
require "cask/installer"
Expand Down
54 changes: 48 additions & 6 deletions 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"
Expand All @@ -10,7 +9,6 @@

module Cask
class Audit
include Checkable
extend Predicable

attr_reader :cask, :commit_range, :download
Expand Down Expand Up @@ -62,12 +60,56 @@ def run!
self
end

def success?
!(errors? || warnings?)
def errors
@errors ||= []
end

def warnings
@warnings ||= []
end

def summary_header
"audit for #{cask}"
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
Expand Down
25 changes: 16 additions & 9 deletions Library/Homebrew/cask/auditor.rb
Expand Up @@ -2,7 +2,6 @@

module Cask
class Auditor
include Checkable
extend Predicable

def self.audit(cask, audit_download: false, audit_appcast: false,
Expand Down Expand Up @@ -39,19 +38,28 @@ def initialize(cask, audit_download: false, audit_appcast: false,
: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)
Expand All @@ -71,8 +79,7 @@ def audit_cask_instance(cask)
quarantine: quarantine?,
commit_range: commit_range)
audit.run!
puts audit.summary
audit.success?
audit
end

def language_blocks
Expand Down
53 changes: 0 additions & 53 deletions Library/Homebrew/cask/checkable.rb

This file was deleted.

16 changes: 9 additions & 7 deletions Library/Homebrew/cask/cmd/audit.rb
Expand Up @@ -48,13 +48,15 @@ def run
failed_casks = casks(alternative: -> { Cask.to_a })
.reject do |cask|
odebug "Auditing Cask #{cask}"
Auditor.audit(cask, audit_download: download,
audit_appcast: appcast,
audit_online: online,
audit_strict: strict,
audit_new_cask: new_cask_arg?,
audit_token_conflicts: token_conflicts,
quarantine: quarantine?)
result = Auditor.audit(cask, audit_download: download,
audit_appcast: appcast,
audit_online: online,
audit_strict: strict,
audit_new_cask: new_cask_arg?,
audit_token_conflicts: token_conflicts,
quarantine: quarantine?)

result[:warnings].empty? && result[:errors].empty?
end

return if failed_casks.empty?
Expand Down
1 change: 0 additions & 1 deletion Library/Homebrew/cask/cmd/doctor.rb
@@ -1,7 +1,6 @@
# frozen_string_literal: true

require "system_config"
require "cask/checkable"
require "diagnostic"

module Cask
Expand Down
1 change: 0 additions & 1 deletion Library/Homebrew/sorbet/files.yaml
Expand Up @@ -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
Expand Down
25 changes: 13 additions & 12 deletions Library/Homebrew/test/cask/cmd/audit_spec.rb
Expand Up @@ -4,14 +4,15 @@

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"

describe "selection of Casks to audit" 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
Expand All @@ -28,7 +29,7 @@
audit_online: false,
audit_strict: false,
quarantine: true)
.and_return(true)
.and_return(result)

described_class.run(cask_token)
end
Expand All @@ -45,7 +46,7 @@
audit_online: false,
audit_strict: false,
quarantine: true)
.and_return(true)
.and_return(result)

described_class.run("casktoken")
end
Expand All @@ -60,7 +61,7 @@
audit_online: false,
audit_strict: false,
quarantine: true)
.and_return(true)
.and_return(result)

described_class.run("casktoken", "--download")
end
Expand All @@ -77,7 +78,7 @@
audit_online: false,
audit_strict: false,
quarantine: true)
.and_return(true)
.and_return(result)

described_class.run("casktoken")
end
Expand All @@ -92,7 +93,7 @@
audit_online: false,
audit_strict: false,
quarantine: true)
.and_return(true)
.and_return(result)

described_class.run("casktoken", "--token-conflicts")
end
Expand All @@ -109,7 +110,7 @@
audit_online: false,
audit_strict: false,
quarantine: true)
.and_return(true)
.and_return(result)

described_class.run("casktoken")
end
Expand All @@ -124,7 +125,7 @@
audit_online: false,
audit_strict: true,
quarantine: true)
.and_return(true)
.and_return(result)

described_class.run("casktoken", "--strict")
end
Expand All @@ -141,7 +142,7 @@
audit_online: false,
audit_strict: false,
quarantine: true)
.and_return(true)
.and_return(result)

described_class.run("casktoken")
end
Expand All @@ -156,7 +157,7 @@
audit_online: true,
audit_strict: false,
quarantine: true)
.and_return(true)
.and_return(result)

described_class.run("casktoken", "--online")
end
Expand All @@ -173,7 +174,7 @@
audit_online: false,
audit_strict: false,
quarantine: true)
.and_return(true)
.and_return(result)

described_class.run("casktoken")
end
Expand All @@ -188,7 +189,7 @@
audit_online: true,
audit_strict: true,
quarantine: true)
.and_return(true)
.and_return(result)

described_class.run("casktoken", "--new-cask")
end
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/test/cmd/reinstall_spec.rb
Expand Up @@ -7,7 +7,7 @@
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"
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/test/dev-cmd/test_spec.rb
Expand Up @@ -7,7 +7,7 @@
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This definitely shouldn't be taking anywhere near the timeout? I wonder what's going on here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea, I saw it spuriously fail yesterday.

it "tests a given Formula" do
install_test_formula "testball", <<~'RUBY'
test do
Expand Down