tests: add cmd/style integration test

[eirinikos]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew tests with your changes locally?

---

This change adds some test coverage for brew style. It looks like the parts concerning JSON output (including the RubocopResults RubocopOffense, and RubocopLineLocation classes) would be served by adding test coverage for brew audit --strict (which I'll add to my to-do list).

It seemed necessary to add rubocop to the Gemfile, but if that's incorrect, I look forward to finding out!

I'll be sure to update the creation and unlinking of the foo formula once this pull request is finalized/merged.

Thanks!

[UniqMartin]

Adding RuboCop to the Gemfile is problematic in several ways:

• It and some of its dependencies require Ruby 1.9+ (if I recollect correctly), thus will make our tests always fail on our Mavericks bot where we test against Ruby 1.8.7.
• It adds a non-trivial dependency that further increases the test setup time on our test bots, where bundler needs to fetch and setup the gems on every run.

I don't have a good solution to address these. But since brew style is exclusively used by maintainers and contributors, thus only affects a comparatively small number of people, I think we can live with not having a test for this command (at least until we find a better solution).

It seemed necessary to add rubocop to the Gemfile, […]

Can you elaborate on this? In normal operation, brew style automatically installs the rubocop gem into the ~/.gem directory if it isn't already installed. Is that causing issues in the testing environment? What are the problems, error messages, etc.?

[MikeMcQuaid]

Can you elaborate on this? In normal operation, brew style automatically installs the rubocop gem into the ~/.gem directory if it isn't already installed. Is that causing issues in the testing environment? What are the problems, error messages, etc.?

Yeh, I'm also interested in that. A step-by-step walkthrough of the commands you run and the errors you see would be great 👍

[eirinikos]

Adding RuboCop to the Gemfile is problematic in several ways:

• It and some of its dependencies require Ruby 1.9+ (if I recollect correctly), thus will make our tests always fail on our Mavericks bot where we test against Ruby 1.8.7.
• It adds a non-trivial dependency that further increases the test setup time on our test bots, where bundler needs to fetch and setup the gems on every run.

I don't have a good solution to address these. But since brew style is exclusively used by maintainers and contributors, thus only affects a comparatively small number of people, I think we can live with not having a test for this command (at least until we find a better solution).

Good to know and to keep in mind. 👍

Can you elaborate on this? In normal operation, brew style automatically installs the rubocop gem into the ~/.gem directory if it isn't already installed. Is that causing issues in the testing environment? What are the problems, error messages, etc.?

Yeh, I'm also interested in that. A step-by-step walkthrough of the commands you run and the errors you see would be great 👍

Sure thing! I did see Homebrew.install_gem_setup_path! "rubocop", "0.40" in cmd/style.rb so I was also surprised by the problem.

I've checked out a test branch from the master branch and added test_style to the integration tests (without modifying Gemfile). brew tests and brew tests --only=integration_cmds/style both return this failure (slightly modified for readability):

  1) Failure:
IntegrationCommandTests#test_style [/usr/local/Library/Homebrew/test/test_integration_cmds.rb:855]:
Expected /Prefer\ double\-quoted\ strings/
to match "==> Installing or updating 'rubocop' gem
\nSuccessfully installed rainbow-2.1.0
\nSuccessfully installed ast-2.3.0\nSuccessfully installed parser-2.3.1.2
\nSuccessfully installed powerpack-0.1.1\nSuccessfully installed ruby-progressbar-1.8.1
\nSuccessfully installed unicode-display_width-1.1.0\nSuccessfully installed rubocop-0.40.0
\n7 gems installed\n/Users/mizou/.gem/ruby/2.0.0/gems/bundler-
1.11.2/lib/bundler/rubygems_integration.rb:304:in `block in replace_gem':
rubocop is not part of the bundle. Add it to Gemfile. (Gem::LoadError)
\n\tfrom /Users/mizou/.gem/ruby/2.0.0/bin/rubocop:22:in `<main>'"

I also receive this rake error (possibly unrelated to the specific problem at hand):

rake aborted!
Command failed with status (1):
[ruby -I"lib:/usr/local/Library/Homebrew/test" -I
"/usr/local/Library/Homebrew/test/vendor/bundle/ruby/2.0.0/gems/rake-10.5.0/lib" 
"/usr/local/Library/Homebrew/test/vendor/bundle/ruby/2.0.0/gems/rake-
10.5.0/lib/rake/rake_test_loader.rb"

(followed by the names of the tests that were run)

[MikeMcQuaid]

Try wrapping this line and the one below with:

Bundler.with_clean_env do
...
end

and remove the Gemfile* changes.

[eirinikos]

Just curious, why wrap only these 2 lines?

assert_match "1 file inspected, 1 offense detected", cmd_fail("style", "#{foo_file}")
assert_match "1 file inspected, 1 offense detected", cmd_fail("style", "foo")

In any case, I removed the Gemfile* changes and tried wrapping those 2 lines (and all 3 lines). Received the same error in both cases. :-/

[UniqMartin]

I'm not sure what the problem and what the correct solution is, but Bundler.with_clean_env sadly isn't going to help (or change anything) here. It is already used in the implementation of cmd[_fail], or to be more precise in cmd_output where it is used to start a separate Homebrew process (but with the necessary adjustments for the testing environment) in which it then executes the specified Homebrew command.

[eirinikos]

Thanks for the explanation and sorry for the late response! I guess this pull request will be relegated to the dustbin, but it was worth a try, and I now have a better understanding of cmd and cmd_output. 👍

[MikeMcQuaid]

Yeh, it's probably worth closing this out for now, sorry @eirinikos 😭

[UniqMartin]

Don't worry! It's always a bit sad to let go of a PR, but it would be strange if every single one of them was a success story. On the positive side, I think we all learned a few things here. :)