-
-
Notifications
You must be signed in to change notification settings - Fork 163
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
Make have_file_content diffable #562
Make have_file_content diffable #562
Conversation
Only if expected value is a string, to ensure existing behavior stays the same.
As far as I know we don't have any tests for diff-functionality of our matchers. LGTM. |
The AppVeyor build is failing due to ffi incompatibilities with ruby < 2.0, and byebug incompatibilities with ruby < 2.2. Perhaps we should remove the debuggers from CI? Edit: this is unrelated to this PR, so I'll merge this anyway. |
Hi @cllns, Thanks for your making your first contribution to Cucumber, and welcome to the Cucumber committers team! You can now push directly to this repo and all other repos under the cucumber organization! 🍾 In return for this generous offer we hope you will:
On behalf of the Cucumber core team, |
(@xtrasimplicity - removing debuggers from CI is a Good Thing.) |
Agreed. I'll submit a PR when I get back to my computer, if it hasn't already been resolved in master. Edit: There's a PR already, so I might jump on that. |
@aslakhellesoy Thanks, I just joined! A little disappointing to see that I've been automatically subscribed to notifications for all 35 repos in the cucumber group. I know it's not your fault, but there's no way to unsubscribe from all, so I have to click each one individually 😟 . Might just be a fault with GitHub's UI, I'm not sure, but maybe you can change the settings to not be automatically subscribed to more than just the most popular repos?
|
Summary
This makes the
have_file_content
matcherdiffable
in cases when it's useful, without breaking existing behavior.Details
Custom RSpec matchers aren't
diffable
by default. You have to manually specify that a matcher should be diffable. This change makes thishave_file_content
matcherdiffable
. In particular, it's important for files to be diffable, since they're likely going to be several lines, and RSpec will abbreviate the expectation / actual with a darned...
.Motivation and Context
I'm a core team member for Hanami, and we used to use MiniTest for everything. We have since changed all our internal test suites to RSpec, but the generated projects still use MiniTest. I'm working on changing that, and there are a number of changes to generated files.
A big part of the Hanami ethos is to be good Ruby ecosystem citizens. We want the whole Ruby community to work well together, since it's in all of our interests. This PR will help us (it's already helped me solve my problem locally) and I'm sure it'll help others too. 🌸
How Has This Been Tested?
I did not add a test for this, since I wasn't sure how file generation worked. I did make sure that the tests did not fail (that's why I added
.is_a?(String)
. Another approach I played with, which worked, was only showing the diff if the file was longer than one line.)I'm happy to add a test for the
diffable
behavior, I just need a little direction at this point :)Screenshots:
Before:
After:
"Better... much better"
Types of changes
Checklist: