Merge pull request #10896 from Rylan12/cask-audit-display-changes

audit: limit non-failure cask output
2021-03-23 01:33:28 -04:00 · 2021-03-23 01:33:28 -04:00 · 9a355b07c3
commit 9a355b07c3
parent 2d3aca706d 665578438b
11 changed files with 104 additions and 44 deletions
--- a/.github/workflows/tests.yml
+++ b/.github/workflows/tests.yml
@ -77,7 +77,7 @@ jobs:
        run: brew style --display-cop-names homebrew/core

      - name: Run brew audit --skip-style on all taps
-        run: brew audit --skip-style
+        run: brew audit --skip-style --display-failures-only

      - name: Set up all Homebrew taps
        run: |
@ -277,10 +277,10 @@ jobs:

      - name: Run brew audit --skip-style on Cask taps
        run: |
-          brew audit --skip-style --tap=homebrew/cask
-          brew audit --skip-style --tap=homebrew/cask-drivers
-          brew audit --skip-style --tap=homebrew/cask-fonts
-          brew audit --skip-style --tap=homebrew/cask-versions
+          brew audit --skip-style --display-failures-only --tap=homebrew/cask
+          brew audit --skip-style --display-failures-only --tap=homebrew/cask-drivers
+          brew audit --skip-style --display-failures-only --tap=homebrew/cask-fonts
+          brew audit --skip-style --display-failures-only --tap=homebrew/cask-versions

      - name: Install brew tests dependencies
        run: |
--- a/Library/Homebrew/cask/audit.rb
+++ b/Library/Homebrew/cask/audit.rb
@ -126,16 +126,21 @@ module Cask
      end
    end

-    sig { returns(String) }
-    def summary
+    sig { params(include_passed: T::Boolean, include_warnings: T::Boolean).returns(String) }
+    def summary(include_passed: false, include_warnings: true)
+      return if success? && !include_passed
+      return if warnings? && !errors? && !include_warnings
+
      summary = ["audit for #{cask}: #{result}"]

      errors.each do |error|
        summary << " #{Formatter.error("-")} #{error}"
      end

-      warnings.each do |warning|
-        summary << " #{Formatter.warning("-")} #{warning}"
+      if include_warnings
+        warnings.each do |warning|
+          summary << " #{Formatter.warning("-")} #{warning}"
+        end
      end

      summary.join("\n")
--- a/Library/Homebrew/cask/auditor.rb
+++ b/Library/Homebrew/cask/auditor.rb
@ -18,7 +18,9 @@ module Cask
      audit_token_conflicts: nil,
      quarantine: nil,
      any_named_args: nil,
-      language: nil
+      language: nil,
+      display_passes: nil,
+      display_failures_only: nil
    )
      new(
        cask,
@ -31,6 +33,8 @@ module Cask
        quarantine:            quarantine,
        any_named_args:        any_named_args,
        language:              language,
+        display_passes:        display_passes,
+        display_failures_only: display_failures_only,
      ).audit
    end

@ -46,7 +50,9 @@ module Cask
      audit_new_cask: nil,
      quarantine: nil,
      any_named_args: nil,
-      language: nil
+      language: nil,
+      display_passes: nil,
+      display_failures_only: nil
    )
      @cask = cask
      @audit_download = audit_download
@ -58,6 +64,8 @@ module Cask
      @audit_token_conflicts = audit_token_conflicts
      @any_named_args = any_named_args
      @language = language
+      @display_passes = display_passes
+      @display_failures_only = display_failures_only
    end

    def audit
@ -67,13 +75,18 @@ module Cask
      if !language && language_blocks
        language_blocks.each_key do |l|
          audit = audit_languages(l)
-          puts audit.summary if output_summary?(audit)
+          summary = audit.summary(include_passed: output_passed?, include_warnings: output_warnings?)
+          if summary.present? && output_summary?(audit)
+            ohai "Auditing language: #{l.map { |lang| "'#{lang}'" }.to_sentence}" if output_summary?
+            puts summary
+          end
          warnings += audit.warnings
          errors += audit.errors
        end
      else
        audit = audit_cask_instance(cask)
-        puts audit.summary if output_summary?(audit)
+        summary = audit.summary(include_passed: output_passed?, include_warnings: output_warnings?)
+        puts summary if summary.present? && output_summary?(audit)
        warnings += audit.warnings
        errors += audit.errors
      end
@ -91,9 +104,20 @@ module Cask
      audit.errors?
    end

-    def audit_languages(languages)
-      ohai "Auditing language: #{languages.map { |lang| "'#{lang}'" }.to_sentence}" if output_summary?
+    def output_passed?
+      return false if @display_failures_only.present?
+      return true if @display_passes.present?

+      false
+    end
+
+    def output_warnings?
+      return false if @display_failures_only.present?
+
+      true
+    end
+
+    def audit_languages(languages)
      original_config = cask.config
      localized_config = original_config.merge(Config.new(explicit: { languages: languages }))
      cask.config = localized_config
--- a/Library/Homebrew/cask/cmd/audit.rb
+++ b/Library/Homebrew/cask/cmd/audit.rb
@ -27,6 +27,8 @@ module Cask
                 description: "Run various additional style checks to determine if a new cask is eligible " \
                              "for Homebrew. This should be used when creating new casks and implies " \
                              "`--strict` and `--online`"
+          switch "--display-failures-only",
+                 description: "Only display casks that fail the audit. This is the default for formulae."
        end
      end

@ -44,15 +46,17 @@ module Cask

        results = self.class.audit_casks(
          *casks,
-          download:        args.download?,
-          appcast:         args.appcast?,
-          online:          args.online?,
-          strict:          args.strict?,
-          new_cask:        args.new_cask?,
-          token_conflicts: args.token_conflicts?,
-          quarantine:      args.quarantine?,
-          any_named_args:  any_named_args,
-          language:        args.language,
+          download:              args.download?,
+          appcast:               args.appcast?,
+          online:                args.online?,
+          strict:                args.strict?,
+          new_cask:              args.new_cask?,
+          token_conflicts:       args.token_conflicts?,
+          quarantine:            args.quarantine?,
+          any_named_args:        any_named_args,
+          language:              args.language,
+          display_passes:        args.verbose? || args.named.count == 1,
+          display_failures_only: args.display_failures_only?,
        )

        self.class.print_annotations(results)
@ -73,7 +77,9 @@ module Cask
        token_conflicts: nil,
        quarantine: nil,
        any_named_args: nil,
-        language: nil
+        language: nil,
+        display_passes: nil,
+        display_failures_only: nil
      )
        options = {
          audit_download:        download,
@ -85,6 +91,8 @@ module Cask
          quarantine:            quarantine,
          language:              language,
          any_named_args:        any_named_args,
+          display_passes:        display_passes,
+          display_failures_only: display_failures_only,
        }.compact

        options[:quarantine] = true if options[:quarantine].nil?
--- a/Library/Homebrew/dev-cmd/audit.rb
+++ b/Library/Homebrew/dev-cmd/audit.rb
@ -56,6 +56,8 @@ module Homebrew
      switch "--display-filename",
             description: "Prefix every line of output with the file or formula name being audited, to "\
                          "make output easy to grep."
+      switch "--display-failures-only",
+             description: "Only display casks that fail the audit. This is the default for formulae."
      switch "--skip-style",
             description: "Skip running non-RuboCop style checks. Useful if you plan on running "\
                          "`brew style` separately. Enabled by default unless a formula is specified by name."
@ -213,15 +215,17 @@ module Homebrew

      Cask::Cmd::Audit.audit_casks(
        *audit_casks,
-        download:        nil,
-        appcast:         args.appcast?,
-        online:          args.online?,
-        strict:          args.strict?,
-        new_cask:        args.new_cask?,
-        token_conflicts: args.token_conflicts?,
-        quarantine:      nil,
-        any_named_args:  !no_named_args,
-        language:        nil,
+        download:              nil,
+        appcast:               args.appcast?,
+        online:                args.online?,
+        strict:                args.strict?,
+        new_cask:              args.new_cask?,
+        token_conflicts:       args.token_conflicts?,
+        quarantine:            nil,
+        any_named_args:        !no_named_args,
+        language:              nil,
+        display_passes:        args.verbose? || args.named.count == 1,
+        display_failures_only: args.display_failures_only?,
      )
    end

--- a/Library/Homebrew/test/cask/cmd/audit_spec.rb
+++ b/Library/Homebrew/test/cask/cmd/audit_spec.rb
@ -21,7 +21,8 @@ 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, audit_new_cask: false, quarantine: true, any_named_args: true)
+        .with(cask, audit_new_cask: false, quarantine: true, any_named_args: true,
+              display_failures_only: false, display_passes: true)
        .and_return(result)

      described_class.run(cask_token)
@ -31,7 +32,8 @@ 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, audit_new_cask: false, quarantine: true, any_named_args: true)
+      .with(cask, audit_new_cask: false, quarantine: true, any_named_args: true,
+            display_failures_only: false, display_passes: true)
      .and_return(result)

    described_class.run("casktoken")
@ -40,7 +42,8 @@ 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, audit_new_cask: false, quarantine: true, any_named_args: true)
+      .with(cask, audit_download: true, audit_new_cask: false, quarantine: true, any_named_args: true,
+            display_failures_only: false, display_passes: true)
      .and_return(result)

    described_class.run("casktoken", "--download")
@ -49,7 +52,8 @@ 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, audit_new_cask: false, quarantine: true, any_named_args: true)
+      .with(cask, audit_token_conflicts: true, audit_new_cask: false, quarantine: true, any_named_args: true,
+            display_failures_only: false, display_passes: true)
      .and_return(result)

    described_class.run("casktoken", "--token-conflicts")
@ -58,7 +62,8 @@ 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, audit_new_cask: false, quarantine: true, any_named_args: true)
+      .with(cask, audit_strict: true, audit_new_cask: false, quarantine: true, any_named_args: true,
+            display_failures_only: false, display_passes: true)
      .and_return(result)

    described_class.run("casktoken", "--strict")
@ -67,7 +72,8 @@ 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, audit_new_cask: false, quarantine: true, any_named_args: true)
+      .with(cask, audit_online: true, audit_new_cask: false, quarantine: true, any_named_args: true,
+            display_failures_only: false, display_passes: true)
      .and_return(result)

    described_class.run("casktoken", "--online")
@ -76,7 +82,8 @@ 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, any_named_args: true)
+      .with(cask, audit_new_cask: true, quarantine: true, any_named_args: true,
+            display_failures_only: false, display_passes: true)
      .and_return(result)

    described_class.run("casktoken", "--new-cask")
@ -85,7 +92,8 @@ 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, audit_new_cask: false, quarantine: true, language: ["de-AT"], any_named_args: true)
+      .with(cask, audit_new_cask: false, quarantine: true, language: ["de-AT"], any_named_args: true,
+            display_failures_only: false, display_passes: true)
      .and_return(result)

    described_class.run("casktoken", "--language=de-AT")
@ -94,7 +102,8 @@ 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, audit_new_cask: false, quarantine: false, any_named_args: true)
+      .with(cask, audit_new_cask: false, quarantine: false, any_named_args: true,
+            display_failures_only: false, display_passes: true)
      .and_return(result)

    described_class.run("casktoken", "--no-quarantine")
@ -105,7 +114,8 @@ describe Cask::Cmd::Audit, :cask do

    allow(Cask::CaskLoader).to receive(:load).and_return(cask)
    expect(Cask::Auditor).to receive(:audit)
-      .with(cask, audit_new_cask: false, quarantine: false, any_named_args: true)
+      .with(cask, audit_new_cask: false, quarantine: false, any_named_args: true,
+            display_failures_only: false, display_passes: true)
      .and_return(result)

    described_class.run("casktoken")
--- a/completions/bash/brew
+++ b/completions/bash/brew
@ -324,6 +324,7 @@ _brew_audit() {
      --cask
      --debug
      --display-cop-names
+      --display-failures-only
      --display-filename
      --except
      --except-cops
--- a/completions/fish/brew.fish
+++ b/completions/fish/brew.fish
@ -325,6 +325,7 @@ __fish_brew_complete_arg 'audit' -l audit-debug -d 'Enable debugging and profili
 __fish_brew_complete_arg 'audit' -l cask -d 'Treat all named arguments as casks'
 __fish_brew_complete_arg 'audit' -l debug -d 'Display any debugging information'
 __fish_brew_complete_arg 'audit' -l display-cop-names -d 'Include the RuboCop cop name for each violation in the output'
+__fish_brew_complete_arg 'audit' -l display-failures-only -d 'Only display casks that fail the audit. This is the default for formulae'
 __fish_brew_complete_arg 'audit' -l display-filename -d 'Prefix every line of output with the file or formula name being audited, to make output easy to grep'
 __fish_brew_complete_arg 'audit' -l except -d 'Specify a comma-separated method list to skip running the methods named `audit_`method'
 __fish_brew_complete_arg 'audit' -l except-cops -d 'Specify a comma-separated cops list to skip checking for violations of the listed RuboCop cops'
--- a/completions/zsh/_brew
+++ b/completions/zsh/_brew
@ -406,6 +406,7 @@ _brew_audit() {
    '--audit-debug[Enable debugging and profiling of audit methods]' \
    '--debug[Display any debugging information]' \
    '(--skip-style --only-cops --except-cops)--display-cop-names[Include the RuboCop cop name for each violation in the output]' \
+    '--display-failures-only[Only display casks that fail the audit. This is the default for formulae]' \
    '--display-filename[Prefix every line of output with the file or formula name being audited, to make output easy to grep]' \
    '(--only)--except[Specify a comma-separated method list to skip running the methods named `audit_`method]' \
    '(--only-cops --strict --only-cops --only --display-cop-names)--except-cops[Specify a comma-separated cops list to skip checking for violations of the listed RuboCop cops]' \
--- a/docs/Manpage.md
+++ b/docs/Manpage.md
@ -780,6 +780,8 @@ non-zero status if any errors are found.
  Include the RuboCop cop name for each violation in the output.
 * `--display-filename`:
  Prefix every line of output with the file or formula name being audited, to make output easy to grep.
+* `--display-failures-only`:
+  Only display casks that fail the audit. This is the default for formulae.
 * `--skip-style`:
  Skip running non-RuboCop style checks. Useful if you plan on running `brew style` separately. Enabled by default unless a formula is specified by name.
 * `-D`, `--audit-debug`:
--- a/manpages/brew.1
+++ b/manpages/brew.1
@ -1062,6 +1062,10 @@ Include the RuboCop cop name for each violation in the output\.
 Prefix every line of output with the file or formula name being audited, to make output easy to grep\.
 .
 .TP
+\fB\-\-display\-failures\-only\fR
+Only display casks that fail the audit\. This is the default for formulae\.
+.
+.TP
 \fB\-\-skip\-style\fR
 Skip running non\-RuboCop style checks\. Useful if you plan on running \fBbrew style\fR separately\. Enabled by default unless a formula is specified by name\.
 .