From 6a6c7e64f5bbea9b66c8b93eb85278805c2700a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Tue, 30 Apr 2024 21:43:54 +0000 Subject: [PATCH] Remove configuration to control what we do with tests without assertions This is too much complexity for something low value. Let's just always warn when a test doesn't have any assertions. If people want to raise an error or ignore them, they can do so by overriding `Warning.warn`. --- activesupport/CHANGELOG.md | 8 +-- activesupport/lib/active_support/railtie.rb | 4 -- activesupport/lib/active_support/test_case.rb | 4 +- .../testing/assertionless_tests.rb | 57 ---------------- .../testing/tests_without_assertions.rb | 22 +++++++ .../test/testing/assertionless_tests_test.rb | 66 ------------------- .../testing/test_without_assertions_test.rb | 26 ++++++++ guides/source/configuring.md | 13 ---- .../lib/rails/application/configuration.rb | 6 -- .../new_framework_defaults_7_2.rb.tt | 8 --- 10 files changed, 52 insertions(+), 162 deletions(-) delete mode 100644 activesupport/lib/active_support/testing/assertionless_tests.rb create mode 100644 activesupport/lib/active_support/testing/tests_without_assertions.rb delete mode 100644 activesupport/test/testing/assertionless_tests_test.rb create mode 100644 activesupport/test/testing/test_without_assertions_test.rb diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index ae95b24d8b..1c4195c9ba 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,12 +1,8 @@ -* Allow assertionless tests to be reported. +* Warn on tests without assertions. - `ActiveSupport::TestCase` can be configured to report tests that do not run any assertions. + `ActiveSupport::TestCase` now warns when tests do not run any assertions. This is helpful in detecting broken tests that do not perform intended assertions. - ```ruby - config.active_support.assertionless_tests_behavior = :raise - ``` - *fatkodima* * Support `hexBinary` type in `ActiveSupport::XmlMini`. diff --git a/activesupport/lib/active_support/railtie.rb b/activesupport/lib/active_support/railtie.rb index 118a932f81..133fbc4f52 100644 --- a/activesupport/lib/active_support/railtie.rb +++ b/activesupport/lib/active_support/railtie.rb @@ -124,10 +124,6 @@ class Railtie < Rails::Railtie # :nodoc: ActiveSupport.deprecator.warn("config.active_support.remove_deprecated_time_with_zone_name is deprecated and will be removed in Rails 7.2.") elsif k == :use_rfc4122_namespaced_uuids ActiveSupport.deprecator.warn("config.active_support.use_rfc4122_namespaced_uuids is deprecated and will be removed in Rails 7.2.") - elsif k == :assertionless_tests_behavior - ActiveSupport.on_load(:active_support_test_case) do - ActiveSupport::TestCase.assertionless_tests_behavior = v - end else k = "#{k}=" ActiveSupport.public_send(k, v) if ActiveSupport.respond_to? k diff --git a/activesupport/lib/active_support/test_case.rb b/activesupport/lib/active_support/test_case.rb index 17b8352ed4..bd2847fc4f 100644 --- a/activesupport/lib/active_support/test_case.rb +++ b/activesupport/lib/active_support/test_case.rb @@ -3,7 +3,7 @@ require "minitest" require "active_support/testing/tagged_logging" require "active_support/testing/setup_and_teardown" -require "active_support/testing/assertionless_tests" +require "active_support/testing/tests_without_assertions" require "active_support/testing/assertions" require "active_support/testing/error_reporter_assertions" require "active_support/testing/deprecation" @@ -143,7 +143,7 @@ def parallelize_teardown(&block) include ActiveSupport::Testing::TaggedLogging prepend ActiveSupport::Testing::SetupAndTeardown - prepend ActiveSupport::Testing::AssertionlessTests + prepend ActiveSupport::Testing::TestsWithoutAssertions include ActiveSupport::Testing::Assertions include ActiveSupport::Testing::ErrorReporterAssertions include ActiveSupport::Testing::Deprecation diff --git a/activesupport/lib/active_support/testing/assertionless_tests.rb b/activesupport/lib/active_support/testing/assertionless_tests.rb deleted file mode 100644 index d9524b4ace..0000000000 --- a/activesupport/lib/active_support/testing/assertionless_tests.rb +++ /dev/null @@ -1,57 +0,0 @@ -# frozen_string_literal: true - -module ActiveSupport - module Testing - # Allows to configure the behavior when the test case has no assertions in it. - # This is helpful in detecting broken tests that do not perform intended assertions. - module AssertionlessTests # :nodoc: - def self.prepended(klass) - klass.class_attribute :_assertionless_tests_behavior, instance_accessor: false, instance_predicate: false - klass.extend ClassMethods - klass.assertionless_tests_behavior = :ignore - end - - module ClassMethods - def assertionless_tests_behavior - _assertionless_tests_behavior - end - - def assertionless_tests_behavior=(behavior) - self._assertionless_tests_behavior = - case behavior - when :ignore, nil - nil - when :log - logger = - if defined?(Rails.logger) && Rails.logger - Rails.logger - else - require "active_support/logger" - ActiveSupport::Logger.new($stderr) - end - - ->(message) { logger.warn(message) } - when :raise - ->(message) { raise Minitest::Assertion, message } - when Proc - behavior - else - raise ArgumentError, "assertionless_tests_behavior must be one of :ignore, :log, :raise, or a custom proc." - end - end - end - - def after_teardown - super - - return if skipped? || error? - - if assertions == 0 && (behavior = self.class.assertionless_tests_behavior) - file, line = method(name).source_location - message = "Test is missing assertions: `#{name}` #{file}:#{line}" - behavior.call(message) - end - end - end - end -end diff --git a/activesupport/lib/active_support/testing/tests_without_assertions.rb b/activesupport/lib/active_support/testing/tests_without_assertions.rb new file mode 100644 index 0000000000..86bd1c57da --- /dev/null +++ b/activesupport/lib/active_support/testing/tests_without_assertions.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module ActiveSupport + module Testing + # Warns when a test case does not perform any assertions. + # + # This is helpful in detecting broken tests that do not perform intended assertions. + module TestsWithoutAssertions # :nodoc: + def after_teardown + super + + return if skipped? || error? + + if assertions == 0 + file, line = method(name).source_location + message = "Test is missing assertions: `#{name}` #{file}:#{line}" + warn message + end + end + end + end +end diff --git a/activesupport/test/testing/assertionless_tests_test.rb b/activesupport/test/testing/assertionless_tests_test.rb deleted file mode 100644 index 3f6e15517c..0000000000 --- a/activesupport/test/testing/assertionless_tests_test.rb +++ /dev/null @@ -1,66 +0,0 @@ -# frozen_string_literal: true - -require_relative "../abstract_unit" -require "active_support/core_ext/object/with" - -module AssertionlessTestsTest - module Tests - def test_without_assertions - end - end - - class AssertionlessTestsIgnore < ActiveSupport::TestCase - module AfterTeardown - def after_teardown - ActiveSupport::TestCase.with(assertionless_tests_behavior: :ignore) do - assert_nothing_raised do - super - end - end - end - end - - include Tests - prepend AfterTeardown - end - - class AssertionlessTestsLog < ActiveSupport::TestCase - module AfterTeardown - def after_teardown - _out, err = capture_io do - ActiveSupport::TestCase.with(assertionless_tests_behavior: :log) do - super - end - end - assert_match(/Test is missing assertions: `test_without_assertions` .+assertionless_tests_test\.rb:\d+/, err) - end - end - - include Tests - prepend AfterTeardown - end - - class AssertionlessTestsRaise < ActiveSupport::TestCase - module AfterTeardown - def after_teardown - ActiveSupport::TestCase.with(assertionless_tests_behavior: :raise) do - assert_raise(Minitest::Assertion, - match: /Test is missing assertions: `test_without_assertions` .+assertionless_tests_test\.rb:\d+/) do - super - end - end - end - end - - include Tests - prepend AfterTeardown - end - - class AssertionlessTestsUnknown < ActiveSupport::TestCase - def test_raises_when_unknown_behavior - assert_raises(ArgumentError, match: /assertionless_tests_behavior must be one of/) do - ActiveSupport::TestCase.assertionless_tests_behavior = :unknown - end - end - end -end diff --git a/activesupport/test/testing/test_without_assertions_test.rb b/activesupport/test/testing/test_without_assertions_test.rb new file mode 100644 index 0000000000..ab0cd51b51 --- /dev/null +++ b/activesupport/test/testing/test_without_assertions_test.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require_relative "../abstract_unit" +require "active_support/core_ext/object/with" + +module TestsWithoutAssertionsTest + module Tests + def test_without_assertions + end + end + + class TestsWithoutAssertionsWarnTest < ActiveSupport::TestCase + module AfterTeardown + def after_teardown + _out, err = capture_io do + super + end + + assert_match(/Test is missing assertions: `test_without_assertions` .+test_without_assertions_test\.rb:\d+/, err) + end + end + + include Tests + prepend AfterTeardown + end +end diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 774325f689..924008d0cc 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -64,7 +64,6 @@ Below are the default values associated with each target version. In cases of co - [`config.active_record.automatically_invert_plural_associations`](#config-active-record-automatically-invert-plural-associations): `true` - [`config.active_record.validate_migration_timestamps`](#config-active-record-validate-migration-timestamps): `true` - [`config.active_storage.web_image_content_types`](#config-active-storage-web-image-content-types): `%w[image/png image/jpeg image/gif image/webp]` -- [`config.active_support.assertionless_tests_behavior`](#config-active-support-assertionless-tests-behavior): `:log` #### Default Values for Target Version 7.1 @@ -2657,18 +2656,6 @@ The default value depends on the `config.load_defaults` target version: | (original) | `false` | | 7.0 | `true` | -#### `config.active_support.assertionless_tests_behavior` - -Controls the behavior when a test do not run any assertions. The following options are available: - - * `:ignore` - Assertionless tests will be ignored. This is the default. - - * `:log` - Assertionless tests will be logged via `Rails.logger` at the `:warn` level. - - * `:raise` - Assertionless tests will be considered as failed and raise an error. - - * Custom proc - A custom proc can be provided. It should accept an error message. - #### `ActiveSupport::Logger.silencer` Is set to `false` to disable the ability to silence logging in a block. The default is `true`. diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index ced9ce7398..498826d2fe 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -334,12 +334,6 @@ def load_defaults(target_version) active_record.validate_migration_timestamps = true active_record.automatically_invert_plural_associations = true end - - if respond_to?(:active_support) - if Rails.env.local? - active_support.assertionless_tests_behavior = :log - end - end else raise "Unknown version #{target_version.to_s.inspect}" 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 8526664ebd..48552491e9 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 @@ -66,11 +66,3 @@ # on `Post`. With this option enabled, it will also look for a `:comments` association. #++ # Rails.application.config.active_record.automatically_invert_plural_associations = true - -### -# Controls whether Active Support should log tests without assertions. -# This is helpful in detecting broken tests that do not perform intended assertions. -# To disable this behavior, set the value to `:ignore` inside the `config/application.rb` (NOT this file). -# -#++ -# Rails.application.config.active_support.assertionless_tests_behavior = :log