Follow up to https://github.com/rails/rails/pull/37313
- Adds regression tests
- Logs a warning in cases where assertions are nested in a way that's likely to be confusing
I'm writing this patch for two purposes:
1. I want to reduce the number of times `object_id` is called. Calling
`object_id` can have negative impacts on performance in Ruby 2.7+, so
it would be nice to stop calling it.
2. I'm not sure why we're treating lambdas specially here. It looks
like we wanted to prevent people from skipping callbacks that were
defined with a lambda, but I think that is silly. If the user has a
reference to a lambda, and they want to skip it, we should let them.
I think this cleans up some code, helps with performance, and is a more
intuitive interface.
This provides better more specific diffs when comparing complex objects.
Reverts bbe437faecca5fd6bdc2327a4bc7a31ba21afe2e.
Closes#37507. Closes#38220.
Co-authored-by: michdsouza <michd2005@gmail.com>
Co-authored-by: Rahul Purohit <rahulpuroht@gmail.com>
Co-authored-by: Nicholas Koh <nkohwl@gmail.com>
* Enable `Lint/UselessAssignment` cop to avoid unused variable warnings
Since we've addressed the warning "assigned but unused variable"
frequently.
370537de05092aeea552146b42042833212a1acc
3040446cece8e7a6d9e29219e636e13f180a1e03
5ed618e192e9788094bd92c51255dda1c4fd0eae
76ebafe594fc23abc3764acc7a3758ca473799e5
And also, I've found the unused args in c1b14ad which raises no warnings
by the cop, it shows the value of the cop.
When `to:` is passed to `assert_changes`, it now prints the well-known `"Expected: x\n Actual: y"` message.
Before, the message only contained the actual value.
* Support hash as first argument for `assert_difference`.
This allows to specify multiple numeric differences in the same assertion.
Example:
assert_difference 'Article.count' => 1, 'Notification.count' => 2 do
# post :create, params: { article: {...} }
end
* Support error message when passing a hash as a first parameter
* Format CHANGELOG properly
[Julien Meichelbeck + Rafael Mendonça França]
While using `assert_changes`, I came across some unexpected behavior:
if you provide a `to:` argument, and the expression matches but didn't
actually change, the assertion will pass.
The way `assert_changes` reads, I assumed that it would both assert
that there was any change at all, _and_ that the expression changed to
match my `to:` argument.
In the case of just a `from:` argument, `assert_changes` does what I
expect as well. It asserts that the before value `=== from` and that
the after value changed.
My key change is that `assert_changes` will now _always_ assert that
expression changes, no matter what combination of `from:` and `to:`
arguments
Seeing the previously issued PRs about it, we can avoid the `nil`
comparisons that can happen in `assert_changes` by using plain `assert`
calls.
This is to avoid a deprecation warning about comparing `nil` values in
`assert_equal` for Minitest 5 and a crash in Minitest 6.
You can see the preparations done in [`assert_equal`][ae]. You can also
see that [`assert`][a] does not care about `nil`s.
[ae]: ca6a71ca90/lib/minitest/assertions.rb (L159-L188)
[a]: ca6a71ca90/lib/minitest/assertions.rb (L131-L142)
`AlsoDoingNothingTest` was added in cf9be89.
It seems that it added to confirm that the test works in the child class
of `ActiveSupport::TestCase`.
But now basically use `ActiveSupport::TestCase` in test, so I think
it is unnecessary.
Those are assertions that I really do miss from the standard
`ActiveSupport::TestCase`. Think of those as a more general version of
`assert_difference` and `assert_no_difference` (those can be implemented
by assert_changes, should this change be accepted).
Why do we need those? They are useful when you want to check a
side-effect of an operation. `assert_difference` do cover a really
common case, but we `assert_changes` gives us more control. Having a
global error flag? You can test it easily with `assert_changes`. In
fact, you can be really specific about the initial state and the
terminal one.
```ruby
error = Error.new(:bad)
assert_changes -> { Error.current }, from: nil, to: error do
expected_bad_operation
end
```
`assert_changes` follows `assert_difference` and a string can be given
for evaluation as well.
```ruby
error = Error.new(:bad)
assert_changes 'Error.current', from: nil, to: error do
expected_bad_operation
end
```
Check out the test cases if you wanna see more examples.
🍻
With this we can perform new assertions on the returned value without having
to cache it with an outer variable or wrapping all subsequent assertions inside
the `assert_difference` block.
Before:
```
post = nil
assert_difference -> { Post.count }, 1 do
Post.create
end
assert_predicate post, :persisted?
```
Now:
```
post = assert_difference -> { Post.count } do
Post.create
end
assert_predicate post, :persisted?
```
Goals:
1. Default to :random for newly generated applications
2. Default to :sorted for existing applications with a warning
3. Only show the warning once
4. Only show the warning if the app actually uses AS::TestCase
Fixes#16769
+ Namespace changes, overhaul of runners.
+ Internal ivar name changes
- Removed a logger globally applied to tests that spew everywhere?!?
+ Override Minitest#__run to sort tests by name.
+ Reworked testing isolation to work with the new cleaner architecture.
- Removed a bunch of tests that just test minitest straight up. I think these changes were all merged to minitest 4 a long time ago.
- Minor report output differences.
Neither Test::Unit nor MiniTest rescue exceptions like Interrupt or
NoMemoryError, but ActiveSupport::Testing::SetupAndTeardown#run which
overrides MiniTest::Unit::TestCase#run rescues them.
Rescuing an Interrupt exception is annoying, because it means when you
are running a lot of tests e.g. when running one of the rake test tasks,
you cannot break out using ctrl-C.
Rescuing exceptions like NoMemoryError is foolish, because the most
sensible thing to happen is for the process to terminate as soon as
possible.
This solution probably needs some finessing e.g. I'm not clear whether
the assumption is that only MiniTest is supported. Also early versions
of MiniTest did not have this behaviour. However, hopefully it's a
start.
Integrating with Test::Unit & MiniTest has always been a pain. It would
be great if both of them provided sensible extension points for the kind
of things that both Rails and Mocha want to do.
