diff options
author | claudiob <claudiob@gmail.com> | 2014-12-15 08:26:41 -0800 |
---|---|---|
committer | claudiob <claudiob@gmail.com> | 2014-12-15 16:36:50 -0800 |
commit | 8dfa585db2539d8e5d933ac57e19c07fb6daa953 (patch) | |
tree | ff2771493be4495cd460e65a77b79b5442f3f449 /actionpack/test | |
parent | 1b9dca54fd5c65ed8b4aa96e2b6826cefb2c3e62 (diff) | |
download | rails-8dfa585db2539d8e5d933ac57e19c07fb6daa953.tar.gz rails-8dfa585db2539d8e5d933ac57e19c07fb6daa953.tar.bz2 rails-8dfa585db2539d8e5d933ac57e19c07fb6daa953.zip |
Remove misleading test: around_action return false
When an `around_action` does not `yield`, then the corresponding action is
*never* executed and the `after_` actions are *never* invoked.
The value returned by the `around_action` does not have any impact on this:
an `around_action` can "return" `true`, `false`, or `"pizza"`, but as long
as `yield` is not invoked, the corresponding action and after callbacks are
not executed.
The test suite for `ActionController::Callbacks` currently includes separate
tests to distinguish the cases in which a non-yielding `around_actions` returns
`true` or `false`.
In my opinion, having such tests is misleading, giving the impression that the
returned value might have some sort of impact, while it does not. At least
that's the impression I got when I read those tests.
For completeness, the tests were introduced 7 years ago by @NZKoz in e80fabb.
Diffstat (limited to 'actionpack/test')
-rw-r--r-- | actionpack/test/controller/filters_test.rb | 20 |
1 files changed, 1 insertions, 19 deletions
diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index 38533dbf23..829729eb1b 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -504,7 +504,6 @@ class FilterTest < ActionController::TestCase def non_yielding_action @filters << "it didn't yield" - @filter_return_value end def action_three @@ -528,32 +527,15 @@ class FilterTest < ActionController::TestCase end end - def test_non_yielding_around_actions_not_returning_false_do_not_raise + def test_non_yielding_around_actions_do_not_raise controller = NonYieldingAroundFilterController.new - controller.instance_variable_set "@filter_return_value", true assert_nothing_raised do test_process(controller, "index") end end - def test_non_yielding_around_actions_returning_false_do_not_raise - controller = NonYieldingAroundFilterController.new - controller.instance_variable_set "@filter_return_value", false - assert_nothing_raised do - test_process(controller, "index") - end - end - - def test_after_actions_are_not_run_if_around_action_returns_false - controller = NonYieldingAroundFilterController.new - controller.instance_variable_set "@filter_return_value", false - test_process(controller, "index") - assert_equal ["filter_one", "it didn't yield"], controller.assigns['filters'] - end - def test_after_actions_are_not_run_if_around_action_does_not_yield controller = NonYieldingAroundFilterController.new - controller.instance_variable_set "@filter_return_value", true test_process(controller, "index") assert_equal ["filter_one", "it didn't yield"], controller.assigns['filters'] end |