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.

Sign up for GitHub

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

Jump to bottom

Added matcher for file content #238

Merged
merged 9 commits into from May 7, 2015
Merged

Added matcher for file content #238

merged 9 commits into from May 7, 2015

Conversation

ghost
Copy link

@ghost ghost commented Apr 28, 2015

I added a matcher to match string to file content. This might be useful for aruba internals as well, but is itended to be part of the public API.

This PR is based on a refactoring done in #237, that's why there are some more commits added here. I will rebase this PR.

@ghost
Copy link
Author

ghost commented Apr 28, 2015

@jarl-dk @mattwynne Suggestions? I split this up in two PRs. This is why this one depends on changes in #237

@maxmeyer
Copy link
Member

maxmeyer commented May 2, 2015

I added another matcher... 😄

@mattwynne
Copy link
Member

It looks to me like the have_file_content matcher will not show a diff if the contents of two files are different. Our string comparison (e.g. for stdout) does this, and it's very handy.

mattwynne added a commit that referenced this pull request May 7, 2015
@mattwynne
Merge pull request #238 from dg-ratiodata/feature/have_content
60218c6 
Added matcher for file content
@mattwynne mattwynne merged commit 60218c6 into cucumber:master May 7, 2015
@mattwynne
Copy link
Member

I suggest adding that in another iteration.

@ghost
Copy link
Author

ghost commented May 7, 2015

I learned yesterday about "diffable" in a matcher. Will add this.

@mattwynne
Copy link
Member

Cool!

You know, being really picky, it strikes me that the if statement checking the type of the expected value (Regexp or String) appears in three places in the matcher, and it would be better to use polymorphism, and just have two different matcher objects, with the factory method picking the right one.

I'll see if I can show you what I mean.

mattwynne added a commit that referenced this pull request May 7, 2015
@mattwynne
Demonstration of using polymorphism for matchers
5471f78 
@maxmeyer here's what I was talking about in #238. Just a suggestion -
WDYT?

Note that I had to pass in a reference to `aruba` so that I could still
get to the `absolute_path` helper method. Maybe we can put this in a
module that can be mixed into the helpers if we like this way of doing
things...?

@myronmarston I'd appreciate your feedback about whether there's a
neater way to do this if you have time - particularly the namespacing of
the factory method - I used RSpec::Matchers just because I assumed
that's where the DSL puts them.
This was referenced May 11, 2015
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@maxmeyer @mattwynne