diff options
| -rw-r--r-- | actionpack/CHANGELOG.md | 9 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb | 12 | ||||
| -rw-r--r-- | actionview/CHANGELOG.md | 11 | ||||
| -rw-r--r-- | activerecord/lib/active_record/relation.rb | 4 | ||||
| -rw-r--r-- | activerecord/lib/active_record/relation/query_methods.rb | 23 | ||||
| -rw-r--r-- | railties/test/application/test_runner_test.rb | 26 |
6 files changed, 63 insertions, 22 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 4109ae7006..4502c6a2f9 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,12 @@ +* Make system tests take a failed screenshot in a `before_teardown` hook + rather than an `after_teardown` hook. + + This helps minimize the time gap between when an assertion fails and when + the screenshot is taken (reducing the time in which the page could have + been dynamically updated after the assertion failed). + + *Richard Macklin* + * Introduce `ActionDispatch::ActionableExceptions`. The `ActionDispatch::ActionableExceptions` middleware dispatches actions diff --git a/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb b/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb index 600e9c733b..7080dbe022 100644 --- a/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb +++ b/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb @@ -16,12 +16,14 @@ module ActionDispatch super end + def before_teardown + take_failed_screenshot + ensure + super + end + def after_teardown - begin - take_failed_screenshot - ensure - Capybara.reset_sessions! - end + Capybara.reset_sessions! ensure super end diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index be67aff543..f465e08859 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,14 @@ +* Fix partial caching skips same item issue + + If we render cached collection partials with repeated items, those repeated items + will get skipped. For example, if you have 5 identical items in your collection, Rails + only renders the first one when `cached` is set to true. But it should render all + 5 items instead. + + This fixes #35114 + + *Stan Lo* + * Only clear ActionView cache in development on file changes To speed up development mode, view caches are only cleared when files in diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index ab24f67a6d..add95f6a0a 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -553,8 +553,8 @@ module ActiveRecord # # => ActiveRecord::ActiveRecordError: delete_all doesn't support distinct def delete_all invalid_methods = INVALID_METHODS_FOR_DELETE_ALL.select do |method| - value = get_value(method) - SINGLE_VALUE_METHODS.include?(method) ? value : value.any? + value = @values[method] + method == :distinct ? value : value&.any? end if invalid_methods.any? raise ActiveRecordError.new("delete_all doesn't support #{invalid_methods.join(', ')}") diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 90b5e9a118..5d3cea6741 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -67,11 +67,13 @@ module ActiveRecord end class_eval <<-CODE, __FILE__, __LINE__ + 1 def #{method_name} # def includes_values - get_value(#{name.inspect}) # get_value(:includes) + default = DEFAULT_VALUES[:#{name}] # default = DEFAULT_VALUES[:includes] + @values.fetch(:#{name}, default) # @values.fetch(:includes, default) end # end def #{method_name}=(value) # def includes_values=(value) - set_value(#{name.inspect}, value) # set_value(:includes, value) + assert_mutability! # assert_mutability! + @values[:#{name}] = value # @values[:includes] = value end # end CODE end @@ -417,7 +419,8 @@ module ActiveRecord if !VALID_UNSCOPING_VALUES.include?(scope) raise ArgumentError, "Called unscope() with invalid unscoping argument ':#{scope}'. Valid arguments are :#{VALID_UNSCOPING_VALUES.to_a.join(", :")}." end - set_value(scope, DEFAULT_VALUES[scope]) + assert_mutability! + @values[scope] = DEFAULT_VALUES[scope] when Hash scope.each do |key, target_value| if key != :where @@ -1005,17 +1008,6 @@ module ActiveRecord end private - # Returns a relation value with a given name - def get_value(name) - @values.fetch(name, DEFAULT_VALUES[name]) - end - - # Sets the relation value with the given name - def set_value(name, value) - assert_mutability! - @values[name] = value - end - def assert_mutability! raise ImmutableRelation if @loaded raise ImmutableRelation if defined?(@arel) && @arel @@ -1316,7 +1308,8 @@ module ActiveRecord def structurally_incompatible_values_for_or(other) values = other.values STRUCTURAL_OR_METHODS.reject do |method| - get_value(method) == values.fetch(method, DEFAULT_VALUES[method]) + default = DEFAULT_VALUES[method] + @values.fetch(method, default) == values.fetch(method, default) end end diff --git a/railties/test/application/test_runner_test.rb b/railties/test/application/test_runner_test.rb index 0bdd2b314d..1ab45abcd0 100644 --- a/railties/test/application/test_runner_test.rb +++ b/railties/test/application/test_runner_test.rb @@ -794,6 +794,32 @@ module ApplicationTests assert_match "Capybara.reset_sessions! called", output end + def test_failed_system_test_screenshot_should_be_taken_before_other_teardown + app_file "test/system/failed_system_test_screenshot_should_be_taken_before_other_teardown_test.rb", <<~RUBY + require "application_system_test_case" + require "selenium/webdriver" + + class FailedSystemTestScreenshotShouldBeTakenBeforeOtherTeardownTest < ApplicationSystemTestCase + ActionDispatch::SystemTestCase.class_eval do + def take_failed_screenshot + puts "take_failed_screenshot called" + super + end + end + + def teardown + puts "test teardown called" + super + end + + test "dummy" do + end + end + RUBY + output = run_test_command("test/system/failed_system_test_screenshot_should_be_taken_before_other_teardown_test.rb") + assert_match(/take_failed_screenshot called\n.*test teardown called/, output) + end + def test_system_tests_are_not_run_with_the_default_test_command app_file "test/system/dummy_test.rb", <<-RUBY require "application_system_test_case" |