rspec / rspec-support

Common code needed by the other RSpec gems. Not intended for direct use.
https://rspec.info
MIT License
99 stars 102 forks source link

Check for nil instead of truthiness when making a diff #377

Closed lnestor closed 5 years ago

lnestor commented 5 years ago

Previous Behavior

The following lines create different output:

expect(true).to match a_falsy_value

# =>
       expected true to match (a falsey value)
       Diff:
       @@ -1,2 +1,2 @@
       -(a falsey value)
       +true
expect(false).to match a_truthy_value

# =>
       expected false to match (a truthy value)

The first generates a diff, whereas the second does not. This is because the actual and expected values are checked for presence with if actual && expected when creating the diff. In the first example, the actual is true, so it passes the check. In the second, the actual is false and the diff returned is an empty string.

New Behavior

These should be consistent with each other. The two options would be to make them both create a diff or modify the matcher so neither do. However, since all other match(something) matchers produce a diff output, it makes sense to do the same here.

If it is necessary for that check to also check for truthiness, then another solution could be found. But, I cannot find a situation where false would be passed in where we wouldn't want to create a diff.

lnestor commented 5 years ago

I went ahead and checked that a diff is produced for both. I can separate them into their own it instances if you want. Also, CI failed because it timed out, is there a way to retry the job?

pirj commented 5 years ago

@lnestor I suggest you to:

git commit --amend
git push --force-with-lease lnestor diff-generation-nil-check

to re-run the build.