diff options
Diffstat (limited to 'Library/Homebrew/test/dev-cmd/audit_spec.rb')
| -rw-r--r-- | Library/Homebrew/test/dev-cmd/audit_spec.rb | 498 |
1 files changed, 498 insertions, 0 deletions
diff --git a/Library/Homebrew/test/dev-cmd/audit_spec.rb b/Library/Homebrew/test/dev-cmd/audit_spec.rb index 397e6912d..c4afe6592 100644 --- a/Library/Homebrew/test/dev-cmd/audit_spec.rb +++ b/Library/Homebrew/test/dev-cmd/audit_spec.rb @@ -59,3 +59,501 @@ describe FormulaText do expect(ft.without_patch).to eq("class End < Formula\n \nend") end end + +describe FormulaAuditor do + def formula_auditor(name, text, options = {}) + path = Pathname.new "#{dir}/#{name}.rb" + path.open("w") do |f| + f.write text + end + + described_class.new(Formulary.factory(path), options) + end + + let(:dir) { @dir = Pathname.new(Dir.mktmpdir) } + + after(:each) do + dir.rmtree unless @dir.nil? + end + + describe "#problems" do + it "is empty by default" do + fa = formula_auditor "foo", <<-EOS.undent + class Foo < Formula + url "http://example.com/foo-1.0.tgz" + end + EOS + + expect(fa.problems).to be_empty + end + end + + describe "#audit_file" do + specify "file permissions" do + allow(File).to receive(:umask).and_return(022) + + fa = formula_auditor "foo", <<-EOS.undent + class Foo < Formula + url "http://example.com/foo-1.0.tgz" + end + EOS + + path = fa.formula.path + path.chmod 0400 + + fa.audit_file + expect(fa.problems) + .to eq(["Incorrect file permissions (400): chmod 644 #{path}"]) + end + + specify "DATA but no __END__" do + fa = formula_auditor "foo", <<-EOS.undent + class Foo < Formula + url "http://example.com/foo-1.0.tgz" + patch :DATA + end + EOS + + fa.audit_file + expect(fa.problems).to eq(["'DATA' was found, but no '__END__'"]) + end + + specify "__END__ but no DATA" do + fa = formula_auditor "foo", <<-EOS.undent + class Foo < Formula + url "http://example.com/foo-1.0.tgz" + end + __END__ + a patch goes here + EOS + + fa.audit_file + expect(fa.problems).to eq(["'__END__' was found, but 'DATA' is not used"]) + end + + specify "no trailing newline" do + fa = formula_auditor "foo", 'class Foo<Formula; url "file:///foo-1.0.tgz";end' + + fa.audit_file + expect(fa.problems).to eq(["File should end with a newline"]) + end + + specify "no issue" do + fa = formula_auditor "foo", <<-EOS.undent + 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([]) + 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 + specify "missing test" do + fa = formula_auditor "foo", <<-EOS.undent + class Foo < Formula + url "http://example.com/foo-1.0.tgz" + end + EOS + + fa.audit_class + expect(fa.problems).to eq([]) + + fa = formula_auditor "foo", <<-EOS.undent, strict: true + class Foo < Formula + url "http://example.com/foo-1.0.tgz" + end + EOS + + fa.audit_class + expect(fa.problems).to eq(["A `test do` test block should be added"]) + end + + specify "GithubGistFormula", :needs_compat do + ENV.delete("HOMEBREW_DEVELOPER") + + fa = shutup do + formula_auditor "foo", <<-EOS.undent + class Foo < GithubGistFormula + url "http://example.com/foo-1.0.tgz" + end + EOS + end + + fa.audit_class + expect(fa.problems) + .to eq(["GithubGistFormula is deprecated, use Formula instead"]) + end + + specify "ScriptFileFormula", :needs_compat do + ENV.delete("HOMEBREW_DEVELOPER") + + fa = formula_auditor "foo", <<-EOS.undent + class Foo < ScriptFileFormula + url "http://example.com/foo-1.0.tgz" + end + EOS + + fa.audit_class + expect(fa.problems) + .to eq(["ScriptFileFormula is deprecated, use Formula instead"]) + end + + specify "AmazonWebServicesFormula", :needs_compat do + ENV.delete("HOMEBREW_DEVELOPER") + + fa = formula_auditor "foo", <<-EOS.undent + class Foo < AmazonWebServicesFormula + url "http://example.com/foo-1.0.tgz" + end + EOS + + fa.audit_class + expect(fa.problems) + .to eq(["AmazonWebServicesFormula is deprecated, use Formula instead"]) + end + end + + describe "#audit_line" do + specify "pkgshare" do + fa = formula_auditor "foo", <<-EOS.undent, strict: true + class Foo < Formula + url "http://example.com/foo-1.0.tgz" + end + EOS + + fa.audit_line 'ohai "#{share}/foo"', 3 + expect(fa.problems.shift).to eq("Use \#{pkgshare} instead of \#{share}/foo") + + fa.audit_line 'ohai "#{share}/foo/bar"', 3 + expect(fa.problems.shift).to eq("Use \#{pkgshare} instead of \#{share}/foo") + + fa.audit_line 'ohai share/"foo"', 3 + expect(fa.problems.shift).to eq('Use pkgshare instead of (share/"foo")') + + fa.audit_line '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 + expect(fa.problems).to eq([]) + + fa.audit_line 'ohai share/"foo-bar"', 3 + expect(fa.problems).to eq([]) + + fa.audit_line 'ohai share/"bar"', 3 + expect(fa.problems).to eq([]) + end + + # Regression test for https://github.com/Homebrew/legacy-homebrew/pull/48744 + # Formulae with "++" in their name would break various audit regexps: + # Error: nested *?+ in regexp: /^libxml++3\s/ + specify "++ in name" do + fa = formula_auditor "foolibc++", <<-EOS.undent, strict: true + class Foolibcxx < Formula + desc "foolibc++ is a test" + url "http://example.com/foo-1.0.tgz" + end + EOS + + fa.audit_desc + expect(fa.problems.shift) + .to eq("Description shouldn't include the formula name") + + fa.audit_line 'ohai "#{share}/foolibc++"', 3 + expect(fa.problems.shift) + .to eq("Use \#{pkgshare} instead of \#{share}/foolibc++") + + fa.audit_line 'ohai share/"foolibc++"', 3 + expect(fa.problems.shift) + .to eq('Use pkgshare instead of (share/"foolibc++")') + end + + specify "no space in class inheritance" do + fa = formula_auditor "foo", <<-EOS.undent + class Foo<Formula + url '/foo-1.0.tgz' + end + EOS + + fa.audit_line "class Foo<Formula", 1 + expect(fa.problems.shift) + .to eq("Use a space in class inheritance: class Foo < Formula") + end + + 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 + expect(fa.problems.shift).to eq("Commented cmake call found") + + fa.audit_line "# PLEASE REMOVE", 3 + expect(fa.problems.shift).to eq("Please remove default template comments") + end + end + + describe "#audit_github_repository" do + specify "#audit_github_repository when HOMEBREW_NO_GITHUB_API is set" do + ENV["HOMEBREW_NO_GITHUB_API"] = "1" + + fa = formula_auditor "foo", <<-EOS.undent, strict: true, online: true + class Foo < Formula + homepage "https://github.com/example/example" + url "http://example.com/foo-1.0.tgz" + end + EOS + + fa.audit_github_repository + expect(fa.problems).to eq([]) + end + end + + specify "#audit_caveats" do + fa = formula_auditor "foo", <<-EOS.undent + class Foo < Formula + homepage "http://example.com/foo" + url "http://example.com/foo-1.0.tgz" + + def caveats + "setuid" + end + end + EOS + + fa.audit_caveats + expect(fa.problems) + .to eq(["Don't recommend setuid in the caveats, suggest sudo instead."]) + end + + specify "#audit_desc" do + formula_descriptions = [ + { name: "foo", desc: nil, + problem: "Formula should have a desc" }, + { name: "bar", desc: "bar" * 30, + problem: "Description is too long" }, + { name: "baz", desc: "Baz commandline tool", + problem: "Description should use \"command-line\"" }, + { name: "qux", desc: "A tool called Qux", + problem: "Description shouldn't start with an indefinite article" }, + ] + + formula_descriptions.each do |formula| + content = <<-EOS.undent + class #{Formulary.class_s(formula[:name])} < Formula + url "http://example.com/#{formula[:name]}-1.0.tgz" + desc "#{formula[:desc]}" + end + EOS + + fa = formula_auditor formula[:name], content, strict: true + fa.audit_desc + expect(fa.problems.first).to match(formula[:problem]) + end + end + + describe "#audit_homepage" do + specify "homepage URLs" do + fa = formula_auditor "foo", <<-EOS.undent, online: true + class Foo < Formula + homepage "ftp://example.com/foo" + url "http://example.com/foo-1.0.tgz" + end + EOS + + fa.audit_homepage + 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 + + 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}") + end + end + end + + specify "missing homepage" do + fa = formula_auditor "foo", <<-EOS.undent, online: true + class Foo < Formula + url "http://example.com/foo-1.0.tgz" + end + EOS + + fa.audit_homepage + expect(fa.problems.first).to match("Formula should have a homepage.") + end + end + + describe "#audit_text" do + specify "xcodebuild suggests symroot" do + fa = formula_auditor "foo", <<-EOS.undent + class Foo < Formula + url "http://example.com/foo-1.0.tgz" + homepage "http://example.com" + + def install + xcodebuild "-project", "meow.xcodeproject" + end + end + EOS + + fa.audit_text + expect(fa.problems.first) + .to match('xcodebuild should be passed an explicit "SYMROOT"') + end + + specify "bare xcodebuild also suggests symroot" do + fa = formula_auditor "foo", <<-EOS.undent + class Foo < Formula + url "http://example.com/foo-1.0.tgz" + homepage "http://example.com" + + def install + xcodebuild + end + end + EOS + + fa.audit_text + expect(fa.problems.first) + .to match('xcodebuild should be passed an explicit "SYMROOT"') + end + end +end |