Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

test: check number of message events #13125

Closed
wants to merge 1 commit into from
Closed

Conversation

Trott
Copy link
Member

@Trott Trott commented May 20, 2017

When a no-op message event handler is used in a test, make it clear what
is expected by using common.mustCall() and common.mustNotCall().

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test child_process

When a no-op message event handler is used in a test, make it clear what
is expected by using `common.mustCall()` and `common.mustNotCall()`.
@Trott Trott added child_process Issues and PRs related to the child_process subsystem. test Issues and PRs related to the tests. labels May 20, 2017
Copy link
Contributor

@refack refack left a comment

Choose a reason for hiding this comment

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

LGTM if CI is green

@@ -45,7 +45,7 @@ if (process.argv[2] !== 'child') {
// the only thing keeping this worker alive will be IPC. This is important,
// because it means a worker with no parent will have no referenced handles,
// thus no work to do, and will exit immediately, preventing process leaks.
process.on('message', common.noop);
process.on('message', common.mustCall());
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this an exact 1 or atLeast 1?

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems to be exactly 1 to me. This is run by the 80 (?!) child processes. The parent process sends each of them exactly 1 message.

@Trott
Copy link
Member Author

Trott commented May 20, 2017

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

Happy to see these tests being tightened up :-)

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

@mhdawson
Copy link
Member

@addaleax
Copy link
Member

LGTM

Landed in b5b6f5d

@addaleax addaleax closed this May 23, 2017
addaleax pushed a commit that referenced this pull request May 23, 2017
When a no-op message event handler is used in a test, make it clear what
is expected by using `common.mustCall()` and `common.mustNotCall()`.

PR-URL: #13125
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <mhdawson@ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
jasnell pushed a commit that referenced this pull request May 24, 2017
When a no-op message event handler is used in a test, make it clear what
is expected by using `common.mustCall()` and `common.mustNotCall()`.

PR-URL: #13125
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <mhdawson@ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
jasnell pushed a commit that referenced this pull request May 28, 2017
When a no-op message event handler is used in a test, make it clear what
is expected by using `common.mustCall()` and `common.mustNotCall()`.

PR-URL: #13125
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <mhdawson@ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@jasnell jasnell mentioned this pull request May 28, 2017
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
3 tasks
@MylesBorins
Copy link
Contributor

Seeing failures in this one... it would appear that common.mustCall on v6.x does not work if called without a function being provided. Is there a PR that introduced that functionality?

@refack
Copy link
Contributor

refack commented Jul 17, 2017

Seeing failures in this one... it would appear that common.mustCall on v6.x does not work if called without a function being provided. Is there a PR that introduced that functionality?

#12027

MylesBorins pushed a commit that referenced this pull request Aug 14, 2017
When a no-op message event handler is used in a test, make it clear what
is expected by using `common.mustCall()` and `common.mustNotCall()`.

PR-URL: #13125
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <mhdawson@ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this pull request Aug 16, 2017
When a no-op message event handler is used in a test, make it clear what
is expected by using `common.mustCall()` and `common.mustNotCall()`.

PR-URL: #13125
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <mhdawson@ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Aug 16, 2017
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
child_process Issues and PRs related to the child_process subsystem. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants