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.

Sign up for GitHub

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

Jump to bottom

test: refactor domain tests #13480

Closed
wants to merge 2 commits into from
Closed

test: refactor domain tests #13480

wants to merge 2 commits into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Jun 5, 2017

  • Check that noop callback is or isn't invoked as appropriate using
    common.mustCall() and common.mustNotCall()
  • Fix typo in array literal
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test domain

@Trott
test: refactor domain tests
cbba7dc 
* Check that noop callback is or isn't invoked as appropriate using
  common.mustCall() and common.mustNotCall()
* Fix typo in array literal
@Trott Trott added domain Issues and PRs related to the domain subsystem. test Issues and PRs related to the tests. labels Jun 5, 2017
refack
refack suggested changes Jun 5, 2017
View reviewed changes
test/parallel/test-domain.js
@@ -259,7 +259,7 @@ const fst = fs.createReadStream('stream for nonexistent file');
d.add(fst);
expectCaught++;

[42, null, , false, common.noop, 'string'].forEach(function(something) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it was meant to add undefined?
Also re: #12822 /s/common.noop/() => {}/

Copy link
Member Author

@Trott Trott Jun 5, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack, done, thanks!

mhdawson
mhdawson approved these changes Jun 6, 2017
View reviewed changes
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Trott
Copy link
Member Author

Trott commented Jun 7, 2017

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

Trott added a commit to Trott/io.js that referenced this pull request Jun 8, 2017
@Trott
test: refactor domain tests
b4da30f 
* Check that noop callback is or isn't invoked as appropriate using
  common.mustCall() and common.mustNotCall()
* Fix typo in array literal

PR-URL: nodejs#13480
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@Trott
Copy link
Member Author

Trott commented Jun 8, 2017

Landed in b4da30f

@Trott Trott closed this Jun 8, 2017
addaleax pushed a commit that referenced this pull request Jun 10, 2017
@Trott @addaleax
test: refactor domain tests
ad07c46 
* Check that noop callback is or isn't invoked as appropriate using
  common.mustCall() and common.mustNotCall()
* Fix typo in array literal

PR-URL: #13480
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@addaleax addaleax mentioned this pull request Jun 10, 2017
Merged
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
Closed
3 tasks
MylesBorins pushed a commit that referenced this pull request Aug 14, 2017
@Trott @MylesBorins
test: refactor domain tests
1a95a51 
* Check that noop callback is or isn't invoked as appropriate using
  common.mustCall() and common.mustNotCall()
* Fix typo in array literal

PR-URL: #13480
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MylesBorins pushed a commit that referenced this pull request Aug 16, 2017
@Trott @MylesBorins
test: refactor domain tests
3bdf7bf 
* Check that noop callback is or isn't invoked as appropriate using
  common.mustCall() and common.mustNotCall()
* Fix typo in array literal

PR-URL: #13480
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Aug 16, 2017
Merged
This was referenced Sep 5, 2017
Closed
Closed
Closed
Closed
Closed
@Trott Trott deleted the seinfeld-joke-goes-here branch January 13, 2022 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mhdawson mhdawson mhdawson approved these changes

@cjihrig cjihrig cjihrig approved these changes

@lpinca lpinca lpinca approved these changes

@jasnell jasnell jasnell approved these changes

@refack refack refack approved these changes

Assignees
No one assigned
Labels
domain Issues and PRs related to the domain subsystem. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@Trott @refack @jasnell @lpinca @cjihrig @mhdawson @MylesBorins