Merge pull request #723 from ruby/fix/testopts-preserve-existing-value

Preserve `ENV["TESTOPTS"]` when verbose is enabled

Author: hsbt

lib/rake/file_utils_ext.rb

@@ -53,7 +53,7 @@ def #{name}(*args, **options, &block)
     def verbose(value=nil)
       oldvalue = FileUtilsExt.verbose_flag
       FileUtilsExt.verbose_flag = value unless value.nil?
-      ENV["TESTOPTS"] = "-v" if value
+
       if block_given?
         begin
           yield

lib/rake/testtask.rb

@@ -108,10 +108,11 @@ def initialize(name=:test)
     def define
       desc @description
       task @name => Array(deps) do
-        FileUtilsExt.verbose(@verbose) do
+        effective_verbose = @verbose || FileUtilsExt.verbose_flag == true
+        FileUtilsExt.verbose(effective_verbose) do
           args =
             "#{ruby_opts_string} #{run_code} " +
-            "#{file_list_string} #{option_list}"
+            "#{file_list_string} #{option_list(verbose: effective_verbose)}"
           ruby args do |ok, status|
             if !ok && status.respond_to?(:signaled?) && status.signaled?
               raise SignalException.new(status.termsig)
@@ -133,13 +134,17 @@ def define
       self
     end
 
-    def option_list # :nodoc:
-      (ENV["TESTOPTS"] ||
+    def option_list(verbose: @verbose) # :nodoc:
+      opts = ENV["TESTOPTS"] ||
         ENV["TESTOPT"] ||
         ENV["TEST_OPTS"] ||
         ENV["TEST_OPT"] ||
         @options ||
-        "")
+        ""
+      if verbose && !opts.split.include?("-v")
+        opts = opts.empty? ? "-v" : "#{opts} -v"
+      end
+      opts
     end
 
     def ruby_opts_string # :nodoc:

test/test_rake_file_utils.rb

@@ -12,7 +12,6 @@ def teardown
     FileUtils::LN_SUPPORTED[0] = true
     RakeFileUtils.verbose_flag = Rake::FileUtilsExt::DEFAULT
     ENV["RAKE_TEST_SH"] = @rake_test_sh
-    ENV["TESTOPTS"] = nil
 
     super
   end
@@ -107,12 +106,9 @@ def test_safe_ln_fails_on_script_error
   def test_verbose
     verbose true
     assert_equal true, verbose
-    assert_equal "-v", ENV["TESTOPTS"]
 
-    ENV["TESTOPTS"] = nil
     verbose false
     assert_equal false, verbose
-    assert_equal nil, ENV["TESTOPTS"]
 
     verbose(true) {
       assert_equal true, verbose

test/test_rake_test_task.rb

@@ -5,6 +5,21 @@
 class TestRakeTestTask < Rake::TestCase # :nodoc:
   include Rake
 
+  def setup
+    super
+    @_previous_testopts = ENV["TESTOPTS"]
+    ENV.delete "TESTOPTS"
+  end
+
+  def teardown
+    if @_previous_testopts.nil?
+      ENV.delete "TESTOPTS"
+    else
+      ENV["TESTOPTS"] = @_previous_testopts
+    end
+    super
+  end
+
   def test_initialize
     tt = Rake::TestTask.new do |t| end
     refute_nil tt
@@ -191,6 +206,34 @@ def test_task_order_only_prerequisites
     assert_equal [b, c], t.prerequisite_tasks
   end
 
+  def test_option_list_verbose_without_testopts
+    tt = Rake::TestTask.new { |t| t.verbose = true }
+    assert_equal "-v", tt.option_list
+  end
+
+  def test_option_list_verbose_with_testopts
+    ENV["TESTOPTS"] = "--ci-reporter"
+    tt = Rake::TestTask.new { |t| t.verbose = true }
+    assert_equal "--ci-reporter -v", tt.option_list
+  end
+
+  def test_option_list_not_verbose_with_testopts
+    ENV["TESTOPTS"] = "--ci-reporter"
+    tt = Rake::TestTask.new { |t| t.verbose = false }
+    assert_equal "--ci-reporter", tt.option_list
+  end
+
+  def test_option_list_skips_duplicate_v
+    ENV["TESTOPTS"] = "-v --ci-reporter"
+    tt = Rake::TestTask.new { |t| t.verbose = true }
+    assert_equal "-v --ci-reporter", tt.option_list
+  end
+
+  def test_option_list_verbose_keyword_overrides
+    tt = Rake::TestTask.new { |t| t.verbose = false }
+    assert_equal "-v", tt.option_list(verbose: true)
+  end
+
   def test_task_order_only_prerequisites_key
     t = task "a" => "b", order_only: ["c"]
     b, c = task("b"), task("c")