Skip to content

test: refactor test-dns-regress-6244.js - #13058

Closed
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:6244-refactor
Closed

test: refactor test-dns-regress-6244.js#13058
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:6244-refactor

Conversation

@Trott

Copy link
Copy Markdown
Member
  • Move from parallel to internet because it performs a DNS query
  • Provide link to relevant issue in comments
  • Remove unnecessary explicit no-op function
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test dns

* Move from parallel to internet because it performs a DNS query
* Provide link to relevant issue in comments
* Remove unnecessary explicit no-op function
@TrottTrott added dns Issues and PRs related to the dns subsystem. test Issues and PRs related to the tests. labels May 16, 2017
@Trott

Copy link
Copy Markdown
MemberAuthor

@nodejs/testing

@gibfahn

Copy link
Copy Markdown
Member

My only issue with this would be that the internet tests don't get run very much, so moving to internet basically means ignoring. Of course if there's no feasible way to do this offline then we have no choice.

@Trott

Copy link
Copy Markdown
MemberAuthor

My only issue with this would be that the internet tests don't get run very much, so moving to internet basically means ignoring.

@gibfahn Discussion to come up with a solution to that at #13061

@addaleax

Copy link
Copy Markdown
Member

Landed in 47e3d00

Trott added a commit to Trott/io.js that referenced this pull request May 18, 2017
* Move from parallel to internet because it performs a DNS query
* Provide link to relevant issue in comments
* Remove unnecessary explicit no-op function
PR-URL: nodejs#13058
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
anchnk pushed a commit to anchnk/node that referenced this pull request May 19, 2017
* Move from parallel to internet because it performs a DNS query
* Provide link to relevant issue in comments
* Remove unnecessary explicit no-op function
PR-URL: nodejs#13058
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@jasnelljasnell mentioned this pull request May 28, 2017
@gibfahngibfahn mentioned this pull request Jun 15, 2017
3 tasks
@MylesBorins

Copy link
Copy Markdown
Contributor

This test does not pass on v6.x

Please feel free to backport

@Trott

Copy link
Copy Markdown
MemberAuthor

This test does not pass on v6.x

@MylesBorins Are you sure? That's strange because this moves the test from parallel to internet which means make test and make test-ci now no longer run the test. The only thing this would need to pass is linting.

@gibfahn

Copy link
Copy Markdown
Member

@Trott when backporting we normally run any tests that were changed, not just the ones that make test tests.

This fails as follows:

nodegit:(v6.x-staging)tools/test.pytest/internet/test-dns-regress-6244.js~/wrk/com/DANGER/node===releasetest-dns-regress-6244===Path: internet/test-dns-regress-6244/Users/gib/wrk/com/DANGER/node/test/common/index.js:431
name: fn.name||'<anonymous>'^TypeError: Cannotreadproperty'name'ofundefinedatObject.exports.mustCall(/Users/gib/wrk/com/DANGER/node/test/common/index.js:431:13)atObject.<anonymous>(/Users/gib/wrk/com/DANGER/node/test/internet/test-dns-regress-6244.js:6:34)atModule._compile(module.js:570:32)atObject.Module._extensions..js(module.js:579:10)atModule.load(module.js:487:32)attryModuleLoad(module.js:446:12)atFunction.Module._load(module.js:438:3)atModule.runMain(module.js:604:10)atrun(bootstrap_node.js:389:7)atstartup(bootstrap_node.js:149:9)
Command: out/Release/node/Users/gib/wrk/com/DANGER/node/test/internet/test-dns-regress-6244.js[00:00|%100|+0|-1]: Done

Given that the only code change in the pr is common.mustCall( () => {} ) -> common.mustCall(), I'm thinking v6.x must be missing the PR to allow mustCall to take no parameters.

@Trott
Trott deleted the 6244-refactor branch January 13, 2022 22:45
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dnsIssues and PRs related to the dns subsystem.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@Trott@gibfahn@addaleax@MylesBorins@santigimeno@lpinca@cjihrig