Skip to content

Conversation

@edsadr
Copy link
Member

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-botnodejs-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?

@edsadredsadrforce-pushed the edsadr/test-event-emitter-invalid-listener branch from 1ae50b5 to 13e3442CompareApril 11, 2020 18:16
@edsadr
Copy link
MemberAuthor

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

@edsadredsadr requested a review from richardlauApril 11, 2020 18:18
@nodejs-github-bot
Copy link
Collaborator

@addaleaxaddaleax 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 💙

@edsadredsadr 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]>
@BethGriggsBethGriggs 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]>
@BridgeARBridgeAR 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]>
@targostargos mentioned this pull request May 2, 2020
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 freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@edsadr@nodejs-github-bot@richardlau@addaleax@gireeshpunathil@Flarna