Skip to content
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

internet/test-dns fails on v6.x #19032

Closed
gibfahn opened this issue Feb 27, 2018 · 6 comments
Closed

internet/test-dns fails on v6.x #19032

gibfahn opened this issue Feb 27, 2018 · 6 comments
Assignees
Labels
test Issues and PRs related to the tests.

Comments

@gibfahn
Copy link
Member

gibfahn commented Feb 27, 2018

  • Version: 6.13.0
  • Platform: macOS High Sierra (suspected to be all platforms though)
  • Subsystem: test

Reproduction:

# Inside a clone of node:
git checkout v6.13.0
nvm install 6.13.0
node test/internet/test-dns.js

Error:

▶▶▶ node test/internet/test-dns.js                                                                                                                                      ~/wrk/com/node (!tags/v6.13.0^0)
test_reverse_bogus
test_resolve4_ttl
looking up nodejs.org...
test_resolve6_ttl
nodejs.org =  [ '104.20.22.46', '104.20.23.46' ]
test_resolveMx
test_resolveMx_failure
test_resolveNs
test_resolveNs_failure
test_resolveSrv
test_resolveSrv_failure
test_resolvePtr
test_resolvePtr_failure
test_resolveNaptr
test_resolveNaptr_failure
test_resolveSoa
test_resolveSoa_failure
test_resolveCname
test_resolveCname_failure
test_resolveTxt
test_resolveTxt_failure
17 tests completed

assert.js:84
  throw new assert.AssertionError({
  ^
AssertionError: 2 === 1
    at /Users/gib/wrk/com/node/test/internet/test-dns.js:361:12
    at QueryReqWrap.asyncCallback [as callback] (dns.js:62:16)
    at QueryReqWrap.onresolve [as oncomplete] (dns.js:221:10)
@gibfahn gibfahn added test Issues and PRs related to the tests. v6.x labels Feb 27, 2018
@gibfahn
Copy link
Member Author

gibfahn commented Feb 27, 2018

This doesn't fail on 8.x, so I suspect a change was made to the tests that hasn't been backported yet.

@gibfahn
Copy link
Member Author

gibfahn commented Feb 27, 2018

Problem is in this function:

TEST(function test_resolveTxt(done) {
const req = dns.resolveTxt(addresses.TXT_HOST, function(err, records) {
assert.ifError(err);
assert.strictEqual(records.length, 1);

On v6.x we expect 'google.com' to have records.length == 1, but it is now two:

▶▶▶ node -p "require('dns').resolveTxt('google.com', (err, records) => { console.log(records.length) } )"                                                               ~/wrk/com/node (!tags/v6.13.0^0✦)
QueryReqWrap {
  bindingName: 'queryTxt',
  callback: { [Function: asyncCallback] immediately: true },
  hostname: 'google.com',
  oncomplete: [Function: onresolve],
  ttl: false }
2

On master we check TXT_HOST instead, which is set to:

TXT_HOST: 'nodejs.org',

@gibfahn
Copy link
Member Author

gibfahn commented Feb 27, 2018

Basically we need to backport #16390 to v6.x.

@joyeecheung is that something you'd be willing to do?

@joyeecheung
Copy link
Member

@gibfahn I can do that, just have not found the time recently. I'll try to backport #16390 and #18547 (actual fix to the the TXT record test) this week

@joyeecheung joyeecheung assigned hiroppy and joyeecheung and unassigned hiroppy Feb 27, 2018
@apapirovski
Copy link
Member

@joyeecheung would you still like to follow up on this?

@joyeecheung
Copy link
Member

@apapirovski Probably not going to need it now that 6.x is near end of life.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

No branches or pull requests

4 participants