Revert "* Don't eagerly require Rails' minitest plugin."

This reverts commit 796a1cf0.

activerecord/lib/active_record/tasks/database_tasks.rb

@@ -260,7 +260,7 @@ def load_schema_current(format = ActiveRecord::Base.schema_format, file = nil, e
       def check_schema_file(filename)
         unless File.exist?(filename)
           message = %{#{filename} doesn't exist yet. Run `rails db:migrate` to create it, then try again.}
-          message << %{ If you do not intend to use a database, you should instead alter #{Rails.root}/config/application.rb to limit the frameworks that will be loaded.} if defined?(::Rails.root)
+          message << %{ If you do not intend to use a database, you should instead alter #{Rails.root}/config/application.rb to limit the frameworks that will be loaded.} if defined?(::Rails)
           Kernel.abort message
         end
       end

activesupport/lib/active_support/testing/autorun.rb

@@ -2,4 +2,8 @@
 
 require 'minitest'
 
+if Minitest.respond_to?(:run_via) && !Minitest.run_via.set?
+  Minitest.run_via = :ruby
+end
+
 Minitest.autorun

railties/CHANGELOG.md

-*   Make Rails' test runner work better with minitest plugins.
-
-    By demoting the Rails test runner to just another minitest plugin —
-    and thereby not eager loading it — we can co-exist much better with
-    other minitest plugins such as pride and minitest-focus.
-
-    *Kasper Timm Hansen*
-
 ## Rails 5.0.4 (June 19, 2017) ##
 
 *   No changes.

railties/lib/rails/test_help.rb

@@ -2,6 +2,7 @@
 # so fixtures aren't loaded into that environment
 abort("Abort testing: Your Rails environment is running in production mode!") if Rails.env.production?
 
+require "rails/test_unit/minitest_plugin"
 require 'active_support/test_case'
 require 'action_controller'
 require 'action_controller/test_case'

railties/lib/rails/test_unit/line_filtering.rb

 require 'method_source'
-require "rails/test_unit/runner"
 
 module Rails
   module LineFiltering # :nodoc:
     def run(reporter, options = {})
-      options[:filter] = Rails::TestUnit::Runner.compose_filter(self, options[:filter])
+      if options[:patterns] && options[:patterns].any? { |p| p =~ /:\d+/ }
+        options[:filter] = \
+          CompositeFilter.new(self, options[:filter], options[:patterns])
+      end
 
       super
     end
   end
+
+  class CompositeFilter # :nodoc:
+    attr_reader :named_filter
+
+    def initialize(runnable, filter, patterns)
+      @runnable = runnable
+      @named_filter = derive_named_filter(filter)
+      @filters = [ @named_filter, *derive_line_filters(patterns) ].compact
+    end
+
+    # Minitest uses === to find matching filters.
+    def ===(method)
+      @filters.any? { |filter| filter === method }
+    end
+
+    private
+      def derive_named_filter(filter)
+        if filter.respond_to?(:named_filter)
+          filter.named_filter
+        elsif filter =~ %r%/(.*)/% # Regexp filtering copied from Minitest.
+          Regexp.new $1
+        elsif filter.is_a?(String)
+          filter
+        end
+      end
+
+      def derive_line_filters(patterns)
+        patterns.flat_map do |file_and_line|
+          file, *lines = file_and_line.split(':')
+
+          if lines.empty?
+            Filter.new(@runnable, file, nil) if file
+          else
+            lines.map { |line| Filter.new(@runnable, file, line) }
+          end
+        end
+      end
+  end
+
+  class Filter # :nodoc:
+    def initialize(runnable, file, line)
+      @runnable, @file = runnable, File.expand_path(file)
+      @line = line.to_i if line
+    end
+
+    def ===(method)
+      return unless @runnable.method_defined?(method)
+
+      if @line
+        test_file, test_range = definition_for(@runnable.instance_method(method))
+        test_file == @file && test_range.include?(@line)
+      else
+        @runnable.instance_method(method).source_location.first == @file
+      end
+    end
+
+    private
+      def definition_for(method)
+        file, start_line = method.source_location
+        end_line = method.source.count("\n") + start_line - 1
+
+        return file, start_line..end_line
+      end
+  end
 end

railties/lib/minitest/rails_plugin.rb → railties/lib/rails/test_unit/minitest_plugin.rb

 require "active_support/core_ext/module/attribute_accessors"
 require "rails/test_unit/reporter"
+require "rails/test_unit/test_requirer"
+require 'shellwords'
 
 module Minitest
   class SuppressedSummaryReporter < SummaryReporter
@@ -10,29 +12,80 @@ def aggregated_results(*)
   end
 
   def self.plugin_rails_options(opts, options)
-    opts.on("-b", "--backtrace", "Show the complete backtrace") do
+    executable = ::Rails::TestUnitReporter.executable
+    opts.separator ""
+    opts.separator "Usage: #{executable} [options] [files or directories]"
+    opts.separator "You can run a single test by appending a line number to a filename:"
+    opts.separator ""
+    opts.separator "    #{executable} 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 "    #{executable} test/controllers test/integration/login_test.rb"
+    opts.separator ""
+    opts.separator "By default test failures and errors are reported inline during a run."
+    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
 
-    opts.on("-d", "--defer-output", "Output test failures and errors after the test run") do
+    opts.on("-d", "--defer-output",
+            "Output test failures and errors after the test run") do
       options[:output_inline] = false
     end
 
-    opts.on("-f", "--fail-fast", "Abort test run on first failure or error") do
+    opts.on("-f", "--fail-fast",
+            "Abort test run on first failure or error") do
       options[:fail_fast] = true
     end
 
-    opts.on("-c", "--[no-]color", "Enable color in the output") do |value|
+    opts.on("-c", "--[no-]color",
+            "Enable color in the output") do |value|
       options[:color] = value
     end
 
     options[:color] = true
     options[:output_inline] = true
+    options[:patterns] = opts.order! unless run_via.rake?
   end
 
+  def self.rake_run(patterns) # :nodoc:
+    self.run_via = :rake unless run_via.set?
+    ::Rails::TestRequirer.require_files(patterns)
+    autorun
+  end
+
+  module RunRespectingRakeTestopts
+    def run(args = [])
+      if run_via.rake?
+        args = Shellwords.split(ENV["TESTOPTS"] || "")
+      end
+
+      super
+    end
+  end
+
+  singleton_class.prepend RunRespectingRakeTestopts
+
   # Owes great inspiration to test runner trailblazers like RSpec,
   # minitest-reporters, maxitest and others.
   def self.plugin_rails_init(options)
+    ENV["RAILS_ENV"] = options[:environment] || "test"
+
+    # If run via `ruby` we've been passed the files to run directly, or if run
+    # via `rake` then they have already been eagerly required.
+    unless run_via.ruby? || run_via.rake?
+      ::Rails::TestRequirer.require_files(options[:patterns])
+    end
+
     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)
@@ -44,6 +97,38 @@ def self.plugin_rails_init(options)
     reporter << ::Rails::TestUnitReporter.new(options[:io], options)
   end
 
-  # Backwardscompatibility with Rails 5.0 generated plugin test scripts
-  mattr_reader :run_via, default: {}
+  def self.run_via=(runner)
+    if run_via.set?
+      raise ArgumentError, "run_via already assigned"
+    else
+      run_via.runner = runner
+    end
+  end
+
+  class RunVia
+    attr_accessor :runner
+    alias set? runner
+
+    # Backwardscompatibility with Rails 5.0 generated plugin test scripts.
+    def []=(runner, *)
+      @runner = runner
+    end
+
+    def ruby?
+      runner == :ruby
+    end
+
+    def rake?
+      runner == :rake
+    end
+  end
+
+  mattr_reader(:run_via) { RunVia.new }
 end
+
+# Put Rails as the first plugin minitest initializes so other plugins
+# can override or replace our default reporter setup.
+# Since minitest only loads plugins if its extensions are empty we have
+# to call `load_plugins` first.
+Minitest.load_plugins
+Minitest.extensions.unshift 'rails'

railties/lib/rails/test_unit/runner.rb

-require "shellwords"
-require "method_source"
-require "rake/file_list"
-require "active_support/core_ext/module/attribute_accessors"
-
-module Rails
-  module TestUnit
-    class Runner
-      mattr_reader :filters, default: []
-
-      class << self
-        def options(opts)
-          opts.on("--warnings", "-w", "Run with Ruby warnings enabled") {}
-          opts.on("--environment", "-e", "Run tests in the ENV environment") {}
-        end
-
-        def parse_options(argv)
-          # Perform manual parsing and cleanup since option parser raises on unknown options.
-          env_index = argv.index("--environment") || argv.index("-e")
-          if env_index
-            argv.delete_at(env_index)
-            environment = argv.delete_at(env_index).strip
-          end
-          ENV["RAILS_ENV"] = environment || "test"
-
-          w_index = argv.index("--warnings") || argv.index("-w")
-          $VERBOSE = argv.delete_at(w_index) if w_index
-        end
-
-        def rake_run(argv = [])
-          ARGV.replace Shellwords.split(ENV["TESTOPTS"] || "")
-
-          run(argv)
-        end
-
-        def run(argv = [])
-          load_tests(argv)
-
-          require "active_support/testing/autorun"
-        end
-
-        def load_tests(argv)
-          patterns = extract_filters(argv)
-
-          tests = Rake::FileList[patterns.any? ? patterns : "test/**/*_test.rb"]
-          tests.exclude("test/system/**/*") if patterns.empty?
-
-          tests.to_a.each { |path| require File.expand_path(path) }
-        end
-
-        def compose_filter(runnable, filter)
-          if filters.any? { |_, lines| lines.any? }
-            CompositeFilter.new(runnable, filter, filters)
-          else
-            filter
-          end
-        end
-
-        private
-          def extract_filters(argv)
-            argv.select { |arg| arg =~ /^\w+\// }.map do |path|
-              case
-              when path =~ /(:\d+)+$/
-                file, *lines = path.split(":")
-                filters << [ file, lines ]
-                file
-              when Dir.exist?(path)
-                "#{path}/**/*_test.rb"
-              else
-                filters << [ path, [] ]
-                path
-              end
-            end
-          end
-      end
-    end
-
-    class CompositeFilter # :nodoc:
-      attr_reader :named_filter
-
-      def initialize(runnable, filter, patterns)
-        @runnable = runnable
-        @named_filter = derive_named_filter(filter)
-        @filters = [ @named_filter, *derive_line_filters(patterns) ].compact
-      end
-
-      # Minitest uses === to find matching filters.
-      def ===(method)
-        @filters.any? { |filter| filter === method }
-      end
-
-      private
-        def derive_named_filter(filter)
-          if filter.respond_to?(:named_filter)
-            filter.named_filter
-          elsif filter =~ %r%/(.*)/% # Regexp filtering copied from Minitest.
-            Regexp.new $1
-          elsif filter.is_a?(String)
-            filter
-          end
-        end
-
-        def derive_line_filters(patterns)
-          patterns.flat_map do |file, lines|
-            if lines.empty?
-              Filter.new(@runnable, file, nil) if file
-            else
-              lines.map { |line| Filter.new(@runnable, file, line) }
-            end
-          end
-        end
-    end
-
-    class Filter # :nodoc:
-      def initialize(runnable, file, line)
-        @runnable, @file = runnable, File.expand_path(file)
-        @line = line.to_i if line
-      end
-
-      def ===(method)
-        return unless @runnable.method_defined?(method)
-
-        if @line
-          test_file, test_range = definition_for(@runnable.instance_method(method))
-          test_file == @file && test_range.include?(@line)
-        else
-          @runnable.instance_method(method).source_location.first == @file
-        end
-      end
-
-      private
-        def definition_for(method)
-          file, start_line = method.source_location
-          end_line = method.source.count("\n") + start_line - 1
-
-          return file, start_line..end_line
-        end
-    end
-  end
-end

railties/lib/rails/test_unit/test_requirer.rb

+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"
+            else
+              arg
+            end
+          end
+        end
+    end
+  end
+end

railties/lib/rails/test_unit/testing.rake

-gem "minitest"
-require "minitest"
-require_relative "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"
-
-  if ENV.key?("TEST")
-    Rails::TestUnit::Runner.rake_run([ENV["TEST"]])
-  else
-    Rails::TestUnit::Runner.rake_run
-  end
+  pattern = if ENV.key?('TEST')
+               ENV['TEST']
+             else
+               "test"
+             end
+  Minitest.rake_run([pattern])
 end
 
 namespace :test do
@@ -29,22 +29,22 @@ namespace :test do
   ["models", "helpers", "controllers", "mailers", "integration", "jobs"].each do |name|
     task name => "test:prepare" do
       $: << "test"
-      Rails::TestUnit::Runner.rake_run(["test/#{name}"])
+      Minitest.rake_run(["test/#{name}"])
     end
   end
 
   task :generators => "test:prepare" do
     $: << "test"
-    Rails::TestUnit::Runner.rake_run(["test/lib/generators"])
+    Minitest.rake_run(["test/lib/generators"])
   end
 
   task :units => "test:prepare" do
     $: << "test"
-    Rails::TestUnit::Runner.rake_run(["test/models", "test/helpers", "test/unit"])
+    Minitest.rake_run(["test/models", "test/helpers", "test/unit"])
   end
 
   task :functionals => "test:prepare" do
     $: << "test"
-    Rails::TestUnit::Runner.rake_run(["test/controllers", "test/mailers", "test/functional"])
+    Minitest.rake_run(["test/controllers", "test/mailers", "test/functional"])
   end
 end

railties/test/application/test_runner_test.rb

@@ -16,13 +16,13 @@ def teardown
     end
 
     def test_run_via_backwardscompatibility
-      require "minitest/rails_plugin"
+      require "rails/test_unit/minitest_plugin"
 
       assert_nothing_raised do
         Minitest.run_via[:ruby] = true
       end
 
-      assert Minitest.run_via[:ruby]
+      assert_predicate Minitest.run_via, :ruby?
     end
 
     def test_run_single_file

railties/test/test_unit/reporter_test.rb

@@ -70,7 +70,7 @@ def woot; end
     @reporter.record(errored_test)
     @reporter.report
 
-    expect = %r{\AE\n\nError:\nTestUnitReporterTest::ExampleTest#woot:\nArgumentError: wups\n    No backtrace\n\nbin/rails test .*test/test_unit/reporter_test\.rb:\d+\n\n\z}
+    expect = %r{\AE\n\nError:\nTestUnitReporterTest::ExampleTest#woot:\nArgumentError: wups\n    No backtrace\n\nbin/rails test .*test/test_unit/reporter_test.rb:\d+\n\n\z}
     assert_match expect, @output.string
   end
 
@@ -150,7 +150,7 @@ def woot; end
       colored = Rails::TestUnitReporter.new @output, color: true, output_inline: true
       colored.record(errored_test)
 
-      expected = %r{\e\[31mE\e\[0m\n\n\e\[31mError:\nTestUnitReporterTest::ExampleTest#woot:\nArgumentError: wups\n    \n\e\[0m}
+      expected = %r{\e\[31mE\e\[0m\n\n\e\[31mError:\nTestUnitReporterTest::ExampleTest#woot:\nArgumentError: wups\n    No backtrace\n\e\[0m}
       assert_match expected, @output.string
     end
   end
@@ -171,11 +171,8 @@ def failed_test
   end
 
   def errored_test
-    error = ArgumentError.new("wups")
-    error.set_backtrace([ "some_test.rb:4" ])
-
     et = ExampleTest.new(:woot)
-    et.failures << Minitest::UnexpectedError.new(error)
+    et.failures << Minitest::UnexpectedError.new(ArgumentError.new("wups"))
     et
   end
 

tools/test.rb

@@ -3,10 +3,8 @@
 require 'bundler'
 Bundler.setup
 
-require "rails/test_unit/runner"
-require "rails/test_unit/reporter"
+require "rails/test_unit/minitest_plugin"
 require "rails/test_unit/line_filtering"
-require "active_support"
 require "active_support/test_case"
 
 module Rails
@@ -18,6 +16,5 @@ def self.root
 
 ActiveSupport::TestCase.extend Rails::LineFiltering
 Rails::TestUnitReporter.executable = "bin/test"
-
-Rails::TestUnit::Runner.parse_options(ARGV)
-Rails::TestUnit::Runner.run(ARGV)
+Minitest.run_via = :rails
+require "active_support/testing/autorun"