test: deflake test-dns

[lpinca]

Prevent responses from being cached.

Fixes: #54124

[anonrig]

Would you mind adding some comments and explaning those magical (magical to me, any maybe others) numbers?

[lpinca]

Feel free to push on the branch, I think ttl: 0 is self explanatory.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.04%. Comparing base (9db6327) to head (9f599f7).
Report is 24 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #54902      +/-   ##
==========================================
- Coverage   88.05%   88.04%   -0.01%     
==========================================
  Files         651      651              
  Lines      183405   183405              
  Branches    35822    35822              
==========================================
- Hits       161499   161485      -14     
- Misses      15159    15171      +12     
- Partials     6747     6749       +2     

see 25 files with indirect coverage changes

[anonrig]

Feel free to push on the branch, I think ttl: 0 is self explanatory.

Why 333333 is flaky but 0 isn't, was not obvious to me

[lpinca]

It's written in the commit message. With a value greater than 0 the response is cached. See also the number of requests reaching the server.

[mhdawson]

Looking at the PR that added the test I don't see any reason why they chose a longer ttl, and ttl is described as a tradeoff between perf and reliability where 0 would reliably get the most current data. I'm not quite sure why the caching would cause the test to be intermittent, but if if @lpinca's runs/experimentation shows that then I'm happy to land.

[mhdawson]

LGTM

[lpinca]

I'm not quite sure why the caching would cause the test to be intermittent

Because the returned ttl value is updated for cached responses

const assert = require('assert');
const dgram = require('dgram');
const dns = require('dns/promises');
const dnstools = require('../common/dns');
const { setTimeout } = require('timers/promises');

const server = dgram.createSocket('udp4');

server.once('message', function (msg, { address, port }) {
  const parsed = dnstools.parseDNSPacket(msg);
  const domain = parsed.questions[0].domain;
  assert.strictEqual(domain, 'example.org');

  server.send(
    dnstools.writeDNSPacket({
      id: parsed.id,
      questions: parsed.questions,
      answers: [{ domain, type: 'A', address: '1.2.3.4', ttl: 123 }]
    }),
    port,
    address
  );
});

server.bind(0, async function () {
  await dns.setServers([`127.0.0.1:${server.address().port}`]);

  let result = await dns.resolve4('example.org', { ttl: true });
  console.log(result); // [ { address: '1.2.3.4', ttl: 123 } ]

  await setTimeout(1000);

  result = await dns.resolve4('example.org', { ttl: true });
  console.log(result); // [ { address: '1.2.3.4', ttl: 122 } ]

  await setTimeout(1000);

  result = await dns.resolve4('example.org', { ttl: true });
  console.log(result); // [ { address: '1.2.3.4', ttl: 121 } ]

  server.close();
});

and it is irrelevant for the purposes of the test.

P.S. There might be other DNS tests affected by the same issue.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/62414/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/62419/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/62422/

[nodejs-github-bot]

Landed in 3569493