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

feature: uri pattern for test mocking #552

Merged
merged 8 commits into from
Oct 26, 2016
Merged

feature: uri pattern for test mocking #552

merged 8 commits into from
Oct 26, 2016

Conversation

midnight-wonderer
Copy link

Add regular expression to middleware matcher.

@midnight-wonderer
Copy link
Author

The failed test has nothing to do with the feature in this pull request.

Gem::InstallError: addressable requires Ruby version >= 1.9.0.
Installing rack-test 0.6.3
An error occurred while installing addressable (2.4.0), and Bundler cannot

mislav
mislav reviewed Dec 31, 2015
View reviewed changes
lib/faraday/adapter/test.rb
@@ -85,7 +85,7 @@ def verify_stubbed_calls
protected

def new_stub(request_method, path, headers = {}, body=nil, &block)
normalized_path = Faraday::Utils.normalize_path(path)
normalized_path = (path.is_a? Regexp) ? path : Faraday::Utils.normalize_path(path)
Copy link
Contributor

Choose a reason for hiding this comment

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

Style nitpick: could it be path.is_a?(Regexp) instead?

@mislav
Copy link
Contributor

mislav commented Dec 31, 2015

Thanks for the feature proposal! Sorry about the CI failure; you can merge in latest master to fix that.

In addition to addressing my thoughts in inline comments, could you also add code docs for this feature? I realize there is very little documentation for the Test adapter right now. Your PR could try to improve this while adding a new feature. At least some example use would be nice.

@midnight-wonderer
Copy link
Author

@mislav all done
Let me know if you have any more suggestion for new code added.

@iMacTia
Copy link
Member

iMacTia commented Oct 21, 2016

Hi @MidnightWonderer and sorry if your PR remained suspended for so much time.
I would like to merge it in, but I think that tests are outdated now. Can you please pull latest changes from master once more? thanks in advance

@midnight-wonderer
Copy link
Author

@iMacTia done.
The tests are still passed without doing anything.

@iMacTia iMacTia self-assigned this Oct 26, 2016
@iMacTia
Copy link
Member

iMacTia commented Oct 26, 2016

This took a long time, but I'm happy to merge it in as it actually makes tests more powerful.
Good work @MidnightWonderer 👍 !

@iMacTia iMacTia merged commit fb6ef20 into lostisland:master Oct 26, 2016
@iMacTia iMacTia mentioned this pull request Feb 9, 2017
Closed
@iMacTia iMacTia mentioned this pull request May 4, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@iMacTia iMacTia

Labels
feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@midnight-wonderer @mislav @iMacTia