Skip to content

Commit a985ed6

Browse files
Trottjasnell
authored andcommitted
test: refactor test-dns
* track callback invocations with common.mustCall() and common.mustNotCall() * remove test in test/internet/test-dns.js that is duplicated in test/parallel/test-dns.js * move tests that might perform a DNS query from test test/parallel/test-dns.js to test/internet/test-dns.js PR-URL: #13163 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
1 parent 7fe5303 commit a985ed6

File tree

2 files changed

+51
-48
lines changed

2 files changed

+51
-48
lines changed

test/internet/test-dns.js

+9-13
Original file line numberDiff line numberDiff line change
@@ -402,19 +402,6 @@ TEST(function test_lookup_failure(done) {
402402
});
403403

404404

405-
TEST(function test_lookup_null(done) {
406-
const req = dns.lookup(null, function(err, ip, family) {
407-
assert.ifError(err);
408-
assert.strictEqual(ip, null);
409-
assert.strictEqual(family, 4);
410-
411-
done();
412-
});
413-
414-
checkWrap(req);
415-
});
416-
417-
418405
TEST(function test_lookup_ip_all(done) {
419406
const req = dns.lookup('127.0.0.1', {all: true}, function(err, ips, family) {
420407
assert.ifError(err);
@@ -578,3 +565,12 @@ process.on('exit', function() {
578565
assert.strictEqual(expected, completed);
579566
assert.ok(getaddrinfoCallbackCalled);
580567
});
568+
569+
570+
assert.doesNotThrow(() => dns.lookup('nodejs.org', 6, common.mustCall()));
571+
572+
assert.doesNotThrow(() => dns.lookup('nodejs.org', {}, common.mustCall()));
573+
574+
assert.doesNotThrow(() => dns.lookupService('0.0.0.0', '0', common.mustCall()));
575+
576+
assert.doesNotThrow(() => dns.lookupService('0.0.0.0', 0, common.mustCall()));

test/parallel/test-dns.js

+42-35
Original file line numberDiff line numberDiff line change
@@ -91,34 +91,47 @@ assert.doesNotThrow(() => dns.setServers([]));
9191
assert.deepStrictEqual(dns.getServers(), []);
9292

9393
assert.throws(() => {
94-
dns.resolve('test.com', [], common.noop);
94+
dns.resolve('test.com', [], common.mustNotCall());
9595
}, function(err) {
9696
return !(err instanceof TypeError);
9797
}, 'Unexpected error');
9898

99-
// dns.lookup should accept falsey and string values
100-
const errorReg =
101-
/^TypeError: Invalid arguments: hostname must be a string or falsey$/;
99+
// dns.lookup should accept only falsey and string values
100+
{
101+
const errorReg =
102+
/^TypeError: Invalid arguments: hostname must be a string or falsey$/;
102103

103-
assert.throws(() => dns.lookup({}, common.noop), errorReg);
104+
assert.throws(() => dns.lookup({}, common.mustNotCall()), errorReg);
104105

105-
assert.throws(() => dns.lookup([], common.noop), errorReg);
106+
assert.throws(() => dns.lookup([], common.mustNotCall()), errorReg);
106107

107-
assert.throws(() => dns.lookup(true, common.noop), errorReg);
108+
assert.throws(() => dns.lookup(true, common.mustNotCall()), errorReg);
108109

109-
assert.throws(() => dns.lookup(1, common.noop), errorReg);
110+
assert.throws(() => dns.lookup(1, common.mustNotCall()), errorReg);
110111

111-
assert.throws(() => dns.lookup(common.noop, common.noop), errorReg);
112+
assert.throws(() => dns.lookup(common.mustNotCall(), common.mustNotCall()),
113+
errorReg);
114+
}
112115

113-
assert.doesNotThrow(() => dns.lookup('', common.noop));
116+
// dns.lookup should accept falsey values
117+
{
118+
const checkCallback = (err, address, family) => {
119+
assert.ifError(err);
120+
assert.strictEqual(address, null);
121+
assert.strictEqual(family, 4);
122+
};
114123

115-
assert.doesNotThrow(() => dns.lookup(null, common.noop));
124+
assert.doesNotThrow(() => dns.lookup('', common.mustCall(checkCallback)));
116125

117-
assert.doesNotThrow(() => dns.lookup(undefined, common.noop));
126+
assert.doesNotThrow(() => dns.lookup(null, common.mustCall(checkCallback)));
118127

119-
assert.doesNotThrow(() => dns.lookup(0, common.noop));
128+
assert.doesNotThrow(() => dns.lookup(undefined,
129+
common.mustCall(checkCallback)));
120130

121-
assert.doesNotThrow(() => dns.lookup(NaN, common.noop));
131+
assert.doesNotThrow(() => dns.lookup(0, common.mustCall(checkCallback)));
132+
133+
assert.doesNotThrow(() => dns.lookup(NaN, common.mustCall(checkCallback)));
134+
}
122135

123136
/*
124137
* Make sure that dns.lookup throws if hints does not represent a valid flag.
@@ -130,59 +143,53 @@ assert.doesNotThrow(() => dns.lookup(NaN, common.noop));
130143
* flags are either === 1 or even.
131144
*/
132145
assert.throws(() => {
133-
dns.lookup('www.google.com', { hints: (dns.V4MAPPED | dns.ADDRCONFIG) + 1 },
134-
common.noop);
146+
dns.lookup('nodejs.org', { hints: (dns.V4MAPPED | dns.ADDRCONFIG) + 1 },
147+
common.mustNotCall());
135148
}, /^TypeError: Invalid argument: hints must use valid flags$/);
136149

137-
assert.throws(() => dns.lookup('www.google.com'),
150+
assert.throws(() => dns.lookup('nodejs.org'),
138151
/^TypeError: Invalid arguments: callback must be passed$/);
139152

140-
assert.throws(() => dns.lookup('www.google.com', 4),
153+
assert.throws(() => dns.lookup('nodejs.org', 4),
141154
/^TypeError: Invalid arguments: callback must be passed$/);
142155

143-
assert.doesNotThrow(() => dns.lookup('www.google.com', 6, common.noop));
144-
145-
assert.doesNotThrow(() => dns.lookup('www.google.com', {}, common.noop));
146-
147-
assert.doesNotThrow(() => dns.lookup('', {family: 4, hints: 0}, common.noop));
156+
assert.doesNotThrow(() => dns.lookup('', {family: 4, hints: 0},
157+
common.mustCall()));
148158

149159
assert.doesNotThrow(() => {
150160
dns.lookup('', {
151161
family: 6,
152162
hints: dns.ADDRCONFIG
153-
}, common.noop);
163+
}, common.mustCall());
154164
});
155165

156-
assert.doesNotThrow(() => dns.lookup('', {hints: dns.V4MAPPED}, common.noop));
166+
assert.doesNotThrow(() => dns.lookup('', {hints: dns.V4MAPPED},
167+
common.mustCall()));
157168

158169
assert.doesNotThrow(() => {
159170
dns.lookup('', {
160171
hints: dns.ADDRCONFIG | dns.V4MAPPED
161-
}, common.noop);
172+
}, common.mustCall());
162173
});
163174

164175
assert.throws(() => dns.lookupService('0.0.0.0'),
165176
/^Error: Invalid arguments$/);
166177

167-
assert.throws(() => dns.lookupService('fasdfdsaf', 0, common.noop),
178+
assert.throws(() => dns.lookupService('fasdfdsaf', 0, common.mustNotCall()),
168179
/^TypeError: "host" argument needs to be a valid IP address$/);
169180

170-
assert.doesNotThrow(() => dns.lookupService('0.0.0.0', '0', common.noop));
171-
172-
assert.doesNotThrow(() => dns.lookupService('0.0.0.0', 0, common.noop));
173-
174-
assert.throws(() => dns.lookupService('0.0.0.0', null, common.noop),
181+
assert.throws(() => dns.lookupService('0.0.0.0', null, common.mustNotCall()),
175182
/^TypeError: "port" should be >= 0 and < 65536, got "null"$/);
176183

177184
assert.throws(
178-
() => dns.lookupService('0.0.0.0', undefined, common.noop),
185+
() => dns.lookupService('0.0.0.0', undefined, common.mustNotCall()),
179186
/^TypeError: "port" should be >= 0 and < 65536, got "undefined"$/
180187
);
181188

182-
assert.throws(() => dns.lookupService('0.0.0.0', 65538, common.noop),
189+
assert.throws(() => dns.lookupService('0.0.0.0', 65538, common.mustNotCall()),
183190
/^TypeError: "port" should be >= 0 and < 65536, got "65538"$/);
184191

185-
assert.throws(() => dns.lookupService('0.0.0.0', 'test', common.noop),
192+
assert.throws(() => dns.lookupService('0.0.0.0', 'test', common.mustNotCall()),
186193
/^TypeError: "port" should be >= 0 and < 65536, got "test"$/);
187194

188195
assert.throws(() => dns.lookupService('0.0.0.0', 80, null),

0 commit comments

Comments
 (0)