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

Add warning about be_false being falsey in rspec 1.3 #15

Merged
merged 1 commit into from
Jul 23, 2014

Conversation

krisleech
Copy link
Contributor

No description provided.

@khiet
Copy link

khiet commented Jul 22, 2014

💯 agree

@mottalrd
Copy link
Contributor

Looks good 👍
I'll leave it open such that people will actually read it :)

@thejspr
Copy link
Contributor

thejspr commented Jul 22, 2014

Nice gotcha

@dncrht
Copy link
Contributor

dncrht commented Jul 22, 2014

Suggestion, for the end:
Rspec 1.3 only: For methods which return a boolean, e.g. active?, use be(false) or eq false

@krisleech
Copy link
Contributor Author

@dncrht Do you know if be and eq are aliases. I think be is an alias for ==, maybe all three are the same?

@krisleech
Copy link
Contributor Author

For reference I also found a PR for the addition of be_falsey and change to be_false: rspec/rspec-expectations#283

@dncrht
Copy link
Contributor

dncrht commented Jul 23, 2014

@krisleech nope:

a.should be(b) # passes if a.equal?(b)
a.should eq(b) # passes if a == b

Keeping this in mind:

a.equal?(b) # object identity - a and b refer to the same object
a.eql?(b) # object equivalence - a and b have the same value
a == b # object equivalence - a and b have the same value with type conversions

https://www.relishapp.com/rspec/rspec-expectations/v/2-0/docs/matchers/equality-matchers#compare-using-eq-(==)

Thanks for asking! (we all will learn something today)

@krisleech
Copy link
Contributor Author

I didn't realise be was that strict. In our case be / equal? and eq / == will give the same result since there is only one false object AFAIK?

Do you think in this case we should just recommend one, either be(false) or eq(false) for consistency?

@dncrht
Copy link
Contributor

dncrht commented Jul 23, 2014

same result since there is only one false object AFAIK?

Indeed

Do you think in this case we should just recommend one, either be(false) or eq(false) for consistency?

I'd say so, but I'm open to suggestions

@krisleech
Copy link
Contributor Author

👍 I have no preference over be(false) or eq(false).

@dncrht
Copy link
Contributor

dncrht commented Jul 23, 2014

+1 for eq false as is the common form in our new services

@krisleech
Copy link
Contributor Author

Done 😄 Thanks @dncrht

dncrht added a commit that referenced this pull request Jul 23, 2014
Add warning about be_false being falsey in rspec 1.3
@dncrht dncrht merged commit 0863564 into master Jul 23, 2014
@dncrht dncrht deleted the rspec-be_false-warning branch July 23, 2014 09:00
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.

5 participants