Skip to content
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

return size of 0 if json ruby value is nil #80

Closed
wants to merge 1 commit into from

Conversation

eluns
Copy link
Contributor

@eluns eluns commented Sep 30, 2014

An entry can be null such as [null], which is 1 entry. If a null is a value of an Array, then the array has 0 entry. However a null value returns a size of 1

def matches?(json)
ruby = parse_json(json, @path)
#when ruby = nil
#NoMethodError: undefined method `size' for nil:NilClass
# so @Actual = 1 when ruby is nil
@Actual = Enumerable === ruby ? ruby.size : 1
@Actual == @expected
end

@eluns
Copy link
Contributor Author

eluns commented Sep 30, 2014

This scenario failed from the master
Failing Scenarios:
cucumber features/equivalence.feature:262 # Scenario: Table format can fail equivalence

@laserlemon
Copy link
Contributor

@eluns The features/equivalence.feature:262 test is expected to fails and actually asserts that it does.

I can't say that I agree with the presumption that null should be considered to have a size of zero. While I agree that if null were to have any size, it would be zero, I don't believe that the concept of size should apply to null at all.

Rather than evaluating null's size as zero, I would consider a change that includes a better failure message in the case that an enumerable is not encountered.

Thank you! I'll close this pull request in favor of a future request buttoning up the matcher's failure messaging.

@laserlemon laserlemon closed this Sep 30, 2014
@eluns
Copy link
Contributor Author

eluns commented Oct 1, 2014

Hi Steve,
Wow, thank you for the quick response back! I honestly thought my request would be sent off to never, never land. I do agree that null should not have a size, but if it were to have a size it would be zero. However currently, the def matches returns a "size" of 1 for null values. In the ternary, if the variable ruby is null, which is false therefore the ternary returns 1. With the logic that I think we agree on... wouldn't it be 0? I created a test in the size.feature that I think helps commute the possible bug. Deciding the expected behaviors of null values reminds me of pondering the philosophical question to be or not to be. Thank you again for your time!
Beth

}
"""
When I get the JSON
Then the JSON at "array_with_null" should have 1 entry
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that this assertion should pass.

@eluns
Copy link
Contributor Author

eluns commented Oct 1, 2014

Hi Steve,
Ah, yes my logic doesn't follow my code. Null should not return either 0 or 1. In my pull request, I made unexpected null values return false. Please let me know what you think.

Thanks,
Beth

@actual = 0
else
@actual = Enumerable === ruby ? ruby.size : 1
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be more appropriate (and less surprising) to change this to something like:

ruby = parse_json(json, @path)
raise EnumerableExpected.new("Enumerable expected, got #{ruby.inspect}.") unless Enumerable === ruby
@actual = ruby.size
@actual == @expected

Of course, we'd have to define the error class, write tests, etc.

@eluns
Copy link
Contributor Author

eluns commented Oct 6, 2014

Hi Steve,
I created Raise error when checking a size of a json ruby value of nil #82 .
Thanks,
Beth

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants