Consolidate add_{warning,error}
methods into one
- Specify `strictish: true` in `add_error` to specify that it's not a super big critical error. - These will be shown only if `brew audit --strict` is requested.
This commit is contained in:
parent
2b8127d518
commit
df8e97fef6
@ -81,15 +81,12 @@ module Cask
|
||||
!errors?
|
||||
end
|
||||
|
||||
sig { params(message: T.nilable(String), location: T.nilable(String)).void }
|
||||
def add_error(message, location: nil)
|
||||
errors << ({ message: message, location: location })
|
||||
end
|
||||
sig { params(message: T.nilable(String), location: T.nilable(String), strictish: T::Boolean).void }
|
||||
def add_error(message, location: nil, strictish: false)
|
||||
# Only raise non-critical audits if the user specified `--strict`.
|
||||
return if strictish && !@strict
|
||||
|
||||
sig { params(message: T.nilable(String), location: T.nilable(String)).void }
|
||||
def add_warning(message, location: nil)
|
||||
# Warnings are ignored unless `--strict` is passed in which case they're turned into errors.
|
||||
add_error(message, location: location) if strict?
|
||||
errors << ({ message: message, location: location })
|
||||
end
|
||||
|
||||
def result
|
||||
@ -195,7 +192,7 @@ module Cask
|
||||
# increases the maintenance burden.
|
||||
return if cask.tap == "homebrew/cask-fonts"
|
||||
|
||||
add_warning "Cask should have a description. Please add a `desc` stanza." if cask.desc.blank?
|
||||
add_error("Cask should have a description. Please add a `desc` stanza.", strictish: true) if cask.desc.blank?
|
||||
end
|
||||
|
||||
sig { void }
|
||||
@ -383,8 +380,10 @@ module Cask
|
||||
return unless token_conflicts?
|
||||
return unless core_formula_names.include?(cask.token)
|
||||
|
||||
add_warning "possible duplicate, cask token conflicts with Homebrew core formula: " \
|
||||
"#{Formatter.url(core_formula_url)}"
|
||||
add_error(
|
||||
"possible duplicate, cask token conflicts with Homebrew core formula: #{Formatter.url(core_formula_url)}",
|
||||
strictish: true,
|
||||
)
|
||||
end
|
||||
|
||||
sig { void }
|
||||
@ -418,18 +417,19 @@ module Cask
|
||||
add_error "cask token contains version designation '#{match_data[:designation]}'"
|
||||
end
|
||||
|
||||
add_warning "cask token mentions launcher" if token.end_with? "launcher"
|
||||
add_error("cask token mentions launcher", strictish: true) if token.end_with? "launcher"
|
||||
|
||||
add_warning "cask token mentions desktop" if token.end_with? "desktop"
|
||||
add_error("cask token mentions desktop", strictish: true) if token.end_with? "desktop"
|
||||
|
||||
add_warning "cask token mentions platform" if token.end_with? "mac", "osx", "macos"
|
||||
add_error("cask token mentions platform", strictish: true) if token.end_with? "mac", "osx", "macos"
|
||||
|
||||
add_warning "cask token mentions architecture" if token.end_with? "x86", "32_bit", "x86_64", "64_bit"
|
||||
add_error("cask token mentions architecture", strictish: true) if token.end_with? "x86", "32_bit", "x86_64",
|
||||
"64_bit"
|
||||
|
||||
frameworks = %w[cocoa qt gtk wx java]
|
||||
return if frameworks.include?(token) || !token.end_with?(*frameworks)
|
||||
|
||||
add_warning "cask token mentions framework"
|
||||
add_error("cask token mentions framework", strictish: true)
|
||||
end
|
||||
|
||||
sig { void }
|
||||
@ -449,7 +449,10 @@ module Cask
|
||||
return if cask.url.to_s.include? cask.version.csv.second
|
||||
return if cask.version.csv.third.present? && cask.url.to_s.include?(cask.version.csv.third)
|
||||
|
||||
add_warning "Download does not require additional version components. Use `&:short_version` in the livecheck"
|
||||
add_error(
|
||||
"Download does not require additional version components. Use `&:short_version` in the livecheck",
|
||||
strictish: true,
|
||||
)
|
||||
end
|
||||
|
||||
sig { void }
|
||||
@ -493,7 +496,7 @@ module Cask
|
||||
"#{message} fix the signature of their app."
|
||||
end
|
||||
|
||||
add_warning message
|
||||
add_error(message, strictish: true)
|
||||
when Artifact::Pkg
|
||||
path = downloaded_path
|
||||
next unless path.exist?
|
||||
@ -501,7 +504,7 @@ module Cask
|
||||
result = system_command("pkgutil", args: ["--check-signature", path], print_stderr: false)
|
||||
|
||||
unless result.success?
|
||||
add_warning <<~EOS
|
||||
add_error(<<~EOS, strictish: true)
|
||||
Signature verification failed:
|
||||
#{result.merged_output}
|
||||
macOS on ARM requires applications to be signed.
|
||||
@ -513,7 +516,7 @@ module Cask
|
||||
result = system_command("stapler", args: ["validate", path], print_stderr: false)
|
||||
next if result.success?
|
||||
|
||||
add_warning <<~EOS
|
||||
add_error(<<~EOS, strictish: true)
|
||||
Signature verification failed:
|
||||
#{result.merged_output}
|
||||
macOS on ARM requires applications to be signed.
|
||||
@ -638,16 +641,9 @@ module Cask
|
||||
|
||||
metadata = SharedAudits.github_repo_data(user, repo)
|
||||
return if metadata.nil?
|
||||
|
||||
return unless metadata["archived"]
|
||||
|
||||
message = "GitHub repo is archived"
|
||||
|
||||
if cask.discontinued?
|
||||
add_warning message
|
||||
else
|
||||
add_error message
|
||||
end
|
||||
add_error("GitHub repo is archived", strictish: cask.discontinued?)
|
||||
end
|
||||
|
||||
sig { void }
|
||||
@ -659,16 +655,9 @@ module Cask
|
||||
|
||||
metadata = SharedAudits.gitlab_repo_data(user, repo)
|
||||
return if metadata.nil?
|
||||
|
||||
return unless metadata["archived"]
|
||||
|
||||
message = "GitLab repo is archived"
|
||||
|
||||
if cask.discontinued?
|
||||
add_warning message
|
||||
else
|
||||
add_error message
|
||||
end
|
||||
add_error("GitLab repo is archived", strictish: cask.discontinued?)
|
||||
end
|
||||
|
||||
sig { void }
|
||||
|
@ -44,16 +44,6 @@ describe Cask::Audit, :cask do
|
||||
end
|
||||
end
|
||||
|
||||
matcher :warn_with do |message|
|
||||
match do |audit|
|
||||
include_msg?(audit.errors, message)
|
||||
end
|
||||
|
||||
failure_message do |audit|
|
||||
"expected to warn with message #{message.inspect} but #{outcome(audit)}"
|
||||
end
|
||||
end
|
||||
|
||||
let(:cask) { instance_double(Cask::Cask) }
|
||||
let(:new_cask) { nil }
|
||||
let(:online) { nil }
|
||||
@ -111,7 +101,7 @@ describe Cask::Audit, :cask do
|
||||
|
||||
context "when there are no errors and `--strict` is not passed so we should not show anything" do
|
||||
before do
|
||||
audit.add_warning "eh"
|
||||
audit.add_error("eh", strictish: true)
|
||||
end
|
||||
|
||||
it { is_expected.not_to match(/failed/) }
|
||||
@ -128,7 +118,7 @@ describe Cask::Audit, :cask do
|
||||
context "when there are errors and warnings" do
|
||||
before do
|
||||
audit.add_error "bad"
|
||||
audit.add_warning "eh"
|
||||
audit.add_error("eh", strictish: true)
|
||||
end
|
||||
|
||||
it { is_expected.to match(/failed/) }
|
||||
@ -139,7 +129,7 @@ describe Cask::Audit, :cask do
|
||||
|
||||
before do
|
||||
audit.add_error "very bad"
|
||||
audit.add_warning "a little bit bad"
|
||||
audit.add_error("a little bit bad", strictish: true)
|
||||
end
|
||||
|
||||
it { is_expected.to match(/failed/) }
|
||||
@ -147,7 +137,7 @@ describe Cask::Audit, :cask do
|
||||
|
||||
context "when there are warnings and `--strict` is not passed" do
|
||||
before do
|
||||
audit.add_warning "a little bit bad"
|
||||
audit.add_error("a little bit bad", strictish: true)
|
||||
end
|
||||
|
||||
it { is_expected.not_to match(/failed/) }
|
||||
@ -157,7 +147,7 @@ describe Cask::Audit, :cask do
|
||||
let(:strict) { true }
|
||||
|
||||
before do
|
||||
audit.add_warning "a little bit bad"
|
||||
audit.add_error("a little bit bad", strictish: true)
|
||||
end
|
||||
|
||||
it { is_expected.to match(/failed/) }
|
||||
@ -501,7 +491,7 @@ describe Cask::Audit, :cask do
|
||||
it "does not fail" do
|
||||
expect(download_double).not_to receive(:fetch)
|
||||
expect(UnpackStrategy).not_to receive(:detect)
|
||||
expect(run).not_to warn_with(/Audit\.app/)
|
||||
expect(run).not_to error_with(/Audit\.app/)
|
||||
end
|
||||
end
|
||||
|
||||
@ -519,7 +509,7 @@ describe Cask::Audit, :cask do
|
||||
it "does not fail since no extract" do
|
||||
allow(download_double).to receive(:fetch).and_return(Pathname.new("/tmp/test.zip"))
|
||||
allow(UnpackStrategy).to receive(:detect).and_return(nil)
|
||||
expect(run).not_to warn_with(/Audit\.app/)
|
||||
expect(run).not_to error_with(/Audit\.app/)
|
||||
end
|
||||
end
|
||||
end
|
||||
@ -1005,14 +995,14 @@ describe Cask::Audit, :cask do
|
||||
|
||||
it "warns about duplicates" do
|
||||
expect(audit).to receive(:core_formula_names).and_return(formula_names)
|
||||
expect(run).to warn_with(/possible duplicate/)
|
||||
expect(run).to error_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/)
|
||||
expect(run).not_to error_with(/possible duplicate/)
|
||||
end
|
||||
end
|
||||
end
|
||||
@ -1086,7 +1076,7 @@ describe Cask::Audit, :cask do
|
||||
let(:new_cask) { false }
|
||||
|
||||
it "does not warn" do
|
||||
expect(run).not_to warn_with(/should have a description/)
|
||||
expect(run).not_to error_with(/should have a description/)
|
||||
end
|
||||
end
|
||||
|
||||
|
Loading…
x
Reference in New Issue
Block a user