From d8a5e467e82ac642deb46cb22602c3ebd80c8c57 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Thu, 7 Jan 2021 13:31:14 +0000 Subject: [PATCH] audit: quieten down and make casks audit consistent with formulae. The current casks audit is very noisy in the no-op case (i.e. no errors) https://github.com/Homebrew/brew/pull/10234/checks?check_run_id=1655630568#step:15:7 This means when there are errors and you're querying all casks it's pretty hard to quickly identify the problems. This commit silences the `passing`, `warning` and header/summary output when you're querying all casks (rather than a specific cask or tap). This is more consistent with `brew audit` for formulae which is silent unless there are audit failures. --- Library/Homebrew/cask/auditor.rb | 18 +++++++++++++++--- Library/Homebrew/cask/cmd/audit.rb | 4 ++++ Library/Homebrew/dev-cmd/audit.rb | 3 +++ Library/Homebrew/test/cask/cmd/audit_spec.rb | 20 ++++++++++---------- 4 files changed, 32 insertions(+), 13 deletions(-) diff --git a/Library/Homebrew/cask/auditor.rb b/Library/Homebrew/cask/auditor.rb index a201c4fe30..dd86237016 100644 --- a/Library/Homebrew/cask/auditor.rb +++ b/Library/Homebrew/cask/auditor.rb @@ -17,6 +17,7 @@ module Cask audit_strict: nil, audit_token_conflicts: nil, quarantine: nil, + any_named_args: nil, language: nil ) new( @@ -28,6 +29,7 @@ module Cask audit_strict: audit_strict, audit_token_conflicts: audit_token_conflicts, quarantine: quarantine, + any_named_args: any_named_args, language: language, ).audit end @@ -43,6 +45,7 @@ module Cask audit_token_conflicts: nil, audit_new_cask: nil, quarantine: nil, + any_named_args: nil, language: nil ) @cask = cask @@ -53,6 +56,7 @@ module Cask @audit_strict = audit_strict @quarantine = quarantine @audit_token_conflicts = audit_token_conflicts + @any_named_args = any_named_args @language = language end @@ -63,13 +67,13 @@ module Cask if !language && language_blocks language_blocks.each_key do |l| audit = audit_languages(l) - puts audit.summary + puts audit.summary if output_summary?(audit) warnings += audit.warnings errors += audit.errors end else audit = audit_cask_instance(cask) - puts audit.summary + puts audit.summary if output_summary?(audit) warnings += audit.warnings errors += audit.errors end @@ -79,8 +83,16 @@ module Cask private + def output_summary?(audit = nil) + return true if @any_named_args.present? + return true if @audit_strict.present? + return false if audit.blank? + + audit.errors? + end + def audit_languages(languages) - ohai "Auditing language: #{languages.map { |lang| "'#{lang}'" }.to_sentence}" + ohai "Auditing language: #{languages.map { |lang| "'#{lang}'" }.to_sentence}" if output_summary? original_config = cask.config localized_config = original_config.merge(Config.new(explicit: { languages: languages })) diff --git a/Library/Homebrew/cask/cmd/audit.rb b/Library/Homebrew/cask/cmd/audit.rb index 864625f588..acb72e1387 100644 --- a/Library/Homebrew/cask/cmd/audit.rb +++ b/Library/Homebrew/cask/cmd/audit.rb @@ -49,6 +49,7 @@ module Cask name end casks = casks.map { |c| CaskLoader.load(c, config: Config.from_args(args)) } + any_named_args = casks.any? casks = Cask.to_a if casks.empty? results = self.class.audit_casks( @@ -60,6 +61,7 @@ module Cask new_cask: args.new_cask?, token_conflicts: args.token_conflicts?, quarantine: args.quarantine?, + any_named_args: any_named_args, language: args.language, ) @@ -80,6 +82,7 @@ module Cask new_cask: nil, token_conflicts: nil, quarantine: nil, + any_named_args: nil, language: nil ) options = { @@ -91,6 +94,7 @@ module Cask audit_token_conflicts: token_conflicts, quarantine: quarantine, language: language, + any_named_args: any_named_args, }.compact options[:quarantine] = true if options[:quarantine].nil? diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index 47e0f2ba9a..264239680d 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/Library/Homebrew/dev-cmd/audit.rb @@ -109,6 +109,7 @@ module Homebrew online = new_formula || args.online? git = args.git? skip_style = args.skip_style? || args.no_named? || args.tap + no_named_args = false ENV.activate_extensions! ENV.setup_build_environment @@ -121,6 +122,7 @@ module Homebrew ] end elsif args.no_named? + no_named_args = true [Formula, Cask::Cask.to_a] else args.named.to_formulae_and_casks @@ -222,6 +224,7 @@ module Homebrew new_cask: args.new_cask?, token_conflicts: args.token_conflicts?, quarantine: nil, + any_named_args: !no_named_args, language: nil, ) end diff --git a/Library/Homebrew/test/cask/cmd/audit_spec.rb b/Library/Homebrew/test/cask/cmd/audit_spec.rb index 2a1bd90fc3..1357e066b3 100644 --- a/Library/Homebrew/test/cask/cmd/audit_spec.rb +++ b/Library/Homebrew/test/cask/cmd/audit_spec.rb @@ -24,7 +24,7 @@ describe Cask::Cmd::Audit, :cask do expect(Cask::CaskLoader).to receive(:load).with(cask_token, any_args).and_return(cask) expect(Cask::Auditor).to receive(:audit) - .with(cask, quarantine: true) + .with(cask, quarantine: true, any_named_args: true) .and_return(result) described_class.run(cask_token) @@ -34,7 +34,7 @@ describe Cask::Cmd::Audit, :cask do it "does not pass anything if no flags are specified" do allow(Cask::CaskLoader).to receive(:load).and_return(cask) expect(Cask::Auditor).to receive(:audit) - .with(cask, quarantine: true) + .with(cask, quarantine: true, any_named_args: true) .and_return(result) described_class.run("casktoken") @@ -43,7 +43,7 @@ describe Cask::Cmd::Audit, :cask do it "passes `audit_download` if the `--download` flag is specified" do allow(Cask::CaskLoader).to receive(:load).and_return(cask) expect(Cask::Auditor).to receive(:audit) - .with(cask, audit_download: true, quarantine: true) + .with(cask, audit_download: true, quarantine: true, any_named_args: true) .and_return(result) described_class.run("casktoken", "--download") @@ -52,7 +52,7 @@ describe Cask::Cmd::Audit, :cask do it "passes `audit_token_conflicts` if the `--token-conflicts` flag is specified" do allow(Cask::CaskLoader).to receive(:load).and_return(cask) expect(Cask::Auditor).to receive(:audit) - .with(cask, audit_token_conflicts: true, quarantine: true) + .with(cask, audit_token_conflicts: true, quarantine: true, any_named_args: true) .and_return(result) described_class.run("casktoken", "--token-conflicts") @@ -61,7 +61,7 @@ describe Cask::Cmd::Audit, :cask do it "passes `audit_strict` if the `--strict` flag is specified" do allow(Cask::CaskLoader).to receive(:load).and_return(cask) expect(Cask::Auditor).to receive(:audit) - .with(cask, audit_strict: true, quarantine: true) + .with(cask, audit_strict: true, quarantine: true, any_named_args: true) .and_return(result) described_class.run("casktoken", "--strict") @@ -70,7 +70,7 @@ describe Cask::Cmd::Audit, :cask do it "passes `audit_online` if the `--online` flag is specified" do allow(Cask::CaskLoader).to receive(:load).and_return(cask) expect(Cask::Auditor).to receive(:audit) - .with(cask, audit_online: true, quarantine: true) + .with(cask, audit_online: true, quarantine: true, any_named_args: true) .and_return(result) described_class.run("casktoken", "--online") @@ -79,7 +79,7 @@ describe Cask::Cmd::Audit, :cask do it "passes `audit_new_cask` if the `--new-cask` flag is specified" do allow(Cask::CaskLoader).to receive(:load).and_return(cask) expect(Cask::Auditor).to receive(:audit) - .with(cask, audit_new_cask: true, quarantine: true) + .with(cask, audit_new_cask: true, quarantine: true, any_named_args: true) .and_return(result) described_class.run("casktoken", "--new-cask") @@ -88,7 +88,7 @@ describe Cask::Cmd::Audit, :cask do it "passes `language` if the `--language` flag is specified" do allow(Cask::CaskLoader).to receive(:load).and_return(cask) expect(Cask::Auditor).to receive(:audit) - .with(cask, quarantine: true, language: ["de-AT"]) + .with(cask, quarantine: true, language: ["de-AT"], any_named_args: true) .and_return(result) described_class.run("casktoken", "--language=de-AT") @@ -97,7 +97,7 @@ describe Cask::Cmd::Audit, :cask do it "passes `quarantine` if the `--no-quarantine` flag is specified" do allow(Cask::CaskLoader).to receive(:load).and_return(cask) expect(Cask::Auditor).to receive(:audit) - .with(cask, quarantine: false) + .with(cask, quarantine: false, any_named_args: true) .and_return(result) described_class.run("casktoken", "--no-quarantine") @@ -108,7 +108,7 @@ describe Cask::Cmd::Audit, :cask do allow(Cask::CaskLoader).to receive(:load).and_return(cask) expect(Cask::Auditor).to receive(:audit) - .with(cask, quarantine: false) + .with(cask, quarantine: false, any_named_args: true) .and_return(result) described_class.run("casktoken")