Skip to content

Commit

Permalink
fix(dns): remove lookupPromise polyfill for node8 dns promise tests (#…
Browse files Browse the repository at this point in the history
…1223)

* fix(dns): remove lookupPromise polyfill for node8 dns promise tests

The `lookupPromise` function emulates a Promise based dns.lookup by
creating its own Promise() object, and then calling the async callback
version of `dns.lookup()` inside it.

While this does make the tests work under Node 8, it is not at all
what these tests were originally intended to test. That is, all of the
functions in this file were testing the polyfill in this test file,
and *not* testing the promise based version of dns.lookup() that was
introduced in Node 10.6.0.

Tellingly, the tests in this code never exercise the patch code in
instrumentation.ts that handles the non-callback (i.e. Promise-based)
version of the lookup() function.

This change removes the irrelevant test function and instead reverts
to directly calling dns.promises.lookup() as existed in this code
prior to the "cleanup" in PR# 110 that nominally was "Setting up
ESLint".

* chore(lint): eliminate unused type parameter in dns instrumentation

Co-authored-by: Rauno Viskus <Rauno56@users.noreply.github.com>
  • Loading branch information
evantorrie and rauno56 authored Oct 10, 2022
1 parent 180b336 commit 2777a79
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export const setLookupAttributes = (
* @param obj obj to inspect
* @param pattern Match pattern
*/
export const satisfiesPattern = <T>(
export const satisfiesPattern = (
constant: string,
pattern: IgnoreMatcher
): boolean => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,22 +30,6 @@ const memoryExporter = new InMemorySpanExporter();
const provider = new NodeTracerProvider();
provider.addSpanProcessor(new SimpleSpanProcessor(memoryExporter));

async function lookupPromise(
hostname: string,
options: dns.LookupOptions = {}
): Promise<any> {
return new Promise((resolve, reject) => {
dns.lookup(hostname, options, (err, address, family) => {
if (err) reject(err);
if (options.all) {
resolve(address);
} else {
resolve({ address, family });
}
});
});
}

describe('dns.promises.lookup()', () => {
let instrumentation: DnsInstrumentation;

Expand Down Expand Up @@ -83,7 +67,7 @@ describe('dns.promises.lookup()', () => {
[4, 6].forEach(ipversion => {
it(`should export a valid span with "family" arg to ${ipversion}`, async () => {
const hostname = 'google.com';
const { address, family } = await lookupPromise(hostname, {
const { address, family } = await dns.promises.lookup(hostname, {
family: ipversion,
});
assert.ok(address);
Expand All @@ -100,7 +84,7 @@ describe('dns.promises.lookup()', () => {
describe('with no options param', () => {
it('should export a valid span', async () => {
const hostname = 'google.com';
const { address, family } = await lookupPromise(hostname);
const { address, family } = await dns.promises.lookup(hostname);

assert.ok(address);
assert.ok(family);
Expand All @@ -119,7 +103,7 @@ describe('dns.promises.lookup()', () => {
it('should export a valid span with error NOT_FOUND', async () => {
const hostname = 'ᚕ';
try {
await lookupPromise(hostname);
await dns.promises.lookup(hostname);
assert.fail();
} catch (error) {
const spans = memoryExporter.getFinishedSpans();
Expand All @@ -141,7 +125,7 @@ describe('dns.promises.lookup()', () => {
it('should export a valid span with error INVALID_ARGUMENT when "family" param is equal to -1', async () => {
const hostname = 'google.com';
try {
await lookupPromise(hostname, { family: -1 });
await dns.promises.lookup(hostname, { family: -1 });
assert.fail();
} catch (error) {
const spans = memoryExporter.getFinishedSpans();
Expand All @@ -163,8 +147,7 @@ describe('dns.promises.lookup()', () => {
it('should export a valid span with error INVALID_ARGUMENT when "hostname" param is a number', async () => {
const hostname = 1234;
try {
// tslint:disable-next-line:no-any
await lookupPromise(hostname as any, { family: 4 });
await dns.promises.lookup(hostname as any, { family: 4 });
assert.fail();
} catch (error) {
const spans = memoryExporter.getFinishedSpans();
Expand All @@ -187,7 +170,7 @@ describe('dns.promises.lookup()', () => {
[4, 6].forEach(ipversion => {
it(`should export a valid span with "family" to ${ipversion}`, async () => {
const hostname = 'google.com';
const { address, family } = await lookupPromise(hostname, {
const { address, family } = await dns.promises.lookup(hostname, {
family: ipversion,
});

Expand All @@ -203,7 +186,7 @@ describe('dns.promises.lookup()', () => {

it(`should export a valid span when setting "verbatim" property to true and "family" to ${ipversion}`, async () => {
const hostname = 'google.com';
const { address, family } = await lookupPromise(hostname, {
const { address, family } = await dns.promises.lookup(hostname, {
family: ipversion,
verbatim: true,
});
Expand All @@ -221,7 +204,7 @@ describe('dns.promises.lookup()', () => {

it('should export a valid span when setting "all" property to true', async () => {
const hostname = 'montreal.ca';
const addresses = await lookupPromise(hostname, { all: true });
const addresses = await dns.promises.lookup(hostname, { all: true });

assert.ok(addresses instanceof Array);

Expand Down

0 comments on commit 2777a79

Please sign in to comment.