From 4ab1f76499bef114ada1585f781c6a6e7ffd9753 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Thu, 23 Feb 2023 07:47:11 +1000 Subject: [PATCH] DEV: Fix bin/turbo_rspec runtime recording (#20407) This commit 57caf08e1331f7f22c2a0f532480347a8b13fff8 broke `bin/turbo_rspec` timing recording via `TurboTests::Runner`, because we changed to using all `spec/*` folders except `spec/system` as default for the runner, rather than the old `['spec']` array, which is what `TurboTests::Runner` was relying on to determine whether to record test run time with `ParallelTests::RSpec::RuntimeLogger`. Instead, we can just pass a new `use_runtime_info` boolean to the runner class and use it when running against the default set of spec files using `bin/turbo_rspec` and the turbo rspec rake task. --- bin/turbo_rspec | 83 +++++++++++++++++---------------------- lib/tasks/turbo.rake | 6 ++- lib/turbo_tests/runner.rb | 23 +++++++++-- 3 files changed, 59 insertions(+), 53 deletions(-) diff --git a/bin/turbo_rspec b/bin/turbo_rspec index 7d4c4278163..bd9b4471266 100755 --- a/bin/turbo_rspec +++ b/bin/turbo_rspec @@ -1,70 +1,56 @@ #!/usr/bin/env ruby # frozen_string_literal: true -ENV['RAILS_ENV'] ||= 'test' +ENV["RAILS_ENV"] ||= "test" -require './lib/turbo_tests' -require 'optparse' +require "./lib/turbo_tests" +require "optparse" requires = [] formatters = [] verbose = false fail_fast = nil -OptionParser.new do |opts| - opts.on("-r", "--require PATH", "Require a file.") do |filename| - requires << filename - end +OptionParser + .new do |opts| + opts.on("-r", "--require PATH", "Require a file.") { |filename| requires << filename } - opts.on("-f", "--format FORMATTER", "Choose a formatter.") do |name| - formatters << { - name: name, - outputs: [] - } - end - - opts.on("-o", "--out FILE", "Write output to a file instead of $stdout") do |filename| - if formatters.empty? - formatters << { - name: "progress", - outputs: [] - } + opts.on("-f", "--format FORMATTER", "Choose a formatter.") do |name| + formatters << { name: name, outputs: [] } end - formatters.last[:outputs] << filename - end - opts.on("-v", "--verbose", "More output") do - verbose = true - end + opts.on("-o", "--out FILE", "Write output to a file instead of $stdout") do |filename| + formatters << { name: "progress", outputs: [] } if formatters.empty? + formatters.last[:outputs] << filename + end - opts.on("--fail-fast=[N]") do |n| - n = Integer(n) rescue nil - fail_fast = (n.nil? || n < 1) ? 1 : n - end + opts.on("-v", "--verbose", "More output") { verbose = true } -end.parse!(ARGV) + opts.on("--fail-fast=[N]") do |n| + n = + begin + Integer(n) + rescue StandardError + nil + end + fail_fast = (n.nil? || n < 1) ? 1 : n + end + end + .parse!(ARGV) requires.each { |f| require(f) } -if formatters.empty? - formatters << { - name: "progress", - outputs: [] - } -end +formatters << { name: "progress", outputs: [] } if formatters.empty? -formatters.each do |formatter| - if formatter[:outputs].empty? - formatter[:outputs] << '-' - end -end +formatters.each { |formatter| formatter[:outputs] << "-" if formatter[:outputs].empty? } -# We do not want to include system specs by default, they are super -# slow and require a selenium server to be running. -default_spec_dirs = Dir.entries("#{Rails.root}/spec").reject do |entry| - !File.directory?("spec/#{entry}") || ["..", ".", "system"].include?(entry) -end.map { |entry| "spec/#{entry}" } -files = ARGV.empty? ? default_spec_dirs : ARGV +if ARGV.empty? + files = TurboTests::Runner.default_spec_folders + use_runtime_info = true +else + files = ARGV + use_runtime_info = false +end puts "Running turbo_rspec using files in #{files}" success = @@ -72,7 +58,8 @@ success = formatters: formatters, files: files, verbose: verbose, - fail_fast: fail_fast + fail_fast: fail_fast, + use_runtime_info: use_runtime_info, ) if success diff --git a/lib/tasks/turbo.rake b/lib/tasks/turbo.rake index 06d6e14b704..8440252d864 100644 --- a/lib/tasks/turbo.rake +++ b/lib/tasks/turbo.rake @@ -3,5 +3,9 @@ task "turbo:spec" => :test do |t| require "./lib/turbo_tests" - TurboTests::Runner.run(formatters: [{ name: "progress", outputs: ["-"] }], files: ["spec"]) + TurboTests::Runner.run( + formatters: [{ name: "progress", outputs: ["-"] }], + files: TurboTests::Runner.default_spec_folders, + use_runtime_info: true, + ) end diff --git a/lib/turbo_tests/runner.rb b/lib/turbo_tests/runner.rb index b81be543ed9..010a4bbb771 100644 --- a/lib/turbo_tests/runner.rb +++ b/lib/turbo_tests/runner.rb @@ -8,12 +8,27 @@ module TurboTests start_time = opts.fetch(:start_time) { Time.now } verbose = opts.fetch(:verbose, false) fail_fast = opts.fetch(:fail_fast, nil) + use_runtime_info = opts.fetch(:use_runtime_info, false) STDERR.puts "VERBOSE" if verbose reporter = Reporter.from_config(formatters, start_time) - new(reporter: reporter, files: files, verbose: verbose, fail_fast: fail_fast).run + new( + reporter: reporter, + files: files, + verbose: verbose, + fail_fast: fail_fast, + use_runtime_info: use_runtime_info, + ).run + end + + def self.default_spec_folders + # We do not want to include system specs by default, they are quite slow. + Dir + .entries("#{Rails.root}/spec") + .reject { |entry| !File.directory?("spec/#{entry}") || %w[.. . system].include?(entry) } + .map { |entry| "spec/#{entry}" } end def initialize(opts) @@ -21,6 +36,7 @@ module TurboTests @files = opts[:files] @verbose = opts[:verbose] @fail_fast = opts[:fail_fast] + @use_runtime_info = opts[:use_runtime_info] @failure_count = 0 @messages = Queue.new @@ -32,11 +48,10 @@ module TurboTests check_for_migrations @num_processes = ParallelTests.determine_number_of_processes(nil) - use_runtime_info = @files == ["spec"] group_opts = {} - if use_runtime_info + if @use_runtime_info group_opts[:runtime_log] = "tmp/turbo_rspec_runtime.log" else group_opts[:group_by] = :filesize @@ -47,7 +62,7 @@ module TurboTests setup_tmp_dir - subprocess_opts = { record_runtime: use_runtime_info } + subprocess_opts = { record_runtime: @use_runtime_info } start_multisite_subprocess(@files, **subprocess_opts)