Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

rubocops/text: improve test descriptions #10332

Merged
merged 2 commits into from Jan 18, 2021
Merged

rubocops/text: improve test descriptions #10332

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3893cb3
rubocops/text: improve test descriptions
SeekingMeaning Jan 15, 2021
8fa263b
rubocops/text: add "in homebrew/core" to applicable tests
SeekingMeaning Jan 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
72 changes: 27 additions & 45 deletions Library/Homebrew/test/rubocops/text_spec.rb
View file
Open in desktop
Expand Up @@ -6,8 +6,8 @@
describe RuboCop::Cop::FormulaAudit::Text do
subject(:cop) { described_class.new }

context "When auditing formula text" do
it "with `require \"formula\"` is present" do
context "when auditing formula text" do
it 'reports an offense if `require "formula"` is present' do
expect_offense(<<~RUBY)
require "formula"
^^^^^^^^^^^^^^^^^ `require "formula"` is now unnecessary
Expand All @@ -25,7 +25,7 @@ class Foo < Formula
RUBY
end

it "with both openssl and libressl optional dependencies" do
it "reports an offense if both openssl and libressl are dependencies" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
Expand All @@ -36,9 +36,7 @@ class Foo < Formula
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Formulae should not depend on both OpenSSL and LibreSSL (even optionally).
end
RUBY
end

it "with both openssl and libressl dependencies" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
Expand All @@ -51,7 +49,7 @@ class Foo < Formula
RUBY
end

it "when veclibfort is used instead of OpenBLAS" do
it "reports an offense if veclibfort is used instead of OpenBLAS (in homebrew/core)" do
expect_offense(<<~RUBY, "/homebrew-core/")
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
Expand All @@ -62,7 +60,7 @@ class Foo < Formula
RUBY
end

it "when lapack is used instead of OpenBLAS" do
it "reports an offense if lapack is used instead of OpenBLAS (in homebrew/core)" do
expect_offense(<<~RUBY, "/homebrew-core/")
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
Expand All @@ -73,7 +71,7 @@ class Foo < Formula
RUBY
end

it "When xcodebuild is called without SYMROOT" do
it "reports an offense if xcodebuild is called without SYMROOT" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
Expand All @@ -85,9 +83,7 @@ def install
end
end
RUBY
end

it "When xcodebuild is called without any args" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
Expand All @@ -101,7 +97,7 @@ def install
RUBY
end

it "When go get is executed" do
it "reports an offense if `go get` is executed" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
Expand All @@ -115,7 +111,7 @@ def install
RUBY
end

it "When xcodebuild is executed" do
it "reports an offense if `xcodebuild` is executed" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
Expand All @@ -129,7 +125,7 @@ def install
RUBY
end

it "When plist_options are not defined when using a formula-defined plist", :ruby23 do
it "reports an offense if `plist_options` are not defined when using a formula-defined `plist`", :ruby23 do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
Expand Down Expand Up @@ -157,7 +153,7 @@ def plist
RUBY
end

it "When language/go is require'd" do
it 'reports an offense if `require "language/go"` is present' do
expect_offense(<<~RUBY)
require "language/go"
^^^^^^^^^^^^^^^^^^^^^ require "language/go" is unnecessary unless using `go_resource`s
Expand All @@ -174,7 +170,7 @@ def install
RUBY
end

it "When formula uses virtualenv and also `setuptools` resource" do
it "reports an offense if formula uses virtualenv and also `setuptools` resource" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
Expand All @@ -193,7 +189,7 @@ def install
RUBY
end

it "When Formula.factory(name) is used" do
it "reports an offense if `Formula.factory(name)` is present" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
Expand All @@ -207,7 +203,7 @@ def install
RUBY
end

it "When dep ensure is used without `-vendor-only`" do
it "reports an offense if `dep ensure` is used without `-vendor-only`" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
Expand All @@ -221,7 +217,7 @@ def install
RUBY
end

it "When cargo build is executed" do
it "reports an offense if `cargo build` is executed" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
Expand All @@ -235,7 +231,7 @@ def install
RUBY
end

it "When make calls are not separated" do
it "reports an offense if `make` calls are not separated" do
expect_offense(<<~RUBY)
class Foo < Formula
def install
Expand All @@ -246,7 +242,7 @@ def install
RUBY
end

it "When concatenating in string interpolation" do
it "reports an offense if paths are concatenated in string interpolation" do
expect_offense(<<~RUBY)
class Foo < Formula
def install
Expand All @@ -257,7 +253,7 @@ def install
RUBY
end

it "When using `prefix + \"bin\"` instead of `bin`" do
it 'reports an offense if `prefix + "bin"` is present' do
expect_offense(<<~RUBY)
class Foo < Formula
def install
Expand All @@ -266,9 +262,7 @@ def install
end
end
RUBY
end

it "When using `prefix + \"bin/foo\"` instead of `bin`" do
expect_offense(<<~RUBY)
class Foo < Formula
def install
Expand All @@ -284,8 +278,8 @@ def install
describe RuboCop::Cop::FormulaAuditStrict::Text do
subject(:cop) { described_class.new }

context "When auditing formula text" do
it "when deprecated `env :userpaths` is present" do
context "when auditing formula text in homebrew/core" do
it "reports an offense if `env :userpaths` is present" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
Expand All @@ -296,7 +290,7 @@ class Foo < Formula
RUBY
end

it "when deprecated `env :std` is present in homebrew-core" do
it "reports an offense if `env :std` is present in homebrew/core" do
expect_offense(<<~RUBY, "/homebrew-core/")
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
Expand All @@ -307,7 +301,7 @@ class Foo < Formula
RUBY
end

it "when `\#{share}/foo` is used instead of `\#{pkgshare}`" do
it %Q(reports an offense if "\#{share}/<formula name>" is present) do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this not work? Single quotes won't do string interpolation

Suggested change
it %Q(reports an offense if "\#{share}/<formula name>" is present) do
it 'reports an offense if "#{share}/<formula name>" is present' do

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That doesn't work as it's seen as broken interpolation, i.e. we have to explicitly escape the #

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure? When I test it out in brew irb it seems to work fine...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a quick test ruby script:

puts 'reports an offense if "#{share}/<formula name>" is present'

And then ran brew ruby test.rb:

$ brew ruby test.rb
reports an offense if "#{share}/<formula name>" is present

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I meant that it works, but brew style sees it as "broken"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

Library/Homebrew/test/rubocops/text_spec.rb:304:8: W: [Correctable] Interpolation in single quoted string detected. Use double quoted strings if you need interpolation.
    it 'reports an offense if "#{share}/<formula name>" is present' do
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected, 1 offense auto-correctable

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. That's interesting... If it were up to me I'd say let's disable that cop and allow it. But I don't feel strongly about that, so I think %Q is fine. Although, if we use %q instead no need to escape the #:

Suggested change
it %Q(reports an offense if "\#{share}/<formula name>" is present) do
it %q(reports an offense if "#{share}/<formula name>" is present) do

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same error:

Library/Homebrew/test/rubocops/text_spec.rb:304:8: W: [Correctable] Interpolation in single quoted string detected. Use double quoted strings if you need interpolation.
    it %q(reports an offense if "#{share}/<formula name>" is present) do
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah well, that's unfortunate. I agree, then, this is the best option.

expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb")
class Foo < Formula
def install
Expand All @@ -316,9 +310,7 @@ def install
end
end
RUBY
end

it "when `\#{share}/foo/bar` is used instead of `\#{pkgshare}/bar`" do
expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb")
class Foo < Formula
def install
Expand All @@ -327,11 +319,9 @@ def install
end
end
RUBY
end

it "when `\#{share}/foolibc++` is used instead of `\#{pkgshare}/foolibc++`" do
expect_offense(<<~RUBY, "/homebrew-core/Formula/foolibc++.rb")
class Foo < Formula
class Foolibcxx < Formula
def install
ohai "\#{share}/foolibc++"
^^^^^^^^^^^^^^^^^^^^ Use `\#{pkgshare}` instead of `\#{share}/foolibc++`
Expand All @@ -340,7 +330,7 @@ def install
RUBY
end

it "when `share/\"foo\"` is used instead of `pkgshare`" do
it 'reports an offense if `share/"<formula name>"` is present' do
expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb")
class Foo < Formula
def install
Expand All @@ -349,9 +339,7 @@ def install
end
end
RUBY
end

it "when `share/\"foo/bar\"` is used instead of `pkgshare`" do
expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb")
class Foo < Formula
def install
Expand All @@ -360,11 +348,9 @@ def install
end
end
RUBY
end

it "when `share/\"foolibc++\"` is used instead of `pkgshare`" do
expect_offense(<<~RUBY, "/homebrew-core/Formula/foolibc++.rb")
class Foo < Formula
class Foolibcxx < Formula
def install
ohai share/"foolibc++"
^^^^^^^^^^^^^^^^^ Use `pkgshare` instead of `share/"foolibc++"`
Expand All @@ -373,7 +359,7 @@ def install
RUBY
end

it "when `\#{share}/foo-bar` doesn't match formula name" do
it %Q(reports no offenses if "\#{share}/<directory name>" doesn't match formula name) do
SeekingMeaning marked this conversation as resolved.
Show resolved Hide resolved
expect_no_offenses(<<~RUBY, "/homebrew-core/Formula/foo.rb")
class Foo < Formula
def install
Expand All @@ -383,27 +369,23 @@ def install
RUBY
end

it "when `share/foo-bar` doesn't match formula name" do
it 'reports no offenses if `share/"<formula name>"` is not present' do
expect_no_offenses(<<~RUBY, "/homebrew-core/Formula/foo.rb")
class Foo < Formula
def install
ohai share/"foo-bar"
end
end
RUBY
end

it "when `share/bar` doesn't match formula name" do
expect_no_offenses(<<~RUBY, "/homebrew-core/Formula/foo.rb")
class Foo < Formula
def install
ohai share/"bar"
end
end
RUBY
end

it "when formula name appears afer `share/\"bar\"`" do
expect_no_offenses(<<~RUBY, "/homebrew-core/Formula/foo.rb")
class Foo < Formula
def install
Expand All @@ -413,7 +395,7 @@ def install
RUBY
end

it "when formula name appears afer `\"\#{share}/bar\"`" do
it %Q(reports no offenses if formula name appears afer "\#{share}/<directory name>") do
SeekingMeaning marked this conversation as resolved.
Show resolved Hide resolved
expect_no_offenses(<<~RUBY, "/homebrew-core/Formula/foo.rb")
class Foo < Formula
def install
Expand Down