-
Notifications
You must be signed in to change notification settings - Fork 22.1k
Wisetara/deprecate args active support test case#assert nothing raised for pr #23789
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Wisetara/deprecate args active support test case#assert nothing raised for pr #23789
Conversation
|
Well, there may be a bit too much warning! I see that now. But I don't believe sorting that out will fix the failure: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you have to check presence of arguments. Now it's giving deprecation error every time even if you don't send any parameter to it.
70fadc0 to
f888262
Compare
|
That's better! I hope this deprecation PR is helpful, @rafaelfranca! Please let me know if you'd like any additional changes. |
activesupport/CHANGELOG.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for mentioning me but we usually don't do that.
What do you think about?
Deprecate arguments on `assert_nothing_raised`.
`assert_nothing_raised` were not asserting the arguments that were being passed
(usually an specific exception class) since it only yields the block. To not confuse the
users that the arguments have any meaning they are being deprecated.
|
Awesome work. I made some comment. |
f888262 to
d96550b
Compare
|
Thank you for the comments and feedback. I've incorporated your |
…eSupport__TestCase#assert_nothing_raised-for-pr Wisetara/deprecate args active support test case#assert nothing raised for pr
|
Thank you |
Using arguments (to specify an exception error) with
assert_nothing_raisedis not recommended, as discussed with Rafael França on #23773.A deprecation warning has been added to
ActiveSupport::TestCase#assert_nothing_raised(*args), and all tests have been updated to useassert_nothing_raisedwithout arguments.