Workaround common options mutation in Gem::Command test

[ghost]

We need to restore common options if we want to execute commands using
such options in other tests. For example, if a test runs a command with
--silent option, we get this kind of error:

OptionParser::InvalidOption: invalid option: --silent

[ghost]

Description:

Workaround common options mutation in Gem::Command test. My use case
was writing tests for Gem::Commands::SetupCommand#execute. But this
code also executes the Gem::Commands::PristineCommand using
--silent option, which don't exist anymore if it's removed in
Gem::Command test.

Tasks:

• Write tests

No test since it's a change in the test suite only. So I don't think
it's necessary.

• Write code to solve the problem
• Get code review from coworkers / friends

I hope my "workaround" is acceptable, but would be happy to try
another way if needed, please let me know.

[segiddins]

Why is this necessary, given that we always correctly set the common options in setup?

[ghost]

(edit: see next comment for better formatting)

On 2017-11-24 15:45:34 +0000, Samuel Giddins wrote: Why is this necessary, given that we always correctly set the common options in `setup`?

If I understand minitest and the test suite correctly, in the context of `TestGemCommand` I agree my suggestion is useless. As long as we don't need any test relying on "original" common options, or that we run each test case in isolation, there are no problem at all. I hope I run the test suite correctly: `rake`, or directly requiring `minitest/autorun` and test files with ruby like in following examples. I see that all tests run in the same process, and `Gem::Command.common_options` is global state, so any modification is a "side effect" for *other* test cases: $ cat test/test_gem_command_common_options.rb require 'rubygems/test_case' require 'rubygems/command' class TestGemCommandCommonOptions < Gem::TestCase def test_common_options opts = Gem::Command.common_options.map { |options| options[0][0] } assert_includes opts, '-h' end end $ ruby -w -Ilib:test:bundler/lib --disable-gems \ -e 'require "rubygems"; require "minitest/autorun"' \ -e 'require "test_gem_command_common_options"' . 1 tests, 2 assertions, 0 failures, 0 errors, 0 skips $ ruby -w -Ilib:test:bundler/lib --disable-gems \ -e 'require "rubygems"; require "minitest/autorun"' \ -e 'require "test_gem_command_common_options"' \ -e 'require "rubygems/test_gem_command"' ...............F 1) Failure: TestGemCommandCommonOptions#test_common_options […/test_gem_command_common_options.rb:7]: Expected ["-x"] to include "-h". zsh: exit 1 I removed a few lines from minitest output, but I hope this helps. Now for a more concrete example: https://github.com/rubygems/rubygems/blob/8c091b68/lib/rubygems/commands/setup_command.rb#L572-L573 If we want to test `Gem::Commands::SetupCommand#execute`, we are forced to use `--silent` common option, but test setup for `TestGemCommand` would have removed it. We could "reimplement" original common options in `TestGemCommandsSetupCommand#setup` but I'm not sure it feels right? In #2099, first commit, you'll find an example of the test I wanted to write. An alternative would be to "inject" the pristine command as a test double, but I'm not sure it matches current testing style in RubyGems.

…

-- Thibault Jouan

[ghost]

Sorry for previous comment formatting, I tried to follow "github
markdown", but seems ignored when replying by mail. Here is the
main example again:

# test/test_gem_command_common_options.rb
require 'rubygems/test_case'
require 'rubygems/command'

class TestGemCommandCommonOptions < Gem::TestCase
  def test_common_options
    opts = Gem::Command.common_options.map { |options| options[0][0] }
    assert_includes opts, '-h'
  end
end

$ ruby -w -Ilib:test:bundler/lib --disable-gems \
  -e 'require "rubygems"; require "minitest/autorun"' \
  -e 'require "test_gem_command_common_options"'
.
1 tests, 2 assertions, 0 failures, 0 errors, 0 skips

$ ruby -w -Ilib:test:bundler/lib --disable-gems \
  -e 'require "rubygems"; require "minitest/autorun"' \
  -e 'require "test_gem_command_common_options"' \
  -e 'require "rubygems/test_gem_command"'
...............F
  1) Failure:
TestGemCommandCommonOptions#test_common_options […/test_gem_command_common_options.rb:7]:
Expected ["-x"] to include "-h".
zsh: exit 1

The first run shows that the example test
TestGemCommandCommonOptions passes when run in isolation.
The second run includes TestGemCommand and shows that it modifies
behavior globally, which affects other test cases.

[segiddins]

Cool, thanks for the explanation!
@bundlerbot r+

[bundlerbot]

📌 Commit a1336fa has been approved by segiddins

[bundlerbot]

⌛ Testing commit a1336fa with merge 4a04596...

[bundlerbot]

☀️ Test successful - status-travis
Approved by: segiddins
Pushing 4a04596 to master...