From 501627b8d31be701c7e69e62579e0567daa7092f Mon Sep 17 00:00:00 2001 From: Andrew Novoselac Date: Thu, 1 Feb 2024 13:41:15 -0500 Subject: [PATCH] Allow Actionable Errors encountered when running tests to be retried. This can be configured by config.actionable_command_line_errors and is true in the test environment unless the "CI" env variable is set, and false otherwise. Co-authored-by: Gannon McGibbon --- guides/source/configuring.md | 6 +++ railties/CHANGELOG.md | 29 +++++++++++ .../lib/rails/application/configuration.rb | 2 +- railties/lib/rails/command/base.rb | 16 ++++++ .../lib/rails/commands/test/test_command.rb | 4 +- .../templates/config/environments/test.rb.tt | 3 ++ .../new_framework_defaults_7_2.rb.tt | 6 +++ .../lib/rails/testing/maintain_test_schema.rb | 8 +-- railties/test/application/test_test.rb | 49 +++++++++++++++++++ 9 files changed, 115 insertions(+), 8 deletions(-) diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 02af0a17f48bb..15bb99aad70b7 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -161,6 +161,12 @@ Below are the default values associated with each target version. In cases of co The following configuration methods are to be called on a `Rails::Railtie` object, such as a subclass of `Rails::Engine` or `Rails::Application`. +#### `config.actionable_command_line_errors` + +Says whether the user will be prompted to retry `ActiveSupport::ActionableError`s on the command line. + +By default it is `true` in the test environment unless the `"CI"` env variable is set, and false otherwise. + #### `config.add_autoload_paths_to_load_path` Says whether autoload paths have to be added to `$LOAD_PATH`. It is recommended to be set to `false` in `:zeitwerk` mode early, in `config/application.rb`. Zeitwerk uses absolute paths internally, and applications running in `:zeitwerk` mode do not need `require_dependency`, so models, controllers, jobs, etc. do not need to be in `$LOAD_PATH`. Setting this to `false` saves Ruby from checking these directories when resolving `require` calls with relative paths, and saves Bootsnap work and RAM, since it does not need to build an index for them. diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 387f19672e77d..92fbcb8d912c0 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,32 @@ +* Allow Actionable Errors encountered when running tests to be retried. This can be configured by + `config.actionable_command_line_errors` and is `true` in the test environment unless the `"CI"` env variable + is set, and false otherwise. + + ```txt + Migrations are pending. To resolve this issue, run: + + bin/rails db:migrate + + You have 1 pending migration: + + db/migrate/20240201213806_add_a_to_b.rb + Run pending migrations? [Yn] Y + == 20240201213806 AddAToB: migrating ========================================= + == 20240201213806 AddAToB: migrated (0.0000s) ================================ + + Running 7 tests in a single process (parallelization threshold is 50) + Run options: --seed 22200 + + # Running: + + ....... + + Finished in 0.243394s, 28.7600 runs/s, 45.1942 assertions/s. + 7 runs, 11 assertions, 0 failures, 0 errors, 0 skips + ``` + + *Andrew Novoselac & Gannon McGibbon* + * Fix sanitizer vendor configuration in 7.1 defaults. In apps where rails-html-sanitizer was not eagerly loaded, the sanitizer default could end up diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index 0990666424530..62a83a4929d7e 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -23,7 +23,7 @@ class Configuration < ::Rails::Engine::Configuration :content_security_policy_nonce_generator, :content_security_policy_nonce_directives, :require_master_key, :credentials, :disable_sandbox, :sandbox_by_default, :add_autoload_paths_to_load_path, :rake_eager_load, :server_timing, :log_file_size, - :dom_testing_default_html_version + :dom_testing_default_html_version, :actionable_command_line_errors attr_reader :encoding, :api_only, :loaded_config_version, :log_level diff --git a/railties/lib/rails/command/base.rb b/railties/lib/rails/command/base.rb index ee6e04998733e..d8dc424532916 100644 --- a/railties/lib/rails/command/base.rb +++ b/railties/lib/rails/command/base.rb @@ -179,6 +179,22 @@ def invoke_command(command, *) # :nodoc: ensure @current_subcommand = original_subcommand end + + protected + def with_actionable_errors_retried(&block) + block.call + rescue ActiveSupport::ActionableError => e + puts e.to_s.strip + exit 1 unless Rails.application.config.actionable_command_line_errors + + ActiveSupport::ActionableError.actions(e).each_key do |action_name| + if yes? "#{action_name}? [Yn]" + ActiveSupport::ActionableError.dispatch(e, action_name) + return with_actionable_errors_retried(&block) + end + end + exit 1 + end end end end diff --git a/railties/lib/rails/commands/test/test_command.rb b/railties/lib/rails/commands/test/test_command.rb index e183ddf0f3406..0e592963adc0b 100644 --- a/railties/lib/rails/commands/test/test_command.rb +++ b/railties/lib/rails/commands/test/test_command.rb @@ -30,7 +30,9 @@ def perform(*args) Rails::TestUnit::Runner.parse_options(args) run_prepare_task if self.args.none?(EXACT_TEST_ARGUMENT_PATTERN) - Rails::TestUnit::Runner.run(args) + with_actionable_errors_retried do + Rails::TestUnit::Runner.run(args) + end rescue Rails::TestUnit::InvalidTestError => error say error.message end diff --git a/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt index 0ab0765dce5a9..62f37541e5da3 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt @@ -66,4 +66,7 @@ Rails.application.configure do # Raise error when a before_action's only/except options reference missing actions config.action_controller.raise_on_missing_callback_actions = true + + # Prompt user to retry actionable errors on the command line + config.actionable_command_line_errors = !ENV["CI"] end diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_2.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_2.rb.tt index c1a496c3912a6..cf15ead058332 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_2.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_2.rb.tt @@ -27,3 +27,9 @@ # expected format can disable validation by setting this config to `false`. #++ # Rails.application.config.active_record.validate_migration_timestamps = true + +### +# Enable actionable command lines errors. When enabled, actionable errors encountered on the command line +# will prompt the user to take an action to resolve and retry the error. +# +# Rails.application.config.actionable_command_line_errors = Rails.env.test? && !ENV["CI"] diff --git a/railties/lib/rails/testing/maintain_test_schema.rb b/railties/lib/rails/testing/maintain_test_schema.rb index 3197b77d10928..34991c64032f4 100644 --- a/railties/lib/rails/testing/maintain_test_schema.rb +++ b/railties/lib/rails/testing/maintain_test_schema.rb @@ -1,12 +1,8 @@ # frozen_string_literal: true if defined?(ActiveRecord::Base) - begin - ActiveRecord::Migration.maintain_test_schema! - rescue ActiveRecord::PendingMigrationError => e - puts e.to_s.strip - exit 1 - end + + ActiveRecord::Migration.maintain_test_schema! if Rails.configuration.eager_load ActiveRecord::Base.descendants.each do |model| diff --git a/railties/test/application/test_test.rb b/railties/test/application/test_test.rb index 30f92fbe98106..2675411358c10 100644 --- a/railties/test/application/test_test.rb +++ b/railties/test/application/test_test.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "isolation/abstract_unit" +require "thor" module ApplicationTests class TestTest < ActiveSupport::TestCase @@ -110,6 +111,11 @@ def test_failure output = rails("generate", "model", "user", "name:string") version = output.match(/(\d+)_create_users\.rb/)[1] + app_file "config/environments/test.rb", <<-RUBY + Rails.application.configure do + config.actionable_command_line_errors = false + end + RUBY app_file "test/models/user_test.rb", <<-RUBY require "test_helper" @@ -147,6 +153,11 @@ class UserTest < ActiveSupport::TestCase output = rails("generate", "model", "user", "name:string") version = output.match(/(\d+)_create_users\.rb/)[1] + app_file "config/environments/test.rb", <<-RUBY + Rails.application.configure do + config.actionable_command_line_errors = false + end + RUBY app_file "test/models/user_test.rb", <<-RUBY require "test_helper" @@ -345,6 +356,44 @@ def self.load_schema! assert_unsuccessful_run "models/user_test.rb", "SCHEMA LOADED!" end + def test_actionable_command_line_error + rails "generate", "scaffold", "user", "name:string" + app_file "config/environments/test.rb", <<-RUBY + Rails.application.configure do + config.actionable_command_line_errors = true + end + RUBY + app_file "config/initializers/thor_yes.rb", <<-RUBY + Rails::Command::Base.class_eval <<-INITIALIZER + def yes?(statement, color = nil) + raise ArgumentError unless statement == "Run pending migrations? [Yn]" + true + end + INITIALIZER + RUBY + + run_test_file("models/user_test.rb").tap do |output| + assert_match "Migrations are pending. To resolve this issue, run:", output + assert_match "CreateUsers: migrating", output + assert_match "0 runs, 0 assertions, 0 failures, 0 errors, 0 skips", output + end + end + + def test_actionable_command_line_errors_false + rails "generate", "scaffold", "user", "name:string" + app_file "config/environments/test.rb", <<-RUBY + Rails.application.configure do + config.actionable_command_line_errors = false + end + RUBY + + run_test_file("models/user_test.rb").tap do |output| + assert_match "Migrations are pending. To resolve this issue, run:", output + assert_no_match "CreateUsers: migrating", output + assert_no_match "0 runs, 0 assertions, 0 failures, 0 errors, 0 skips", output + end + end + private def assert_unsuccessful_run(name, message) result = run_test_file(name)