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,tools: use .then(common.mustCall()) for all async IIFEs + linter rule #34363

Closed
wants to merge 3 commits into from
Closed

test,tools: use .then(common.mustCall()) for all async IIFEs + linter rule #34363

wants to merge 3 commits into from

Conversation

addaleax
Copy link
Member

test: use .then(common.mustCall()) for all async IIFEs

This makes sure that all async functions finish as expected.

tools: add linting rule for async IIFEs

The result of an async IIFE should always be handled in our tests,
typically by adding .then(common.mustCall()) to verify that the
async function actually finishes executing at some point.

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

@addaleax
test: use .then(common.mustCall()) for all async IIFEs
ee5488d 
This makes sure that all async functions finish as expected.
@addaleax
tools: add linting rule for async IIFEs
1c91794 
The result of an async IIFE should always be handled in our tests,
typically by adding `.then(common.mustCall())` to verify that the
async function actually finishes executing at some point.
@nodejs-github-bot nodejs-github-bot added esm Issues and PRs related to the ECMAScript Modules implementation. test Issues and PRs related to the tests. labels Jul 14, 2020
@addaleax addaleax added tools Issues and PRs related to the tools directory. and removed esm Issues and PRs related to the ECMAScript Modules implementation. labels Jul 14, 2020
@addaleax
Copy link
Member Author

Don’t think there’s a team for eslint stuff, so /cc @cjihrig @Trott I guess?

@nodejs-github-bot
Copy link
Collaborator

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

@cjihrig
Copy link
Contributor

cjihrig commented Jul 14, 2020

Don’t think there’s a team for eslint stuff

There is @nodejs/linting.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 14, 2020 •
edited by addaleax
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/32344/ (:green_heart:)

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 15, 2020
addaleax added a commit that referenced this pull request Jul 20, 2020
@addaleax
test: use .then(common.mustCall()) for all async IIFEs
2c4ebe0 
This makes sure that all async functions finish as expected.

PR-URL: #34363
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
addaleax added a commit that referenced this pull request Jul 20, 2020
@addaleax
tools: add linting rule for async IIFEs
77b68f9 
The result of an async IIFE should always be handled in our tests,
typically by adding `.then(common.mustCall())` to verify that the
async function actually finishes executing at some point.

PR-URL: #34363
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@addaleax
Copy link
Member Author

Landed in da95dd7...77b68f9

@addaleax addaleax closed this Jul 20, 2020
@addaleax addaleax deleted the async-iife-fix branch July 20, 2020 16:15
@ruyadorno
Copy link
Member

This don't land cleanly on v14.x should it be backported?

cjihrig pushed a commit that referenced this pull request Jul 23, 2020
@addaleax @cjihrig
test: use .then(common.mustCall()) for all async IIFEs
fd0096d 
This makes sure that all async functions finish as expected.

PR-URL: #34363
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
cjihrig pushed a commit that referenced this pull request Jul 23, 2020
@addaleax @cjihrig
tools: add linting rule for async IIFEs
f5363e4 
The result of an async IIFE should always be handled in our tests,
typically by adding `.then(common.mustCall())` to verify that the
async function actually finishes executing at some point.

PR-URL: #34363
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@targos targos added backported-to-v14.x and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. backport-requested-v14.x labels May 16, 2021
targos pushed a commit that referenced this pull request May 16, 2021
@addaleax @targos
test: use .then(common.mustCall()) for all async IIFEs
845f3ae 
This makes sure that all async functions finish as expected.

PR-URL: #34363
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
targos pushed a commit that referenced this pull request May 16, 2021
@addaleax @targos
tools: add linting rule for async IIFEs
02233af 
The result of an async IIFE should always be handled in our tests,
typically by adding `.then(common.mustCall())` to verify that the
async function actually finishes executing at some point.

PR-URL: #34363
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@targos targos mentioned this pull request Jun 6, 2021
Merged
targos pushed a commit that referenced this pull request Jun 11, 2021
@addaleax @targos
test: use .then(common.mustCall()) for all async IIFEs
89ee6ab 
This makes sure that all async functions finish as expected.

PR-URL: #34363
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
targos pushed a commit that referenced this pull request Jun 11, 2021
@addaleax @targos
tools: add linting rule for async IIFEs
9fa8d20 
The result of an async IIFE should always be handled in our tests,
typically by adding `.then(common.mustCall())` to verify that the
async function actually finishes executing at some point.

PR-URL: #34363
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@snyk-bot snyk-bot mentioned this pull request Sep 13, 2022
Open
@ron-a20 ron-a20 mentioned this pull request Sep 13, 2022
Open
This was referenced Sep 13, 2022
Open
Open
@wedataintelligence wedataintelligence mentioned this pull request Sep 13, 2022
Open
@snyk-bot snyk-bot mentioned this pull request Sep 13, 2022
Open
@molllyn1 molllyn1 mentioned this pull request Sep 14, 2022
Open
@137717unity 137717unity mentioned this pull request Sep 14, 2022
Open
@snyk-bot snyk-bot mentioned this pull request Sep 25, 2022
Open
@wedataintelligence wedataintelligence mentioned this pull request Sep 26, 2022
Open
@137717unity 137717unity mentioned this pull request Sep 26, 2022
Open
@snyk-bot snyk-bot mentioned this pull request Sep 27, 2022
Open
@wedataintelligence wedataintelligence mentioned this pull request Sep 29, 2022
Open
This was referenced Sep 29, 2022
Open
Open
This was referenced Oct 5, 2022
Open
Open
@snyk-bot snyk-bot mentioned this pull request Oct 6, 2022
Open
@137717unity 137717unity mentioned this pull request Oct 7, 2022
Open
@wedataintelligence wedataintelligence mentioned this pull request Oct 7, 2022
Open
This was referenced Oct 8, 2022
Open
Open
@137717unity 137717unity mentioned this pull request Oct 9, 2022
Open
@snyk-bot snyk-bot mentioned this pull request Oct 9, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richardlau richardlau richardlau approved these changes

@tniessen tniessen tniessen approved these changes

@cjihrig cjihrig cjihrig approved these changes

@lpinca lpinca lpinca approved these changes

@Trott Trott Trott approved these changes

@jasnell jasnell jasnell approved these changes

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

Successfully merging this pull request may close these issues.
10 participants
@addaleax @nodejs-github-bot @cjihrig @ruyadorno @jasnell @Trott @lpinca @tniessen @richardlau @targos