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: refactor events tests for invalid listeners #32769

Conversation

edsadr
Copy link
Member

@edsadr edsadr commented Apr 10, 2020

This PR refactor some duplicated assertions in different event-emitter tests to a single one

  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Apr 10, 2020
@nodejs-github-bot
Copy link
Collaborator

@richardlau
Copy link
Member

How easy will it be in the event of failure to determine which event was the cause?

@edsadr edsadr force-pushed the edsadr/test-event-emitter-invalid-listener branch from 1ae50b5 to 13e3442 Compare April 11, 2020 18:16
@edsadr
Copy link
Member Author

edsadr commented Apr 11, 2020

great catch @richardlau, just added a custom message per assertion revealing the method being tested, so if one fails you will have clarity which one was

@edsadr edsadr requested a review from richardlau April 11, 2020 18:18
@nodejs-github-bot
Copy link
Collaborator

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 14, 2020
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

addaleax pushed a commit that referenced this pull request Apr 24, 2020
PR-URL: #32769
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
@addaleax
Copy link
Member

Landed in cea853c 💙

@addaleax addaleax closed this Apr 24, 2020
@edsadr edsadr deleted the edsadr/test-event-emitter-invalid-listener branch April 25, 2020 21:34
BethGriggs pushed a commit that referenced this pull request Apr 27, 2020
PR-URL: #32769
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
@BethGriggs BethGriggs mentioned this pull request Apr 27, 2020
BridgeAR pushed a commit that referenced this pull request Apr 28, 2020
PR-URL: #32769
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
BridgeAR pushed a commit that referenced this pull request Apr 28, 2020
PR-URL: #32769
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
@BridgeAR BridgeAR mentioned this pull request Apr 28, 2020
BethGriggs pushed a commit that referenced this pull request Apr 28, 2020
PR-URL: #32769
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
targos pushed a commit that referenced this pull request Apr 30, 2020
PR-URL: #32769
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
targos pushed a commit that referenced this pull request May 13, 2020
PR-URL: #32769
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants