-
-
Notifications
You must be signed in to change notification settings - Fork 733
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
Tests should send mocked requests to fake hostname #1351
Comments
Mind if I take this? Seeing as its marked as a first time issue, should I let someone else take it? :) |
Go for it!! I try to be really generous with tagging good first issues. There's a ton of work that can be done on this project and I don't think we need to worry about not creating enough of it. 😁 |
Thanks @paulmelnikow, this great! |
Alright. I think I got most of these done. Some of the tests make assertions based on actually getting through - like enable net connect and allowUnmocked options. How about setting google.com for those requests that actually have to go through, and a.google.com for subdomain checks? |
Ah, I came across one like that before as well. If the tests are still based on getting through, they need to be rewritten. Basically, they should have used |
Oh right. Yeah I'll add TODOs and make an PR out of it. |
Maybe instead of using |
@jlilja How are you coming with this? It'd be great to get these updated so new contributors add tests in the style we want. Even if it's only partly done we could merge it. |
Hi, yes of course. I'm working on it right now. I'll push the changing of names soon, then another PR for converting the other ones needing its own server. |
I may came late to this but, |
I think
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We try to do our best, but nock is maintained by volunteers and there is only so much we can do at a time. Thank you for your contributions. |
I think this completes nock#1351
I think this is done. @paulmelnikow do you want to give the tests one last check before closing this issue? |
These are the ones I'm still seeing:
I'm not sure if any of these involve sending requests, though I think it's better we replace these with |
I've opened PRs for the rest of these. |
Let's change them all from e.g.
example.com
andwww.google.com
toexample.test
.See #1347 (comment)
The text was updated successfully, but these errors were encountered: