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

Regex path matching issue #96

Open
pimterry opened this issue Jun 17, 2022 · 6 comments
Open

Regex path matching issue #96

pimterry opened this issue Jun 17, 2022 · 6 comments

Comments

@pimterry
Copy link
Member

Hello,

I was looking at allowing different RegEx patterns in the url matching.
RegEx test cases

I see in the test cases you only run tests with direct regex expressions and not strings.
await server.get(/.*.txt/).thenReply(200, 'Fake file');

Tried to create a regex pattern via new RegExp("some regex pattern string") but I am having match issues.

const regExPattern = new RegExp(regExPatternString, 'g');
await server.get(regExPattern).thenReply(200, 'Fake file');

Any tips ?

Originally posted by @chrisribe in #9 (comment)

@pimterry
Copy link
Member Author

Hi @chrisribe! I've just moved this to a separate issue since the previous one was resolved.

Yes, if you want to use regexes you do need to provide a real regex object. Any strings passed to .get(x) will be matched exactly, not as a regex pattern.

Your example looks correct though. Can you share some specific patterns that aren't working for you, and the requests that they're failing to match?

@chrisribe
Copy link

Thanks for the reply and new issue for this :)

My example test case that does not match properly.
url to match
https://www.w3schools.com/lib/my-learning.js?v=1.0.9

Code that does not match

server.forGet(/(.*)w3schools.com\/lib\/my-learning.js(.*)9$/)
      .thenReply(200, 'There is a match !');

@pimterry
Copy link
Member Author

That I can explain easily: forGet() matches against the path specifically, not the query string, so the 9 never matches anything.

If you want to match against the query string, you need do so explicitly, for example:

server.forGet(/(.*)w3schools.com\/lib\/my-learning.js/)
      // Either:
      .withExactQuery('?v=1.0.9')
      // Or:
      .withQuery({ v: '1.0.9' })
      .thenReply(200, 'There is a match !');

Does that make sense?

I can see how this is a bit surprising, especially since we include the rest of the base domain here nowadays. It's tricky to change backward compatibly though, and for non-regex cases with exact string you generally do want matching to work like this... I'll keep an eye on that and see if I can improve this in a future major version bump.

@chrisribe
Copy link

chrisribe commented Jun 17, 2022

Ah that would explain it !

Yes this is very surprising as I was expecting that passing a regex would simply match on that for the full url.
So another regex that fails for me, and this fails because of the Query check at ?
/(.*)w3schools.com\/lib\/my-learning.js\?v=(.*)9/

Any way to regex match on the full url and not use .withExactQuery or .withQuery ?

Like if I want to match on certain query parameter patterns ?v=1.(.*).9
RegexPathMatcher ?

@chrisribe
Copy link

I did find this showing all query param matchers do not support RegEx at all :(

query-matching.spec.ts

@pimterry
Copy link
Member Author

Yes, it's not possible to do query matching by regex right now. PRs very welcome though! It should be fairly easy to add a withQueryMatching(/regex/) matcher that would do what you want.

Alternatively, it might make sense to extend withQuery({ v: '1.0.0' }) to allow withQuery({ v: /1.0.[0-9]/ }), so you can provide regexes to match against individual query values, independent of the order.

If you're interested in adding that, I'd suggest starting by writing a couple of tests for it, then exploring the existing matcher definition implementations you can find defined around here:

/**
* Match only requests that include the exact query string provided.
* The query string must start with a ? or be entirely empty.
* @category Matching
*/
withExactQuery(query: string): this {
this.matchers.push(new ExactQueryMatcher(query));
return this;
}
.

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

No branches or pull requests

2 participants