Turn cask warnings into errors when --strict is passed (or implied)
- Ignore them and don't show them otherwise. - Part three of issue 15074: > As a result, I propose that all current cask audit warnings are never > displayed as warnings but the underlying audit checks turned into > errors displayed only with --strict (or one of the other relevant > flags).
This commit is contained in:
Issy Long
parent
a4e8f9e22b
commit
2b8127d518
@ -71,23 +71,14 @@ module Cask
|
||||
@errors ||= []
|
||||
end
|
||||
|
||||
def warnings
|
||||
@warnings ||= []
|
||||
end
|
||||
|
||||
sig { returns(T::Boolean) }
|
||||
def errors?
|
||||
errors.any?
|
||||
end
|
||||
|
||||
sig { returns(T::Boolean) }
|
||||
def warnings?
|
||||
warnings.any?
|
||||
end
|
||||
|
||||
sig { returns(T::Boolean) }
|
||||
def success?
|
||||
!(errors? || warnings?)
|
||||
!errors?
|
||||
end
|
||||
|
||||
sig { params(message: T.nilable(String), location: T.nilable(String)).void }
|
||||
@ -97,25 +88,17 @@ module Cask
|
||||
|
||||
sig { params(message: T.nilable(String), location: T.nilable(String)).void }
|
||||
def add_warning(message, location: nil)
|
||||
if strict?
|
||||
add_error message, location: location
|
||||
else
|
||||
warnings << ({ message: message, location: location })
|
||||
end
|
||||
# Warnings are ignored unless `--strict` is passed in which case they're turned into errors.
|
||||
add_error(message, location: location) if strict?
|
||||
end
|
||||
|
||||
def result
|
||||
if errors?
|
||||
Formatter.error("failed")
|
||||
elsif warnings?
|
||||
Formatter.warning("warning")
|
||||
end
|
||||
Formatter.error("failed") if errors?
|
||||
end
|
||||
|
||||
sig { params(include_warnings: T::Boolean).returns(T.nilable(String)) }
|
||||
def summary(include_warnings: true)
|
||||
sig { returns(T.nilable(String)) }
|
||||
def summary
|
||||
return if success?
|
||||
return if warnings? && !errors? && !include_warnings
|
||||
|
||||
summary = ["audit for #{cask}: #{result}"]
|
||||
|
||||
@ -123,12 +106,6 @@ module Cask
|
||||
summary << " #{Formatter.error("-")} #{error[:message]}"
|
||||
end
|
||||
|
||||
if include_warnings
|
||||
warnings.each do |warning|
|
||||
summary << " #{Formatter.warning("-")} #{warning[:message]}"
|
||||
end
|
||||
end
|
||||
|
||||
summary.join("\n")
|
||||
end
|
||||
|
||||
|
||||
@ -45,7 +45,6 @@ module Cask
|
||||
LANGUAGE_BLOCK_LIMIT = 10
|
||||
|
||||
def audit
|
||||
warnings = Set.new
|
||||
errors = Set.new
|
||||
|
||||
if !language && language_blocks
|
||||
@ -59,23 +58,19 @@ module Cask
|
||||
|
||||
sample_languages.each_key do |l|
|
||||
audit = audit_languages(l)
|
||||
summary = audit.summary(include_warnings: output_warnings?)
|
||||
if summary.present? && output_summary?(audit)
|
||||
if audit.summary.present? && output_summary?(audit)
|
||||
ohai "Auditing language: #{l.map { |lang| "'#{lang}'" }.to_sentence}" if output_summary?
|
||||
puts summary
|
||||
puts audit.summary
|
||||
end
|
||||
warnings += audit.warnings
|
||||
errors += audit.errors
|
||||
end
|
||||
else
|
||||
audit = audit_cask_instance(cask)
|
||||
summary = audit.summary(include_warnings: output_warnings?)
|
||||
puts summary if summary.present? && output_summary?(audit)
|
||||
warnings += audit.warnings
|
||||
puts audit.summary if audit.summary.present? && output_summary?(audit)
|
||||
errors += audit.errors
|
||||
end
|
||||
|
||||
{ warnings: warnings, errors: errors }
|
||||
{ errors: errors }
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
@ -90,7 +90,6 @@ module Cask
|
||||
quarantine: quarantine,
|
||||
language: language,
|
||||
any_named_args: any_named_args,
|
||||
display_failures_only: display_failures_only,
|
||||
only: only,
|
||||
except: except,
|
||||
}.compact
|
||||
|
||||
@ -13,23 +13,14 @@ describe Cask::Audit, :cask do
|
||||
end
|
||||
|
||||
def passed?(audit)
|
||||
!audit.errors? && !audit.warnings?
|
||||
!audit.errors?
|
||||
end
|
||||
|
||||
def outcome(audit)
|
||||
if passed?(audit)
|
||||
"passed"
|
||||
else
|
||||
message = ""
|
||||
|
||||
message += "warned with #{audit.warnings.map { |e| e.fetch(:message).inspect }.join(",")}" if audit.warnings?
|
||||
|
||||
if audit.errors?
|
||||
message += " and " if audit.warnings?
|
||||
message += "errored with #{audit.errors.map { |e| e.fetch(:message).inspect }.join(",")}"
|
||||
end
|
||||
|
||||
message
|
||||
"errored with #{audit.errors.map { |e| e.fetch(:message).inspect }.join(",")}"
|
||||
end
|
||||
end
|
||||
|
||||
@ -55,7 +46,7 @@ describe Cask::Audit, :cask do
|
||||
|
||||
matcher :warn_with do |message|
|
||||
match do |audit|
|
||||
include_msg?(audit.warnings, message)
|
||||
include_msg?(audit.errors, message)
|
||||
end
|
||||
|
||||
failure_message do |audit|
|
||||
@ -118,6 +109,14 @@ describe Cask::Audit, :cask do
|
||||
describe "#result" do
|
||||
subject { audit.result }
|
||||
|
||||
context "when there are no errors and `--strict` is not passed so we should not show anything" do
|
||||
before do
|
||||
audit.add_warning "eh"
|
||||
end
|
||||
|
||||
it { is_expected.not_to match(/failed/) }
|
||||
end
|
||||
|
||||
context "when there are errors" do
|
||||
before do
|
||||
audit.add_error "bad"
|
||||
@ -126,14 +125,6 @@ describe Cask::Audit, :cask do
|
||||
it { is_expected.to match(/failed/) }
|
||||
end
|
||||
|
||||
context "when there are warnings" do
|
||||
before do
|
||||
audit.add_warning "eh"
|
||||
end
|
||||
|
||||
it { is_expected.to match(/warning/) }
|
||||
end
|
||||
|
||||
context "when there are errors and warnings" do
|
||||
before do
|
||||
audit.add_error "bad"
|
||||
@ -143,8 +134,33 @@ describe Cask::Audit, :cask do
|
||||
it { is_expected.to match(/failed/) }
|
||||
end
|
||||
|
||||
context "when there are no errors or warnings" do
|
||||
it { is_expected.to match(/passed/) }
|
||||
context "when there are errors and warnings and `--strict` is passed" do
|
||||
let(:strict) { true }
|
||||
|
||||
before do
|
||||
audit.add_error "very bad"
|
||||
audit.add_warning "a little bit bad"
|
||||
end
|
||||
|
||||
it { is_expected.to match(/failed/) }
|
||||
end
|
||||
|
||||
context "when there are warnings and `--strict` is not passed" do
|
||||
before do
|
||||
audit.add_warning "a little bit bad"
|
||||
end
|
||||
|
||||
it { is_expected.not_to match(/failed/) }
|
||||
end
|
||||
|
||||
context "when there are warnings and `--strict` is passed" do
|
||||
let(:strict) { true }
|
||||
|
||||
before do
|
||||
audit.add_warning "a little bit bad"
|
||||
end
|
||||
|
||||
it { is_expected.to match(/failed/) }
|
||||
end
|
||||
end
|
||||
|
||||
@ -984,12 +1000,23 @@ describe Cask::Audit, :cask do
|
||||
context "when cask token conflicts with a core formula" do
|
||||
let(:formula_names) { %w[with-binary other-formula] }
|
||||
|
||||
context "when `--strict` is passed" do
|
||||
let(:strict) { true }
|
||||
|
||||
it "warns about duplicates" do
|
||||
expect(audit).to receive(:core_formula_names).and_return(formula_names)
|
||||
expect(run).to warn_with(/possible duplicate/)
|
||||
end
|
||||
end
|
||||
|
||||
context "when `--strict` is not passed" do
|
||||
it "does not warn about duplicates" do
|
||||
expect(audit).to receive(:core_formula_names).and_return(formula_names)
|
||||
expect(run).not_to warn_with(/possible duplicate/)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context "when cask token does not conflict with a core formula" do
|
||||
let(:formula_names) { %w[other-formula] }
|
||||
|
||||
@ -1058,8 +1085,8 @@ describe Cask::Audit, :cask do
|
||||
context "when `new_cask` is false" do
|
||||
let(:new_cask) { false }
|
||||
|
||||
it "warns" do
|
||||
expect(run).to warn_with(/should have a description/)
|
||||
it "does not warn" do
|
||||
expect(run).not_to warn_with(/should have a description/)
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
@ -39,9 +39,7 @@ describe Cask::Quarantine, :cask do
|
||||
it "quarantines Cask audits" do
|
||||
expect do
|
||||
Cask::Cmd::Audit.run("local-transmission", "--download")
|
||||
end.to not_raise_error
|
||||
.and output(/audit for local-transmission: passed/).to_stdout
|
||||
.and not_to_output.to_stderr
|
||||
end.to not_raise_error.and not_to_output.to_stderr
|
||||
|
||||
local_transmission = Cask::CaskLoader.load(cask_path("local-transmission"))
|
||||
cached_location = Cask::Download.new(local_transmission).fetch
|
||||
@ -156,9 +154,7 @@ describe Cask::Quarantine, :cask do
|
||||
it "does not quarantine Cask audits" do
|
||||
expect do
|
||||
Cask::Cmd::Audit.run("local-transmission", "--download", "--no-quarantine")
|
||||
end.to not_raise_error
|
||||
.and output(/audit for local-transmission: passed/).to_stdout
|
||||
.and not_to_output.to_stderr
|
||||
end.to not_raise_error.and not_to_output.to_stderr
|
||||
|
||||
local_transmission = Cask::CaskLoader.load(cask_path("local-transmission"))
|
||||
cached_location = Cask::Download.new(local_transmission).fetch
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user