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

add option to disable regexp host matching #49

Merged
merged 1 commit into from
Jun 18, 2019

Conversation

123hurray
Copy link
Contributor

MatchHost function treats host string as plain string but also regexp, which might cause some unexpected condition, like foo.bar.com matches foo-bar.com.
However nock can distinguish plain string and regexp (regexp in js surrounded by /)

This PR adds request.DisableRegexpHostMatching to treat host only as string. After calling DisableRegexpHostMatching, MatchHost will not using regular expression. Regexp host matching is enabled default to keep compatibility.

Before:

gock.New("http://foo.bar.com").
		Get("/bar").
		Reply(200).
		JSON(map[string]string{"foo": "bar"})
http.Get("http://foo-bar.com/bar") // match

After calling DisableRegexpHostMatching

gock.New("http://foo.bar.com").
		DisableRegexpHostMatching().
		Get("/bar").
		Reply(200).
		JSON(map[string]string{"foo": "bar"})
http.Get("http://foo-bar.com/bar") // not match

@123hurray 123hurray changed the title add option to disable MatchHost match regexp host add option to disable regexp host matching Apr 11, 2019
@h2non
Copy link
Owner

h2non commented Apr 11, 2019

What about this instead:

gock.New("http://foo.bar.com").
  WithOptions(gock.Options{ DisableRegexpHost: true })

@123hurray
Copy link
Contributor Author

What about this instead:

gock.New("http://foo.bar.com").
  WithOptions(gock.Options{ DisableRegexpHost: true })

Great! Will update it.

@h2non h2non merged commit 30a20be into h2non:master Jun 18, 2019
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

Successfully merging this pull request may close these issues.

2 participants