From 2327ebfdc6791ce95e6628884ec7b18b73e22bba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20D=C3=ADaz?= Date: Wed, 24 Feb 2021 17:42:34 -0500 Subject: [PATCH] Disable parallel testing when running individual files Setting up the parallel workers could be an overhead when running individual files. This patch disables that process in case the number of files to run is less than one. Results running a sample file: Before: ``` actionpack $ bin/test test/controller/parameters/accessors_test.rb Run options: --seed 48261 ........................................................................ Finished in 0.211923s, 339.7460 runs/s, 552.0873 assertions/s. 72 runs, 117 assertions, 0 failures, 0 errors, 0 skips ``` After ``` actionpack $ bin/test test/controller/parameters/accessors_test.rb Run options: --seed 5461 ........................................................................ Finished in 0.008411s, 8560.2189 runs/s, 13910.3557 assertions/s. 72 runs, 117 assertions, 0 failures, 0 errors, 0 skips ``` --- activesupport/CHANGELOG.md | 6 ++ activesupport/lib/active_support.rb | 5 + activesupport/lib/active_support/test_case.rb | 8 +- railties/lib/rails/test_unit/runner.rb | 4 +- railties/test/application/test_runner_test.rb | 95 ++++++++++++++++++- 5 files changed, 112 insertions(+), 6 deletions(-) diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 8d37d69dc9..cfe4ab9dba 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,9 @@ +* Tests parallelization is now disabled when running individual files to prevent the setup overhead. + + It can still be enforced if the environment variable `PARALLEL_WORKERS` is present and set to a value greater than 1. + + *Ricardo Díaz* + * Fix proxying keyword arguments in `ActiveSupport::CurrentAttributes`. *Marcin Kołodziej* diff --git a/activesupport/lib/active_support.rb b/activesupport/lib/active_support.rb index f8a65cebb2..ff5ebfd6a9 100644 --- a/activesupport/lib/active_support.rb +++ b/activesupport/lib/active_support.rb @@ -87,6 +87,11 @@ module ActiveSupport end cattr_accessor :test_order # :nodoc: + cattr_accessor :test_parallelization_disabled, default: false # :nodoc: + + def self.disable_test_parallelization! + self.test_parallelization_disabled = true unless ENV["PARALLEL_WORKERS"] + end def self.to_time_preserves_timezone DateAndTime::Compatibility.preserve_timezone diff --git a/activesupport/lib/active_support/test_case.rb b/activesupport/lib/active_support/test_case.rb index 7be4108ed7..0cbd2ddab3 100644 --- a/activesupport/lib/active_support/test_case.rb +++ b/activesupport/lib/active_support/test_case.rb @@ -75,7 +75,7 @@ module ActiveSupport workers = Concurrent.physical_processor_count if workers == :number_of_processors workers = ENV["PARALLEL_WORKERS"].to_i if ENV["PARALLEL_WORKERS"] - return if workers <= 1 + return if workers <= 1 || ActiveSupport.test_parallelization_disabled executor = case with when :processes @@ -91,6 +91,12 @@ module ActiveSupport Minitest.parallel_executor = executor parallelize_me! + + # `Minitest::Test.parallelize_me!` will try to change `test_order` to return + # `:parallel`. However, since `ActiveSupport::TestCase` is already overriding it, + # in some inheritance situations, it will have precedence over the `Minitest` one. + # For this reason, it needs to be updated by hand. + self.test_order = :parallel end # Set up hook for parallel testing. This can be used if you have multiple diff --git a/railties/lib/rails/test_unit/runner.rb b/railties/lib/rails/test_unit/runner.rb index 18dbf773a7..fe6c17c03c 100644 --- a/railties/lib/rails/test_unit/runner.rb +++ b/railties/lib/rails/test_unit/runner.rb @@ -3,6 +3,7 @@ require "shellwords" require "method_source" require "rake/file_list" +require "active_support" require "active_support/core_ext/module/attribute_accessors" module Rails @@ -46,7 +47,8 @@ module Rails tests = Rake::FileList[patterns.any? ? patterns : default_test_glob] tests.exclude(default_test_exclude_glob) if patterns.empty? - + # Disable parallel testing if there's only one test file to run. + ActiveSupport.disable_test_parallelization! if tests.size <= 1 tests.to_a.each { |path| require File.expand_path(path) } end diff --git a/railties/test/application/test_runner_test.rb b/railties/test/application/test_runner_test.rb index 2f27aadbcf..de29071fab 100644 --- a/railties/test/application/test_runner_test.rb +++ b/railties/test/application/test_runner_test.rb @@ -539,6 +539,76 @@ module ApplicationTests assert_no_match "create_table(:users)", output end + def test_parallel_is_disabled_when_single_file_is_run + exercise_parallelization_regardless_of_machine_core_count(with: :processes, force: false) + + file_name = app_file "test/unit/parallel_test.rb", <<-RUBY + require "test_helper" + + class ParallelTest < ActiveSupport::TestCase + def test_verify_test_order + puts "Test order: \#{self.class.test_order}" + end + end + RUBY + + output = run_test_command(file_name) + + assert_match "Test order: random", output + end + + def test_parallel_is_enabled_when_multiple_files_are_run + exercise_parallelization_regardless_of_machine_core_count(with: :processes, force: false) + + file_1 = app_file "test/unit/parallel_test_first.rb", <<-RUBY + require "test_helper" + + class ParallelTestFirst < ActiveSupport::TestCase + def test_verify_test_order + puts "Test order (file 1): \#{self.class.test_order}" + end + end + RUBY + + file_2 = app_file "test/unit/parallel_test_second.rb", <<-RUBY + require "test_helper" + + class ParallelTestSecond < ActiveSupport::TestCase + def test_verify_test_order + puts "Test order (file 2): \#{self.class.test_order}" + end + end + RUBY + + output = run_test_command([file_1, file_2].join(" ")) + + assert_match "Test order (file 1): parallel", output + assert_match "Test order (file 2): parallel", output + end + + def test_parallel_is_enabled_when_PARALLEL_WORKERS_is_set + @old = ENV["PARALLEL_WORKERS"] + ENV["PARALLEL_WORKERS"] = "5" + + exercise_parallelization_regardless_of_machine_core_count(with: :processes, force: false) + + file_name = app_file "test/unit/parallel_test.rb", <<-RUBY + require "test_helper" + + class ParallelTest < ActiveSupport::TestCase + def test_verify_test_order + puts "Test order: \#{self.class.test_order}" + end + end + RUBY + + output = run_test_command(file_name) + + assert_match "Test order: parallel", output + ensure + ENV["PARALLEL_WORKERS"] = @old + end + def test_run_in_parallel_with_process_worker_crash exercise_parallelization_regardless_of_machine_core_count(with: :processes) @@ -1039,11 +1109,28 @@ module ApplicationTests RUBY end - def exercise_parallelization_regardless_of_machine_core_count(with:) + def exercise_parallelization_regardless_of_machine_core_count(with:, force: true) + file_content = ERB.new(<<-ERB, trim_mode: "-").result_with_hash(with: with.to_s, force: force) + ENV["RAILS_ENV"] ||= "test" + require_relative "../config/environment" + require "rails/test_help" + + class ActiveSupport::TestCase + <%- if force -%> + # Force parallelization, even with single files + ActiveSupport.test_parallelization_disabled = false + <%- end -%> + + # Run tests in parallel with specified workers + parallelize(workers: 2, with: :<%= with %>) + + # Setup all fixtures in test/fixtures/*.yml for all tests in alphabetical order. + fixtures :all + end + ERB + app_path("test/test_helper.rb") do |file_name| - file = File.read(file_name) - file.sub!(/parallelize\(([^)]*)\)/, "parallelize(workers: 2, with: :#{with})") - File.write(file_name, file) + File.write(file_name, file_content) end end