Skip to content

Commit

Permalink
fix: Switch from throat to p-limit
Browse files Browse the repository at this point in the history
Throat has a significant unacknowledged bug (ForbesLindesay/throat#61) that may cause it to lock up with repeated invocations.

Sindre's p-limit appears to be better supported.  I picked p-limit 3.x (the last version before the switch to pure ESM) to match Jest's usage of other packages from Sindre.

Fixes jestjs#12757
  • Loading branch information
joshkel committed Jun 23, 2022
1 parent fef194f commit 56062ea
Show file tree
Hide file tree
Showing 12 changed files with 30 additions and 29 deletions.
4 changes: 2 additions & 2 deletions e2e/transform/transform-runner/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

import throat from 'throat';
import pLimit from 'p-limit';
import {Test, TestResult, createEmptyTestResult} from '@jest/test-result';
import type {Config} from '@jest/types';
import type {
Expand All @@ -32,7 +32,7 @@ export default class BaseTestRunner {
onResult: OnTestSuccess,
onFailure: OnTestFailure,
): Promise<void> {
const mutex = throat(1);
const mutex = pLimit(1);
return tests.reduce(
(promise, test) =>
mutex(() =>
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
"mock-fs": "^5.1.2",
"netlify-plugin-cache": "^1.0.3",
"node-notifier": "^10.0.0",
"p-limit": "^3.1.0",
"pkg-dir": "^5.0.0",
"prettier": "^2.1.1",
"progress": "^2.0.0",
Expand All @@ -78,7 +79,6 @@
"strip-ansi": "^6.0.0",
"strip-json-comments": "^3.1.1",
"tempy": "^1.0.0",
"throat": "^6.0.1",
"ts-node": "^10.5.0",
"type-fest": "^2.11.2",
"typescript": "^4.7.3",
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-changed-files/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
},
"dependencies": {
"execa": "^5.0.0",
"throat": "^6.0.1"
"p-limit": "^3.1.0"
},
"engines": {
"node": "^12.13.0 || ^14.15.0 || ^16.10.0 || >=17.0.0"
Expand Down
4 changes: 2 additions & 2 deletions packages/jest-changed-files/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*
*/

import throat from 'throat';
import pLimit = require('p-limit');
import git from './git';
import hg from './hg';
import type {ChangedFilesPromise, Options, Repos, SCMAdapter} from './types';
Expand All @@ -21,7 +21,7 @@ function notEmpty<T>(value: T | null | undefined): value is T {

// This is an arbitrary number. The main goal is to prevent projects with
// many roots (50+) from spawning too many processes at once.
const mutex = throat(5);
const mutex = pLimit(5);

const findGitRoot = (dir: string) => mutex(() => git.getRoot(dir));
const findHgRoot = (dir: string) => mutex(() => hg.getRoot(dir));
Expand Down
4 changes: 2 additions & 2 deletions packages/jest-circus/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@
"jest-runtime": "^28.1.1",
"jest-snapshot": "^28.1.1",
"jest-util": "^28.1.1",
"p-limit": "^3.1.0",
"pretty-format": "^28.1.1",
"slash": "^3.0.0",
"stack-utils": "^2.0.3",
"throat": "^6.0.1"
"stack-utils": "^2.0.3"
},
"devDependencies": {
"@babel/core": "^7.11.6",
Expand Down
4 changes: 2 additions & 2 deletions packages/jest-circus/src/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

import throat from 'throat';
import pLimit = require('p-limit');
import type {Circus} from '@jest/types';
import {dispatch, getState} from './state';
import {RETRY_TIMES} from './types';
Expand Down Expand Up @@ -46,7 +46,7 @@ const _runTestsForDescribeBlock = async (

if (isRootBlock) {
const concurrentTests = collectConcurrentTests(describeBlock);
const mutex = throat(getState().maxConcurrency);
const mutex = pLimit(getState().maxConcurrency);
for (const test of concurrentTests) {
try {
const promise = mutex(test.fn);
Expand Down
4 changes: 2 additions & 2 deletions packages/jest-jasmine2/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
"jest-runtime": "^28.1.1",
"jest-snapshot": "^28.1.1",
"jest-util": "^28.1.1",
"pretty-format": "^28.1.1",
"throat": "^6.0.1"
"p-limit": "^3.1.0",
"pretty-format": "^28.1.1"
},
"devDependencies": {
"@types/co": "^4.6.2"
Expand Down
6 changes: 3 additions & 3 deletions packages/jest-jasmine2/src/jasmineAsyncInstall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

import co from 'co';
import isGeneratorFn from 'is-generator-fn';
import throat from 'throat';
import pLimit = require('p-limit');
import type {Config, Global} from '@jest/types';
import isError from './isError';
import type Spec from './jasmine/Spec';
Expand Down Expand Up @@ -192,7 +192,7 @@ function makeConcurrent(
timeout?: number,
) => Spec,
env: Jasmine['currentEnv_'],
mutex: ReturnType<typeof throat>,
mutex: ReturnType<typeof pLimit>,
): Global.ItConcurrentBase {
const concurrentFn = function (
specName: Global.TestNameLike,
Expand Down Expand Up @@ -242,7 +242,7 @@ export default function jasmineAsyncInstall(
global: Global.Global,
): void {
const jasmine = global.jasmine;
const mutex = throat(globalConfig.maxConcurrency);
const mutex = pLimit(globalConfig.maxConcurrency);

const env = jasmine.getEnv();
env.it = promisifyIt(env.it, env, jasmine);
Expand Down
4 changes: 2 additions & 2 deletions packages/jest-runner/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@
"jest-util": "^28.1.1",
"jest-watcher": "^28.1.1",
"jest-worker": "^28.1.1",
"source-map-support": "0.5.13",
"throat": "^6.0.1"
"p-limit": "^3.1.0",
"source-map-support": "0.5.13"
},
"devDependencies": {
"@tsd/typescript": "~4.7.3",
Expand Down
6 changes: 3 additions & 3 deletions packages/jest-runner/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import chalk = require('chalk');
import Emittery = require('emittery');
import throat from 'throat';
import pLimit = require('p-limit');
import type {
Test,
TestEvents,
Expand Down Expand Up @@ -54,7 +54,7 @@ export default class TestRunner extends EmittingTestRunner {

async #createInBandTestRun(tests: Array<Test>, watcher: TestWatcher) {
process.env.JEST_WORKER_ID = '1';
const mutex = throat(1);
const mutex = pLimit(1);
return tests.reduce(
(promise, test) =>
mutex(() =>
Expand Down Expand Up @@ -116,7 +116,7 @@ export default class TestRunner extends EmittingTestRunner {
if (worker.getStdout()) worker.getStdout().pipe(process.stdout);
if (worker.getStderr()) worker.getStderr().pipe(process.stderr);

const mutex = throat(this._globalConfig.maxWorkers);
const mutex = pLimit(this._globalConfig.maxWorkers);

// Send test suites to workers continuously instead of all at once to track
// the start time of individual tests.
Expand Down
7 changes: 4 additions & 3 deletions scripts/buildTs.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import chalk from 'chalk';
import execa from 'execa';
import globby from 'globby';
import fs from 'graceful-fs';
import pLimit from 'p-limit';
import stripJsonComments from 'strip-json-comments';
import throat from 'throat';
import {getPackages} from './buildUtils.mjs';

(async () => {
Expand Down Expand Up @@ -130,9 +130,10 @@ import {getPackages} from './buildUtils.mjs';
const typesNodeReferenceDirective = `${typesReferenceDirective}="node" />`;

try {
const mutex = pLimit(cpus);
await Promise.all(
packagesWithTs.map(
throat(cpus, async ({packageDir, pkg}) => {
packagesWithTs.map(({packageDir, pkg}) =>
mutex(async () => {
const buildDir = path.resolve(packageDir, 'build/**/*.d.ts');

const globbed = await globby([buildDir]);
Expand Down
12 changes: 6 additions & 6 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2722,6 +2722,7 @@ __metadata:
mock-fs: ^5.1.2
netlify-plugin-cache: ^1.0.3
node-notifier: ^10.0.0
p-limit: ^3.1.0
pkg-dir: ^5.0.0
prettier: ^2.1.1
progress: ^2.0.0
Expand All @@ -2735,7 +2736,6 @@ __metadata:
strip-ansi: ^6.0.0
strip-json-comments: ^3.1.1
tempy: ^1.0.0
throat: ^6.0.1
ts-node: ^10.5.0
type-fest: ^2.11.2
typescript: ^4.7.3
Expand Down Expand Up @@ -13111,7 +13111,7 @@ __metadata:
resolution: "jest-changed-files@workspace:packages/jest-changed-files"
dependencies:
execa: ^5.0.0
throat: ^6.0.1
p-limit: ^3.1.0
languageName: unknown
linkType: soft

Expand Down Expand Up @@ -13142,10 +13142,10 @@ __metadata:
jest-runtime: ^28.1.1
jest-snapshot: ^28.1.1
jest-util: ^28.1.1
p-limit: ^3.1.0
pretty-format: ^28.1.1
slash: ^3.0.0
stack-utils: ^2.0.3
throat: ^6.0.1
languageName: unknown
linkType: soft

Expand Down Expand Up @@ -13388,8 +13388,8 @@ __metadata:
jest-runtime: ^28.1.1
jest-snapshot: ^28.1.1
jest-util: ^28.1.1
p-limit: ^3.1.0
pretty-format: ^28.1.1
throat: ^6.0.1
languageName: unknown
linkType: soft

Expand Down Expand Up @@ -13606,8 +13606,8 @@ __metadata:
jest-util: ^28.1.1
jest-watcher: ^28.1.1
jest-worker: ^28.1.1
p-limit: ^3.1.0
source-map-support: 0.5.13
throat: ^6.0.1
tsd-lite: ^0.5.1
languageName: unknown
linkType: soft
Expand Down Expand Up @@ -16930,7 +16930,7 @@ __metadata:
languageName: node
linkType: hard

"p-limit@npm:^3.0.2":
"p-limit@npm:^3.0.2, p-limit@npm:^3.1.0":
version: 3.1.0
resolution: "p-limit@npm:3.1.0"
dependencies:
Expand Down

0 comments on commit 56062ea

Please sign in to comment.