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

Closed
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

This makes sure that all async functions finish as expected.
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

@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

@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
This makes sure that all async functions finish as expected.

PR-URL: #34363
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
addaleax added a commit that referenced this pull request Jul 20, 2020
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 <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@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
This makes sure that all async functions finish as expected.

PR-URL: #34363
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
cjihrig pushed a commit that referenced this pull request Jul 23, 2020
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 <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@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
This makes sure that all async functions finish as expected.

PR-URL: #34363
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this pull request May 16, 2021
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 <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this pull request Jun 11, 2021
This makes sure that all async functions finish as expected.

PR-URL: #34363
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this pull request Jun 11, 2021
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 <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants