Merge pull request #19571 from kaspth/improve-runner-integration

Improve Test Runner's Minitest integration.

10 files changed, 234 insertions, 293 deletions

diff --git a/railties/lib/rails/commands/test.rb b/railties/lib/rails/commands/test.rb
index 598e224a6f..fe5307788a 100644
--- a/railties/lib/rails/commands/test.rb
+++ b/railties/lib/rails/commands/test.rb
@@ -1,5 +1,5 @@
-require "rails/test_unit/runner"
+require "rails/test_unit/minitest_plugin"
 
 $: << File.expand_path("../../test", APP_PATH)
 
-Rails::TestRunner.run(ARGV)
+exit Minitest.run(ARGV)
diff --git a/railties/lib/rails/test_help.rb b/railties/lib/rails/test_help.rb
index a83e39faee..3b444b0932 100644
--- a/railties/lib/rails/test_help.rb
+++ b/railties/lib/rails/test_help.rb
@@ -3,13 +3,17 @@
 abort("Abort testing: Your Rails environment is running in production mode!") if Rails.env.production?
 
 require "rails/test_unit/minitest_plugin"
-require 'active_support/testing/autorun'
 require 'active_support/test_case'
 require 'action_controller'
 require 'action_controller/test_case'
 require 'action_dispatch/testing/integration'
 require 'rails/generators/test_case'
 
+unless Minitest.run_with_rails_extension
+  Minitest.run_with_autorun = true
+  require 'active_support/testing/autorun'
+end
+
 if defined?(ActiveRecord::Base)
   ActiveRecord::Migration.maintain_test_schema!
 
diff --git a/railties/lib/rails/test_unit/minitest_plugin.rb b/railties/lib/rails/test_unit/minitest_plugin.rb
index 70ce9d3360..421f032d81 100644
--- a/railties/lib/rails/test_unit/minitest_plugin.rb
+++ b/railties/lib/rails/test_unit/minitest_plugin.rb
@@ -1,14 +1,51 @@
-require "minitest"
+require "active_support/core_ext/module/attribute_accessors"
 require "rails/test_unit/reporter"
+require "rails/test_unit/test_requirer"
 
-def Minitest.plugin_rails_init(options)
-  self.reporter << Rails::TestUnitReporter.new(options[:io], options)
-  if $rails_test_runner && (method = $rails_test_runner.find_method)
-    options[:filter] = method
+module Minitest
+  def self.plugin_rails_options(opts, options)
+    opts.separator ""
+    opts.separator "Usage: bin/rails test [options] [files or directories]"
+    opts.separator "You can run a single test by appending a line number to a filename:"
+    opts.separator ""
+    opts.separator "    bin/rails test test/models/user_test.rb:27"
+    opts.separator ""
+    opts.separator "You can run multiple files and directories at the same time:"
+    opts.separator ""
+    opts.separator "    bin/rails test test/controllers test/integration/login_test.rb"
+    opts.separator ""
+
+    opts.separator "Rails options:"
+    opts.on("-e", "--environment [ENV]",
+            "Run tests in the ENV environment") do |env|
+      options[:environment] = env.strip
+    end
+
+    opts.on("-b", "--backtrace",
+            "Show the complete backtrace") do
+      options[:full_backtrace] = true
+    end
+
+    options[:patterns] = opts.order!
   end
 
-  if !($rails_test_runner && $rails_test_runner.show_backtrace?)
-    Minitest.backtrace_filter = Rails.backtrace_cleaner
+  def self.plugin_rails_init(options)
+    self.run_with_rails_extension = true
+
+    ENV["RAILS_ENV"] = options[:environment] || "test"
+
+    Rails::TestRequirer.require_files options[:patterns] unless run_with_autorun
+
+    unless options[:full_backtrace] || ENV["BACKTRACE"]
+      # Plugin can run without Rails loaded, check before filtering.
+      Minitest.backtrace_filter = Rails.backtrace_cleaner if Rails.respond_to?(:backtrace_cleaner)
+    end
+
+    self.reporter << Rails::TestUnitReporter.new(options[:io], options)
   end
+
+  mattr_accessor(:run_with_autorun)         { false }
+  mattr_accessor(:run_with_rails_extension) { false }
 end
+
 Minitest.extensions << 'rails'
diff --git a/railties/lib/rails/test_unit/reporter.rb b/railties/lib/rails/test_unit/reporter.rb
index 64e99626eb..bfdc9754d4 100644
--- a/railties/lib/rails/test_unit/reporter.rb
+++ b/railties/lib/rails/test_unit/reporter.rb
@@ -15,8 +15,12 @@ module Rails
       filtered_results.reject!(&:skipped?) unless options[:verbose]
       filtered_results.map do |result|
         location, line = result.method(result.name).source_location
-        "bin/rails test #{location}:#{line}"
+        "bin/rails test #{relative_path_for(location)}:#{line}"
       end.join "\n"
     end
+
+    def relative_path_for(file)
+      file.sub(/^#{Rails.root}\/?/, '')
+    end
   end
 end
diff --git a/railties/lib/rails/test_unit/runner.rb b/railties/lib/rails/test_unit/runner.rb
deleted file mode 100644
index 5573fa6904..0000000000
--- a/railties/lib/rails/test_unit/runner.rb
+++ /dev/null
@@ -1,137 +0,0 @@
-require "optparse"
-require "rake/file_list"
-require "method_source"
-
-module Rails
-  class TestRunner
-    class Options
-      def self.parse(args)
-        options = { backtrace: !ENV["BACKTRACE"].nil?, name: nil, environment: "test" }
-
-        opt_parser = ::OptionParser.new do |opts|
-          opts.banner = "Usage: bin/rails test [options] [file or directory]"
-
-          opts.separator ""
-          opts.on("-e", "--environment [ENV]",
-                  "Run tests in the ENV environment") do |env|
-            options[:environment] = env.strip
-          end
-          opts.separator ""
-          opts.separator "Filter options:"
-          opts.separator ""
-          opts.separator <<-DESC
-  You can run a single test by appending the line number to filename:
-
-    bin/rails test test/models/user_test.rb:27
-
-          DESC
-
-          opts.on("-n", "--name [NAME]",
-                  "Only run tests matching NAME") do |name|
-            options[:name] = name
-          end
-          opts.on("-p", "--pattern [PATTERN]",
-                  "Only run tests matching PATTERN") do |pattern|
-            options[:name] = "/#{pattern}/"
-          end
-
-          opts.separator ""
-          opts.separator "Output options:"
-
-          opts.on("-b", "--backtrace",
-                  "Show the complete backtrace") do
-            options[:backtrace] = true
-          end
-
-          opts.separator ""
-          opts.separator "Common options:"
-
-          opts.on_tail("-h", "--help", "Show this message") do
-            puts opts
-            exit
-          end
-        end
-
-        opt_parser.order!(args)
-
-        options[:patterns] = []
-        while arg = args.shift
-          if (file_and_line = arg.split(':')).size > 1
-            options[:filename], options[:line] = file_and_line
-            options[:filename] = File.expand_path options[:filename]
-            options[:line] &&= options[:line].to_i
-          else
-            arg = arg.gsub(':', '')
-            if Dir.exist?("#{arg}")
-              options[:patterns] << File.expand_path("#{arg}/**/*_test.rb")
-            elsif File.file?(arg)
-              options[:patterns] << File.expand_path(arg)
-            end
-          end
-        end
-        options
-      end
-    end
-
-    def initialize(options = {})
-      @options = options
-    end
-
-    def self.run(arguments)
-      options = Rails::TestRunner::Options.parse(arguments)
-      Rails::TestRunner.new(options).run
-    end
-
-    def run
-      $rails_test_runner = self
-      ENV["RAILS_ENV"] = @options[:environment]
-      run_tests
-    end
-
-    def find_method
-      return @options[:name] if @options[:name]
-      return unless @options[:line]
-      method = test_methods.find do |location, test_method, start_line, end_line|
-        location == @options[:filename] &&
-          (start_line..end_line).include?(@options[:line].to_i)
-      end
-      method[1] if method
-    end
-
-    def show_backtrace?
-      @options[:backtrace]
-    end
-
-    def test_files
-      return [@options[:filename]] if @options[:filename]
-      if @options[:patterns] && @options[:patterns].count > 0
-        pattern = @options[:patterns]
-      else
-        pattern = "test/**/*_test.rb"
-      end
-      Rake::FileList[pattern]
-    end
-
-    private
-    def run_tests
-      test_files.to_a.each do |file|
-        require File.expand_path file
-      end
-    end
-
-    def test_methods
-      methods_map = []
-      suites = Minitest::Runnable.runnables.shuffle
-      suites.each do |suite_class|
-        suite_class.runnable_methods.each do |test_method|
-          method = suite_class.instance_method(test_method)
-          location = method.source_location
-          start_line = location.last
-          end_line = method.source.split("\n").size + start_line - 1
-          methods_map << [File.expand_path(location.first), test_method, start_line, end_line]
-        end
-      end
-      methods_map
-    end
-  end
-end
diff --git a/railties/lib/rails/test_unit/test_requirer.rb b/railties/lib/rails/test_unit/test_requirer.rb
new file mode 100644
index 0000000000..84c2256729
--- /dev/null
+++ b/railties/lib/rails/test_unit/test_requirer.rb
@@ -0,0 +1,28 @@
+require 'active_support/core_ext/object/blank'
+require 'rake/file_list'
+
+module Rails
+  class TestRequirer # :nodoc:
+    class << self
+      def require_files(patterns)
+        patterns = expand_patterns(patterns)
+
+        Rake::FileList[patterns.compact.presence || 'test/**/*_test.rb'].to_a.each do |file|
+          require File.expand_path(file)
+        end
+      end
+
+      private
+        def expand_patterns(patterns)
+          patterns.map do |arg|
+            arg = arg.gsub(/:(\d+)?$/, '')
+            if Dir.exist?(arg)
+              "#{arg}/**/*_test.rb"
+            elsif File.file?(arg)
+              arg
+            end
+          end
+        end
+    end
+  end
+end
diff --git a/railties/lib/rails/test_unit/testing.rake b/railties/lib/rails/test_unit/testing.rake
index 0f26621b59..dda492f974 100644
--- a/railties/lib/rails/test_unit/testing.rake
+++ b/railties/lib/rails/test_unit/testing.rake
@@ -1,12 +1,13 @@
-require "rails/test_unit/runner"
+gem 'minitest'
+require 'minitest'
+require 'rails/test_unit/minitest_plugin'
 
 task default: :test
 
 desc "Runs all tests in test folder"
 task :test do
   $: << "test"
-  args = ARGV[0] == "test" ? ARGV[1..-1] : []
-  Rails::TestRunner.run(args)
+  Minitest.run(['test'])
 end
 
 namespace :test do
@@ -23,22 +24,22 @@ namespace :test do
   ["models", "helpers", "controllers", "mailers", "integration", "jobs"].each do |name|
     task name => "test:prepare" do
       $: << "test"
-      Rails::TestRunner.run(["test/#{name}"])
+      Minitest.run(["test/#{name}"])
     end
   end
 
   task :generators => "test:prepare" do
     $: << "test"
-    Rails::TestRunner.run(["test/lib/generators"])
+    Minitest.run(["test/lib/generators"])
   end
 
   task :units => "test:prepare" do
     $: << "test"
-    Rails::TestRunner.run(["test/models", "test/helpers", "test/unit"])
+    Minitest.run(["test/models", "test/helpers", "test/unit"])
   end
 
   task :functionals => "test:prepare" do
     $: << "test"
-    Rails::TestRunner.run(["test/controllers", "test/mailers", "test/functional"])
+    Minitest.run(["test/controllers", "test/mailers", "test/functional"])
   end
 end
diff --git a/railties/test/application/test_runner_test.rb b/railties/test/application/test_runner_test.rb
index c122b315c0..bbaab42a7f 100644
--- a/railties/test/application/test_runner_test.rb
+++ b/railties/test/application/test_runner_test.rb
@@ -1,9 +1,10 @@
 require 'isolation/abstract_unit'
 require 'active_support/core_ext/string/strip'
+require 'env_helpers'
 
 module ApplicationTests
   class TestRunnerTest < ActiveSupport::TestCase
-    include ActiveSupport::Testing::Isolation
+    include ActiveSupport::Testing::Isolation, EnvHelpers
 
     def setup
       build_app
@@ -14,20 +15,6 @@ module ApplicationTests
       teardown_app
     end
 
-    def test_run_in_test_environment
-      app_file 'test/unit/env_test.rb', <<-RUBY
-        require 'test_helper'
-
-        class EnvTest < ActiveSupport::TestCase
-          def test_env
-            puts "Current Environment: \#{Rails.env}"
-          end
-        end
-      RUBY
-
-      assert_match "Current Environment: test", run_test_command('test/unit/env_test.rb')
-    end
-
     def test_run_single_file
       create_test_file :models, 'foo'
       create_test_file :models, 'bar'
@@ -187,7 +174,7 @@ module ApplicationTests
         end
       RUBY
 
-      run_test_command('-p rikka test/unit/chu_2_koi_test.rb').tap do |output|
+      run_test_command('-n /rikka/ test/unit/chu_2_koi_test.rb').tap do |output|
         assert_match "Rikka", output
         assert_no_match "Sanae", output
       end
@@ -229,24 +216,128 @@ module ApplicationTests
       assert_match "development", run_test_command('test/unit/env_test.rb')
     end
 
+    def test_run_in_test_environment_by_default
+      create_env_test
+
+      assert_match "Current Environment: test", run_test_command('test/unit/env_test.rb')
+    end
+
     def test_run_different_environment
-      env = "development"
-      app_file 'test/unit/env_test.rb', <<-RUBY
+      create_env_test
+
+      assert_match "Current Environment: development",
+        run_test_command("-e development test/unit/env_test.rb")
+    end
+
+    def test_generated_scaffold_works_with_rails_test
+      create_scaffold
+      assert_match "0 failures, 0 errors, 0 skips", run_test_command('')
+    end
+
+    def test_run_multiple_folders
+      create_test_file :models, 'account'
+      create_test_file :controllers, 'accounts_controller'
+
+      run_test_command('test/models test/controllers').tap do |output|
+        assert_match 'AccountTest', output
+        assert_match 'AccountsControllerTest', output
+        assert_match '2 runs, 2 assertions, 0 failures, 0 errors, 0 skips', output
+      end
+    end
+
+    def test_run_with_ruby_command
+      app_file 'test/models/post_test.rb', <<-RUBY
         require 'test_helper'
 
-        class EnvTest < ActiveSupport::TestCase
-          def test_env
-            puts Rails.env
+        class PostTest < ActiveSupport::TestCase
+          test 'declarative syntax works' do
+            puts 'PostTest'
+            assert true
           end
         end
       RUBY
 
-      assert_match env, run_test_command("-e #{env} test/unit/env_test.rb")
+      Dir.chdir(app_path) do
+        `ruby -Itest test/models/post_test.rb`.tap do |output|
+          assert_match 'PostTest', output
+          assert_no_match 'is already defined in', output
+        end
+      end
     end
 
-    def test_generated_scaffold_works_with_rails_test
-      create_scaffold
-      assert_match "0 failures, 0 errors, 0 skips", run_test_command('')
+    def test_mix_files_and_line_filters
+      create_test_file :models, 'account'
+      app_file 'test/models/post_test.rb', <<-RUBY
+        require 'test_helper'
+
+        class PostTest < ActiveSupport::TestCase
+          def test_post
+            puts 'PostTest'
+            assert true
+          end
+
+          def test_line_filter_does_not_run_this
+            assert true
+          end
+        end
+      RUBY
+
+      run_test_command('test/models/account_test.rb test/models/post_test.rb:4').tap do |output|
+        assert_match 'AccountTest', output
+        assert_match 'PostTest', output
+        assert_match '2 runs, 2 assertions', output
+      end
+    end
+
+    def test_multiple_line_filters
+      create_test_file :models, 'account'
+      create_test_file :models, 'post'
+
+      run_test_command('test/models/account_test.rb:4 test/models/post_test.rb:4').tap do |output|
+        assert_match 'AccountTest', output
+        assert_match 'PostTest', output
+      end
+    end
+
+    def test_line_filter_without_line_runs_all_tests
+      create_test_file :models, 'account'
+
+      run_test_command('test/models/account_test.rb:').tap do |output|
+        assert_match 'AccountTest', output
+      end
+    end
+
+    def test_shows_filtered_backtrace_by_default
+      create_backtrace_test
+
+      assert_match 'Rails::BacktraceCleaner', run_test_command('test/unit/backtrace_test.rb')
+    end
+
+    def test_backtrace_option
+      create_backtrace_test
+
+      assert_match 'Minitest::BacktraceFilter', run_test_command('test/unit/backtrace_test.rb -b')
+      assert_match 'Minitest::BacktraceFilter',
+        run_test_command('test/unit/backtrace_test.rb --backtrace')
+    end
+
+    def test_show_full_backtrace_using_backtrace_environment_variable
+      create_backtrace_test
+
+      switch_env 'BACKTRACE', 'true' do
+        assert_match 'Minitest::BacktraceFilter', run_test_command('test/unit/backtrace_test.rb')
+      end
+    end
+
+    def test_run_app_without_rails_loaded
+      # Simulate a real Rails app boot.
+      app_file 'config/boot.rb', <<-RUBY
+        ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../../Gemfile', __FILE__)
+
+        require 'bundler/setup' # Set up gems listed in the Gemfile.
+      RUBY
+
+      assert_match '0 runs, 0 assertions', run_test_command('')
     end
 
     private
@@ -284,6 +375,18 @@ module ApplicationTests
         RUBY
       end
 
+      def create_backtrace_test
+        app_file 'test/unit/backtrace_test.rb', <<-RUBY
+          require 'test_helper'
+
+          class BacktraceTest < ActiveSupport::TestCase
+            def test_backtrace
+              puts Minitest.backtrace_filter
+            end
+          end
+        RUBY
+      end
+
       def create_schema
         app_file 'db/schema.rb', ''
       end
@@ -301,6 +404,18 @@ module ApplicationTests
         RUBY
       end
 
+      def create_env_test
+        app_file 'test/unit/env_test.rb', <<-RUBY
+          require 'test_helper'
+
+          class EnvTest < ActiveSupport::TestCase
+            def test_env
+              puts "Current Environment: \#{Rails.env}"
+            end
+          end
+        RUBY
+      end
+
       def create_scaffold
         script 'generate scaffold user name:string'
         Dir.chdir(app_path) { File.exist?('app/models/user.rb') }
diff --git a/railties/test/application/test_test.rb b/railties/test/application/test_test.rb
index bb30940a74..0e997f4ba7 100644
--- a/railties/test/application/test_test.rb
+++ b/railties/test/application/test_test.rb
@@ -64,8 +64,8 @@ module ApplicationTests
       RUBY
 
       output = run_test_file('unit/failing_test.rb', env: { "BACKTRACE" => "1" })
-      assert_match %r{/app/test/unit/failing_test\.rb}, output
-      assert_match %r{/app/test/unit/failing_test\.rb:4}, output
+      assert_match %r{test/unit/failing_test\.rb}, output
+      assert_match %r{test/unit/failing_test\.rb:4}, output
     end
 
     test "ruby schema migrations" do
diff --git a/railties/test/test_unit/runner_test.rb b/railties/test/test_unit/runner_test.rb
deleted file mode 100644
index 9ea8b2c114..0000000000
--- a/railties/test/test_unit/runner_test.rb
+++ /dev/null
@@ -1,111 +0,0 @@
-require 'abstract_unit'
-require 'env_helpers'
-require 'rails/test_unit/runner'
-
-class TestUnitTestRunnerTest < ActiveSupport::TestCase
-  include EnvHelpers
-
-  setup do
-    @options = Rails::TestRunner::Options
-  end
-
-  test "shows the filtered backtrace by default" do
-    options = @options.parse([])
-    assert_not options[:backtrace]
-  end
-
-  test "has --backtrace (-b) option to show the full backtrace" do
-    options = @options.parse(["-b"])
-    assert options[:backtrace]
-
-    options = @options.parse(["--backtrace"])
-    assert options[:backtrace]
-  end
-
-  test "show full backtrace using BACKTRACE environment variable" do
-    switch_env "BACKTRACE", "true" do
-      options = @options.parse([])
-      assert options[:backtrace]
-    end
-  end
-
-  test "tests run in the test environment by default" do
-    options = @options.parse([])
-    assert_equal "test", options[:environment]
-  end
-
-  test "can run in a specific environment" do
-    options = @options.parse(["-e development"])
-    assert_equal "development", options[:environment]
-  end
-
-  test "parse the filename and line" do
-    file = "test/test_unit/runner_test.rb"
-    absolute_file = File.expand_path __FILE__
-    options = @options.parse(["#{file}:20"])
-    assert_equal absolute_file, options[:filename]
-    assert_equal 20, options[:line]
-
-    options = @options.parse(["#{file}:"])
-    assert_equal [absolute_file], options[:patterns]
-    assert_nil options[:line]
-
-    options = @options.parse([file])
-    assert_equal [absolute_file], options[:patterns]
-    assert_nil options[:line]
-  end
-
-  test "find_method on same file" do
-    options = @options.parse(["#{__FILE__}:#{__LINE__}"])
-    runner = Rails::TestRunner.new(options)
-    assert_equal "test_find_method_on_same_file", runner.find_method
-  end
-
-  test "find_method on a different file" do
-    options = @options.parse(["foobar.rb:#{__LINE__}"])
-    runner = Rails::TestRunner.new(options)
-    assert_nil runner.find_method
-  end
-
-  test "run all tests in a directory" do
-    options = @options.parse([__dir__])
-
-    assert_equal ["#{__dir__}/**/*_test.rb"], options[:patterns]
-    assert_nil options[:filename]
-    assert_nil options[:line]
-  end
-
-  test "run multiple folders" do
-    application_dir = File.expand_path("#{__dir__}/../application")
-
-    options = @options.parse([__dir__, application_dir])
-
-    assert_equal ["#{__dir__}/**/*_test.rb", "#{application_dir}/**/*_test.rb"], options[:patterns]
-    assert_nil options[:filename]
-    assert_nil options[:line]
-
-    runner = Rails::TestRunner.new(options)
-    assert runner.test_files.size > 0
-  end
-
-  test "run multiple files and run one file by line" do
-    line = __LINE__
-    absolute_file = File.expand_path(__FILE__)
-    options = @options.parse([__dir__, "#{__FILE__}:#{line}"])
-
-    assert_equal ["#{__dir__}/**/*_test.rb"], options[:patterns]
-    assert_equal absolute_file, options[:filename]
-    assert_equal line, options[:line]
-
-    runner = Rails::TestRunner.new(options)
-    assert_equal [absolute_file], runner.test_files, 'Only returns the file that running by line'
-  end
-
-  test "running multiple files passing line number" do
-    line = __LINE__
-    options = @options.parse(["foobar.rb:8", "#{__FILE__}:#{line}"])
-
-    assert_equal File.expand_path(__FILE__), options[:filename], 'Returns the last file'
-    assert_equal line, options[:line]
-  end
-end