test: Fix DNS lookup in remote-method test #1937
Conversation
| @@ -261,6 +262,16 @@ tap.test('when the connection fails', (t) => { | |||
| }) | |||
|
|
|||
| t.test('should correctly handle a DNS lookup failure', (t) => { | |||
| const lookup = dns.lookup | |||
There was a problem hiding this comment.
this is fine but could just use sinon too
const err = Error('no dns')
err.code = dns.NOTFOUND
sinon.stub(dns, 'lookup').yields(err)
t.teardown(() => {
dns.lookup.restore()
})There was a problem hiding this comment.
Does that provide anything we need here?
There was a problem hiding this comment.
not really. we typically use sinon for mocking but older tests will still have patching style like this. I just wanted to call it out, feel free to merge
There was a problem hiding this comment.
I am not at all familiar with the sinon API. I like the least amount of mystery in tests as possible.
There was a problem hiding this comment.
that's fair. I suggest you familiarize yourself as this repo does use it heavily. but again not a big deal here
| @@ -261,6 +262,16 @@ tap.test('when the connection fails', (t) => { | |||
| }) | |||
|
|
|||
| t.test('should correctly handle a DNS lookup failure', (t) => { | |||
There was a problem hiding this comment.
do you also want to mock the test above too?
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1937 +/- ##
=======================================
Coverage 97.02% 97.02%
=======================================
Files 215 215
Lines 40239 40239
=======================================
Hits 39040 39040
Misses 1199 1199
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
The current test relies on whatever DNS server the system uses to return a "not found" answer for
failed.domain.clxrg. There are two problems with this:This PR solves both problems.