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

test: exercise once() with varying arguments #13524

Merged
merged 1 commit into from
Jun 9, 2017
Merged

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Jun 7, 2017

This commit regains test coverage for EventEmitter#once() with four or more arguments. To avoid similar regressions in the future, once() is called with enough arguments to cover all of the separate code paths.

Coverage appears to have been lost here, and shows up on this coverage report.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

test

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Jun 7, 2017
@mscdex
Copy link
Contributor

mscdex commented Jun 7, 2017

The PR link is invalid, perhaps it was meant to be a commit link?

@mscdex mscdex added the events Issues and PRs related to the events subsystem / EventEmitter. label Jun 7, 2017
@cjihrig
Copy link
Contributor Author

cjihrig commented Jun 7, 2017

Fixed. Thanks

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

@cjihrig
Copy link
Contributor Author

cjihrig commented Jun 8, 2017

This commit regains test coverage for EventEmitter#once() with
four or more arguments. To avoid similar regressions in the
future, once() is called with enough arguments to cover all of
the separate code paths.

PR-URL: nodejs#13524
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@cjihrig cjihrig merged commit 637819b into nodejs:master Jun 9, 2017
@cjihrig cjihrig deleted the ee-once branch June 9, 2017 19:04
addaleax pushed a commit that referenced this pull request Jun 10, 2017
This commit regains test coverage for EventEmitter#once() with
four or more arguments. To avoid similar regressions in the
future, once() is called with enough arguments to cover all of
the separate code paths.

PR-URL: #13524
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@addaleax addaleax mentioned this pull request Jun 10, 2017
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
3 tasks
MylesBorins pushed a commit that referenced this pull request Jul 17, 2017
This commit regains test coverage for EventEmitter#once() with
four or more arguments. To avoid similar regressions in the
future, once() is called with enough arguments to cover all of
the separate code paths.

PR-URL: #13524
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jul 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
events Issues and PRs related to the events subsystem / EventEmitter. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants