Added / Edited ActiveModel::Errors test cases. #11183
Conversation
| @@ -93,14 +93,45 @@ def test_has_key? | |||
| assert_equal [:foo, :baz], errors.keys | |||
| end | |||
|
|
|||
| test "detecting whether there are errors with empty?, blank?, include?" do | |||
| test 'empty?, blank? doesn\'t detect initial empty value as an error' do | |||
There was a problem hiding this comment.
Please leave it with double quotes to use doesn't, or change it to does not (I think it might be better to leave as double quotes as the rest of the file - also check other tests.)
|
Please rebase and remove your first commit (the doc fix) since I cherry-picked it already from the other pull request. |
| person.errors[:foo] = [] | ||
| assert person.errors.empty? | ||
| assert person.errors.blank? | ||
| end |
There was a problem hiding this comment.
Is this valid API? I think I never used it like that.
There was a problem hiding this comment.
I personally doubt it to be a valid API too and the difference result from include?/empty? and size/count(before this PR) is confusing. Just write test cases to reveal the difference here.
Need more suggestion.
- `empty?`, `blank?` and `include?` has different logics to detect an error, new test cases demostrate the difference
- Empty String(`''`) and Empty Array('[]') are edge cases, and they differentiate the behavior of the above methods.
Given the following code, size and count calculates to different behavior ```ruby person.errors.add(:name, []) person.errors.add(:name, '') person.errors.size person.errors.count ``` According to the doc, I alias `size` to `count`. When a error is added, it should be reported by `count` regardless of its message type.
|
When you say Looking for the implementation looks like the behavior is the same, only the implementation that is different |
|
There is subtle(if not buggy) behavior when message comes in as an Array which is allowed Here is the demo of difference: https://gist.github.com/yangchenyun/5915718 |
ghost
assigned
|
Message as Array should not be allowed. It can be a proc, symbol, or string. There is no sense to pass a complex object or a array as message. We should fix it. |
|
Looks like this can be closed since #19021 was merged |
include?andempty?sizeandcountbehave consistently.