diff options
Diffstat (limited to 'Library/Homebrew/test/dev-cmd/audit_spec.rb')
| -rw-r--r-- | Library/Homebrew/test/dev-cmd/audit_spec.rb | 426 |
1 files changed, 263 insertions, 163 deletions
diff --git a/Library/Homebrew/test/dev-cmd/audit_spec.rb b/Library/Homebrew/test/dev-cmd/audit_spec.rb index a6bb22837..b07ffaadc 100644 --- a/Library/Homebrew/test/dev-cmd/audit_spec.rb +++ b/Library/Homebrew/test/dev-cmd/audit_spec.rb @@ -5,6 +5,13 @@ RSpec::Matchers.alias_matcher :have_data, :be_data RSpec::Matchers.alias_matcher :have_end, :be_end RSpec::Matchers.alias_matcher :have_trailing_newline, :be_trailing_newline +module Count + def self.increment + @count ||= 0 + @count += 1 + end +end + describe FormulaText do let(:dir) { mktmpdir } @@ -141,100 +148,6 @@ describe FormulaAuditor do fa.audit_file expect(fa.problems).to eq([]) end - - specify "strict: ordering issue" do - fa = formula_auditor "foo", <<-EOS.undent, strict: true - class Foo < Formula - url "http://example.com/foo-1.0.tgz" - homepage "http://example.com" - end - EOS - - fa.audit_file - expect(fa.problems) - .to eq(["`homepage` (line 3) should be put before `url` (line 2)"]) - end - - specify "strict: resource placement" do - fa = formula_auditor "foo", <<-EOS.undent, strict: true - class Foo < Formula - url "https://example.com/foo-1.0.tgz" - - resource "foo2" do - url "https://example.com/foo-2.0.tgz" - end - - depends_on "openssl" - end - EOS - - fa.audit_file - expect(fa.problems) - .to eq(["`depends_on` (line 8) should be put before `resource` (line 4)"]) - end - - specify "strict: plist placement" do - fa = formula_auditor "foo", <<-EOS.undent, strict: true - class Foo < Formula - url "https://example.com/foo-1.0.tgz" - - test do - expect(shell_output("./dogs")).to match("Dogs are terrific") - end - - def plist - end - end - EOS - - fa.audit_file - expect(fa.problems) - .to eq(["`plist block` (line 8) should be put before `test block` (line 4)"]) - end - - specify "strict: url outside of stable block" do - fa = formula_auditor "foo", <<-EOS.undent, strict: true - class Foo < Formula - url "http://example.com/foo-1.0.tgz" - stable do - # stuff - end - end - EOS - - fa.audit_file - expect(fa.problems).to eq(["`url` should be put inside `stable block`"]) - end - - specify "strict: head and head do" do - fa = formula_auditor "foo", <<-EOS.undent, strict: true - class Foo < Formula - head "http://example.com/foo.git" - head do - # stuff - end - end - EOS - - fa.audit_file - expect(fa.problems).to eq(["Should not have both `head` and `head do`"]) - end - - specify "strict: bottle and bottle do" do - fa = formula_auditor "foo", <<-EOS.undent, strict: true - class Foo < Formula - url "http://example.com/foo-1.0.tgz" - bottle do - # bottles go here - end - bottle :unneeded - end - EOS - - fa.audit_file - expect(fa.problems) - .to eq(["Should not have `bottle :unneeded/:disable` and `bottle do`"]) - end end describe "#audit_class" do @@ -303,7 +216,7 @@ describe FormulaAuditor do end end - describe "#audit_line" do + describe "#line_problems" do specify "pkgshare" do fa = formula_auditor "foo", <<-EOS.undent, strict: true class Foo < Formula @@ -311,25 +224,25 @@ describe FormulaAuditor do end EOS - fa.audit_line 'ohai "#{share}/foo"', 3 + fa.line_problems 'ohai "#{share}/foo"', 3 expect(fa.problems.shift).to eq("Use \#{pkgshare} instead of \#{share}/foo") - fa.audit_line 'ohai "#{share}/foo/bar"', 3 + fa.line_problems 'ohai "#{share}/foo/bar"', 3 expect(fa.problems.shift).to eq("Use \#{pkgshare} instead of \#{share}/foo") - fa.audit_line 'ohai share/"foo"', 3 + fa.line_problems 'ohai share/"foo"', 3 expect(fa.problems.shift).to eq('Use pkgshare instead of (share/"foo")') - fa.audit_line 'ohai share/"foo/bar"', 3 + fa.line_problems 'ohai share/"foo/bar"', 3 expect(fa.problems.shift).to eq('Use pkgshare instead of (share/"foo")') - fa.audit_line 'ohai "#{share}/foo-bar"', 3 + fa.line_problems 'ohai "#{share}/foo-bar"', 3 expect(fa.problems).to eq([]) - fa.audit_line 'ohai share/"foo-bar"', 3 + fa.line_problems 'ohai share/"foo-bar"', 3 expect(fa.problems).to eq([]) - fa.audit_line 'ohai share/"bar"', 3 + fa.line_problems 'ohai share/"bar"', 3 expect(fa.problems).to eq([]) end @@ -344,11 +257,11 @@ describe FormulaAuditor do end EOS - fa.audit_line 'ohai "#{share}/foolibc++"', 3 + fa.line_problems 'ohai "#{share}/foolibc++"', 3 expect(fa.problems.shift) .to eq("Use \#{pkgshare} instead of \#{share}/foolibc++") - fa.audit_line 'ohai share/"foolibc++"', 3 + fa.line_problems 'ohai share/"foolibc++"', 3 expect(fa.problems.shift) .to eq('Use pkgshare instead of (share/"foolibc++")') end @@ -360,7 +273,7 @@ describe FormulaAuditor do end EOS - fa.audit_line "class Foo<Formula", 1 + fa.line_problems "class Foo<Formula", 1 expect(fa.problems.shift) .to eq("Use a space in class inheritance: class Foo < Formula") end @@ -368,10 +281,10 @@ describe FormulaAuditor do specify "default template" do fa = formula_auditor "foo", "class Foo < Formula; url '/foo-1.0.tgz'; end" - fa.audit_line '# system "cmake", ".", *std_cmake_args', 3 + fa.line_problems '# system "cmake", ".", *std_cmake_args', 3 expect(fa.problems.shift).to eq("Commented cmake call found") - fa.audit_line "# PLEASE REMOVE", 3 + fa.line_problems "# PLEASE REMOVE", 3 expect(fa.problems.shift).to eq("Please remove default template comments") end end @@ -409,77 +322,66 @@ describe FormulaAuditor do .to eq(["Don't recommend setuid in the caveats, suggest sudo instead."]) end - describe "#audit_homepage" do - specify "homepage URLs" do - fa = formula_auditor "foo", <<-EOS.undent, online: true + describe "#audit_keg_only_style" do + specify "keg_only_needs_downcasing" do + fa = formula_auditor "foo", <<-EOS.undent, strict: true class Foo < Formula - homepage "ftp://example.com/foo" url "http://example.com/foo-1.0.tgz" + + keg_only "Because why not" end EOS - fa.audit_homepage + fa.audit_keg_only_style expect(fa.problems) - .to eq(["The homepage should start with http or https (URL is #{fa.formula.homepage})."]) - - formula_homepages = { - "bar" => "http://www.freedesktop.org/wiki/bar", - "baz" => "http://www.freedesktop.org/wiki/Software/baz", - "qux" => "https://code.google.com/p/qux", - "quux" => "http://github.com/quux", - "corge" => "http://savannah.nongnu.org/corge", - "grault" => "http://grault.github.io/", - "garply" => "http://www.gnome.org/garply", - "sf1" => "http://foo.sourceforge.net/", - "sf2" => "http://foo.sourceforge.net", - "sf3" => "http://foo.sf.net/", - "sf4" => "http://foo.sourceforge.io/", - "waldo" => "http://www.gnu.org/waldo", - } - - formula_homepages.each do |name, homepage| - fa = formula_auditor name, <<-EOS.undent - class #{Formulary.class_s(name)} < Formula - homepage "#{homepage}" - url "http://example.com/#{name}-1.0.tgz" - end - EOS + .to eq(["'Because' from the keg_only reason should be 'because'.\n"]) + end - fa.audit_homepage - if homepage =~ %r{http:\/\/www\.freedesktop\.org} - if homepage =~ /Software/ - expect(fa.problems.first).to match( - "#{homepage} should be styled " \ - "`https://wiki.freedesktop.org/www/Software/project_name`", - ) - else - expect(fa.problems.first).to match( - "#{homepage} should be styled " \ - "`https://wiki.freedesktop.org/project_name`", - ) - end - elsif homepage =~ %r{https:\/\/code\.google\.com} - expect(fa.problems.first) - .to match("#{homepage} should end with a slash") - elsif homepage =~ /foo\.(sf|sourceforge)\.net/ - expect(fa.problems.first) - .to match("#{homepage} should be `https://foo.sourceforge.io/`") - else - expect(fa.problems.first) - .to match("Please use https:// for #{homepage}") + specify "keg_only_redundant_period" do + fa = formula_auditor "foo", <<-EOS.undent, strict: true + class Foo < Formula + url "http://example.com/foo-1.0.tgz" + + keg_only "because this line ends in a period." end - end + EOS + + fa.audit_keg_only_style + expect(fa.problems) + .to eq(["keg_only reason should not end with a period."]) + end + + specify "keg_only_handles_block_correctly" do + fa = formula_auditor "foo", <<-EOS.undent, strict: true + class Foo < Formula + url "http://example.com/foo-1.0.tgz" + + keg_only <<-EOF.undent + this line starts with a lowercase word. + + This line does not but that shouldn't be a + problem + EOF + end + EOS + + fa.audit_keg_only_style + expect(fa.problems) + .to eq([]) end - specify "missing homepage" do - fa = formula_auditor "foo", <<-EOS.undent, online: true + specify "keg_only_handles_whitelist_correctly" do + fa = formula_auditor "foo", <<-EOS.undent, strict: true class Foo < Formula url "http://example.com/foo-1.0.tgz" + + keg_only "Apple ships foo in the CLT package" end EOS - fa.audit_homepage - expect(fa.problems.first).to match("Formula should have a homepage.") + fa.audit_keg_only_style + expect(fa.problems) + .to eq([]) end end @@ -517,5 +419,203 @@ describe FormulaAuditor do expect(fa.problems.first) .to match('xcodebuild should be passed an explicit "SYMROOT"') end + + specify "disallow go get usage" do + fa = formula_auditor "foo", <<-EOS.undent + class Foo <Formula + url "http://example.com/foo-1.0.tgz" + + def install + system "go", "get", "bar" + end + end + EOS + fa.audit_text + expect(fa.problems.first) + .to match("Formulae should not use `go get`. If non-vendored resources are required use `go_resource`s.") + end + end + + describe "#audit_revision_and_version_scheme" do + let(:origin_tap_path) { Tap::TAP_DIRECTORY/"homebrew/homebrew-foo" } + let(:formula_subpath) { "Formula/foo#{@foo_version}.rb" } + let(:origin_formula_path) { origin_tap_path/formula_subpath } + let(:tap_path) { Tap::TAP_DIRECTORY/"homebrew/homebrew-bar" } + let(:formula_path) { tap_path/formula_subpath } + + before(:each) do + @foo_version = Count.increment + + origin_formula_path.write <<-EOS.undent + class Foo#{@foo_version} < Formula + url "https://example.com/foo-1.0.tar.gz" + revision 2 + version_scheme 1 + end + EOS + + origin_tap_path.mkpath + origin_tap_path.cd do + shutup do + system "git", "init" + system "git", "add", "--all" + system "git", "commit", "-m", "init" + end + end + + tap_path.mkpath + tap_path.cd do + shutup do + system "git", "clone", origin_tap_path, "." + end + end + end + + subject do + fa = described_class.new(Formulary.factory(formula_path)) + fa.audit_revision_and_version_scheme + fa.problems.first + end + + def formula_gsub(before, after = "") + text = formula_path.read + text.gsub! before, after + formula_path.unlink + formula_path.write text + end + + def formula_gsub_commit(before, after = "") + text = origin_formula_path.read + text.gsub!(before, after) + origin_formula_path.unlink + origin_formula_path.write text + + origin_tap_path.cd do + shutup do + system "git", "commit", "-am", "commit" + end + end + + tap_path.cd do + shutup do + system "git", "fetch" + system "git", "reset", "--hard", "origin/master" + end + end + end + + context "revisions" do + context "should not be removed when first committed above 0" do + it { is_expected.to be_nil } + end + + context "should not decrease with the same version" do + before { formula_gsub_commit "revision 2", "revision 1" } + + it { is_expected.to match("revision should not decrease (from 2 to 1)") } + end + + context "should not be removed with the same version" do + before { formula_gsub_commit "revision 2" } + + it { is_expected.to match("revision should not decrease (from 2 to 0)") } + end + + context "should not decrease with the same, uncommitted version" do + before { formula_gsub "revision 2", "revision 1" } + + it { is_expected.to match("revision should not decrease (from 2 to 1)") } + end + + context "should be removed with a newer version" do + before { formula_gsub_commit "foo-1.0.tar.gz", "foo-1.1.tar.gz" } + + it { is_expected.to match("'revision 2' should be removed") } + end + + context "should not warn on an newer version revision removal" do + before do + formula_gsub_commit "revision 2", "" + formula_gsub_commit "foo-1.0.tar.gz", "foo-1.1.tar.gz" + end + + it { is_expected.to be_nil } + end + + context "should only increment by 1 with an uncommitted version" do + before do + formula_gsub "foo-1.0.tar.gz", "foo-1.1.tar.gz" + formula_gsub "revision 2", "revision 4" + end + + it { is_expected.to match("revisions should only increment by 1") } + end + + context "should not warn on past increment by more than 1" do + before do + formula_gsub_commit "revision 2", "# no revision" + formula_gsub_commit "foo-1.0.tar.gz", "foo-1.1.tar.gz" + formula_gsub_commit "# no revision", "revision 3" + end + + it { is_expected.to be_nil } + end + end + + context "version_schemes" do + context "should not decrease with the same version" do + before { formula_gsub_commit "version_scheme 1" } + + it { is_expected.to match("version_scheme should not decrease (from 1 to 0)") } + end + + context "should not decrease with a new version" do + before do + formula_gsub_commit "foo-1.0.tar.gz", "foo-1.1.tar.gz" + formula_gsub_commit "version_scheme 1", "" + formula_gsub_commit "revision 2", "" + end + + it { is_expected.to match("version_scheme should not decrease (from 1 to 0)") } + end + + context "should only increment by 1" do + before do + formula_gsub_commit "version_scheme 1", "# no version_scheme" + formula_gsub_commit "foo-1.0.tar.gz", "foo-1.1.tar.gz" + formula_gsub_commit "revision 2", "" + formula_gsub_commit "# no version_scheme", "version_scheme 3" + end + + it { is_expected.to match("version_schemes should only increment by 1") } + end + end + + context "versions" do + context "uncommitted should not decrease" do + before { formula_gsub "foo-1.0.tar.gz", "foo-0.9.tar.gz" } + + it { is_expected.to match("stable version should not decrease (from 1.0 to 0.9)") } + end + + context "committed can decrease" do + before do + formula_gsub_commit "revision 2" + formula_gsub_commit "foo-1.0.tar.gz", "foo-0.9.tar.gz" + end + + it { is_expected.to be_nil } + end + + context "can decrease with version_scheme increased" do + before do + formula_gsub "revision 2" + formula_gsub "foo-1.0.tar.gz", "foo-0.9.tar.gz" + formula_gsub "version_scheme 1", "version_scheme 2" + end + + it { is_expected.to be_nil } + end + end end end |