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

API to search through calls using matcher syntax #88

Closed
motiz88 opened this issue Apr 14, 2016 · 3 comments
Closed

API to search through calls using matcher syntax #88

motiz88 opened this issue Apr 14, 2016 · 3 comments

Comments

@motiz88
Copy link
Contributor

motiz88 commented Apr 14, 2016

This is one of the two related issues that led me to submit #83.

I propose an API that applies a matcher (string|RegExp|function(url, opts)) to the whole list of calls, regardless of any configured routes or their names. I consider this behavior useful in composing assertions that aren't possible with calls(routeName) or called(routeName), such as:

// Assuming a new method: fetchMock.testCalls
fetchMock.mock('^http://example.com/');
fetch('http://example.com/?arg1=123').then(() => {
  // called() would return false for each of the following
  fetchMock.testCalls('http://example.com/?arg1=123'); // true
  fetchMock.testCalls('^http://example.com/?arg1='); // true
  fetchMock.testCalls(/arg1=\d+$/); // true
});

So basically, it would do what I had the impression called() and calls() should do, as described in #87.
#83 contains a proposed implementation in the form of callsMatching() and calledMatching() filterCalls() and testCalls().

@motiz88
Copy link
Contributor Author

motiz88 commented Apr 14, 2016

@wheresrhys Picking up from #83 (comment):

  1. I don't particularly want to have two supported ways of checking for matched calls - that will add to the complexity of the api, and thus increase confusion when it's being used. So if your suggestion was adopted I see it being a major release

Could clearer and more distinct naming (e.g. filterCalls(), testCalls() as in the updated PR #83) help reduce confusion despite the larger API surface?

@wheresrhys
Copy link
Owner

I'm afraid I'm against adding to the API unless this gets a few 👍s

@wheresrhys
Copy link
Owner

I've decided to close this - it's been a long time with no supporting comments. Thanks fro submitting though, and for highlighting the confusing nature of the docs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants