From 909fb5400a23675cfa7f5a9299ffbb97ebfa1486 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Wed, 23 Jan 2019 00:37:02 +0100 Subject: [PATCH 1/2] fix: add option to disable `worker_threads` in `jest-worker` --- CHANGELOG.md | 2 ++ .../src/reporters/coverage_reporter.js | 1 + packages/jest-haste-map/src/index.js | 1 + packages/jest-runner/src/index.js | 1 + packages/jest-worker/README.md | 6 ++++ packages/jest-worker/src/WorkerPool.js | 2 +- .../src/__tests__/WorkerPool.test.js | 29 +++++++++++++++++++ packages/jest-worker/src/index.js | 1 + packages/jest-worker/src/types.js | 2 ++ 9 files changed, 44 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11e6c5e964a5..1e32366f8b43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,7 @@ - `[jest-runner]` Instantiate the test environment class with the current `testPath` ([#7442](https://github.com/facebook/jest/pull/7442)) - `[jest-config]` Always resolve jest-environment-jsdom from jest-config ([#7476](https://github.com/facebook/jest/pull/7476)) - `[expect]` Improve report when assertion fails, part 6 ([#7621](https://github.com/facebook/jest/pull/7621)) +- `[jest-worker]` Add `disableWorkerThreads` option to explicitly opt out of `worker_threads` even if available ### Fixes @@ -129,6 +130,7 @@ - `[jest-util]` [**BREAKING**] Remove long-deprecated globals for fake timers ([#7285](https://github.com/facebook/jest/pull/7285)) - `[*]` [**BREAKING**] Upgrade to Micromatch 3 ([#6650](https://github.com/facebook/jest/pull/6650)) - `[*]` [**BREAKING**] Remove regenerator-runtime injection ([#7595](https://github.com/facebook/jest/pull/7595)) +- `[jest-worker]` Disable `worker_threads` to avoid issues with libraries to ready for it - `[docs]` Fix message property in custom matcher example to return a function instead of a constant. ([#7426](https://github.com/facebook/jest/pull/7426)) - `[jest-circus]` Standardize file naming in `jest-circus` ([#7301](https://github.com/facebook/jest/pull/7301)) - `[docs]` Add synchronous test.each setup ([#7150](https://github.com/facebook/jest/pull/7150)) diff --git a/packages/jest-cli/src/reporters/coverage_reporter.js b/packages/jest-cli/src/reporters/coverage_reporter.js index 81e45ec08a1e..ba9dccf0cca7 100644 --- a/packages/jest-cli/src/reporters/coverage_reporter.js +++ b/packages/jest-cli/src/reporters/coverage_reporter.js @@ -155,6 +155,7 @@ export default class CoverageReporter extends BaseReporter { } else { // $FlowFixMe: assignment of a worker with custom properties. worker = new Worker(require.resolve('./coverage_worker'), { + disableWorkerThreads: true, exposedMethods: ['worker'], maxRetries: 2, numWorkers: this._globalConfig.maxWorkers, diff --git a/packages/jest-haste-map/src/index.js b/packages/jest-haste-map/src/index.js index c7c6a79ac841..a16457fabe32 100644 --- a/packages/jest-haste-map/src/index.js +++ b/packages/jest-haste-map/src/index.js @@ -672,6 +672,7 @@ class HasteMap extends EventEmitter { } else { // $FlowFixMe: assignment of a worker with custom properties. this._worker = (new Worker(require.resolve('./worker'), { + disableWorkerThreads: true, exposedMethods: ['getSha1', 'worker'], maxRetries: 3, numWorkers: this._options.maxWorkers, diff --git a/packages/jest-runner/src/index.js b/packages/jest-runner/src/index.js index ca4ccc141895..bbb6abb026a3 100644 --- a/packages/jest-runner/src/index.js +++ b/packages/jest-runner/src/index.js @@ -96,6 +96,7 @@ class TestRunner { ) { // $FlowFixMe: class object is augmented with worker when instantiating. const worker: WorkerInterface = new Worker(TEST_WORKER_PATH, { + disableWorkerThreads: true, exposedMethods: ['worker'], forkOptions: {stdio: 'pipe'}, maxRetries: 3, diff --git a/packages/jest-worker/README.md b/packages/jest-worker/README.md index 09e17904242a..37394a9b92a3 100644 --- a/packages/jest-worker/README.md +++ b/packages/jest-worker/README.md @@ -45,6 +45,8 @@ Node 10 shipped with [worker-threads](https://nodejs.org/api/worker_threads.html We will use worker threads where available. To enable in Node 10+, run the Node process with the `--experimental-worker` flag. +You can explicitly opt-out of this by passing `disableWorkerThreads: true`. + ## API The only exposed method is a constructor (`Worker`) that is initialized by passing the worker path, plus an options object. @@ -89,6 +91,10 @@ Provide a custom worker pool to be used for spawning child processes. By default The arguments that will be passed to the `setup` method during initialization. +#### `disableWorkerThreads: boolean` (optional) + +`jest-worker` will automatically detect if `worker_threads` are available and use them. However, running under threads comes with [some caveats](https://nodejs.org/api/worker_threads.html#worker_threads_class_worker), and is still experimental, so you can `opt-out` of this and use `disableWorkerThreads: true`. + ## Worker The returned `Worker` instance has all the exposed methods, plus some additional ones to interact with the workers itself: diff --git a/packages/jest-worker/src/WorkerPool.js b/packages/jest-worker/src/WorkerPool.js index 9827c0ff85d7..0d4491bfd456 100644 --- a/packages/jest-worker/src/WorkerPool.js +++ b/packages/jest-worker/src/WorkerPool.js @@ -42,7 +42,7 @@ class WorkerPool extends BaseWorkerPool implements WorkerPoolInterface { createWorker(workerOptions: WorkerOptions): WorkerInterface { let Worker; - if (canUseWorkerThreads()) { + if (!this._options.disableWorkerThreads && canUseWorkerThreads()) { Worker = require('./workers/NodeThreadsWorker').default; } else { Worker = require('./workers/ChildProcessWorker').default; diff --git a/packages/jest-worker/src/__tests__/WorkerPool.test.js b/packages/jest-worker/src/__tests__/WorkerPool.test.js index 71ec4dc29fd9..7d8e5a35eb9a 100644 --- a/packages/jest-worker/src/__tests__/WorkerPool.test.js +++ b/packages/jest-worker/src/__tests__/WorkerPool.test.js @@ -101,4 +101,33 @@ describe('WorkerPool', () => { onEnd, ); }); + + it('should avoid NodeThreadWorker if passed disableWorkerThreads', () => { + jest.mock('worker_threads', () => 'Defined'); + const workerPool = new WorkerPool('/path', { + disableWorkerThreads: true, + forkOptions: {}, + maxRetries: 1, + numWorkers: 1, + workerId: 0, + workerPath: '/path', + }); + + const onStart = () => {}; + const onEnd = () => {}; + workerPool.send(0, {foo: 'bar'}, onStart, onEnd); + + expect(ChildProcessWorker).toBeCalledWith({ + forkOptions: {}, + maxRetries: 1, + workerId: 0, + workerPath: '/path', + }); + expect(NodeThreadWorker).not.toBeCalled(); + expect(workerPool._workers[0].send).toBeCalledWith( + {foo: 'bar'}, + onStart, + onEnd, + ); + }); }); diff --git a/packages/jest-worker/src/index.js b/packages/jest-worker/src/index.js index 0b3c4a40b59b..55775e091f23 100644 --- a/packages/jest-worker/src/index.js +++ b/packages/jest-worker/src/index.js @@ -77,6 +77,7 @@ export default class JestWorker { this._options = {...options}; const workerPoolOptions: WorkerPoolOptions = { + disableWorkerThreads: this._options.disableWorkerThreads || false, forkOptions: this._options.forkOptions || {}, maxRetries: this._options.maxRetries || 3, numWorkers: this._options.numWorkers || Math.max(os.cpus().length - 1, 1), diff --git a/packages/jest-worker/src/types.js b/packages/jest-worker/src/types.js index b0a80f325c77..24fcd7a6f47e 100644 --- a/packages/jest-worker/src/types.js +++ b/packages/jest-worker/src/types.js @@ -70,6 +70,7 @@ export type FarmOptions = { workerPath: string, options?: WorkerPoolOptions, ) => WorkerPoolInterface, + disableWorkerThreads?: boolean, }; export type WorkerPoolOptions = {| @@ -77,6 +78,7 @@ export type WorkerPoolOptions = {| forkOptions: ForkOptions, maxRetries: number, numWorkers: number, + disableWorkerThreads: boolean, |}; export type WorkerOptions = {| From 3a4a41428c56a4c0c47f67e15200ae7f072bf227 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Wed, 23 Jan 2019 00:51:06 +0100 Subject: [PATCH 2/2] link to PR --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e32366f8b43..ea4aa16aaf57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,7 +52,7 @@ - `[jest-runner]` Instantiate the test environment class with the current `testPath` ([#7442](https://github.com/facebook/jest/pull/7442)) - `[jest-config]` Always resolve jest-environment-jsdom from jest-config ([#7476](https://github.com/facebook/jest/pull/7476)) - `[expect]` Improve report when assertion fails, part 6 ([#7621](https://github.com/facebook/jest/pull/7621)) -- `[jest-worker]` Add `disableWorkerThreads` option to explicitly opt out of `worker_threads` even if available +- `[jest-worker]` Add `disableWorkerThreads` option to explicitly opt out of `worker_threads` even if available ([#7681](https://github.com/facebook/jest/pull/7681)) ### Fixes @@ -130,7 +130,7 @@ - `[jest-util]` [**BREAKING**] Remove long-deprecated globals for fake timers ([#7285](https://github.com/facebook/jest/pull/7285)) - `[*]` [**BREAKING**] Upgrade to Micromatch 3 ([#6650](https://github.com/facebook/jest/pull/6650)) - `[*]` [**BREAKING**] Remove regenerator-runtime injection ([#7595](https://github.com/facebook/jest/pull/7595)) -- `[jest-worker]` Disable `worker_threads` to avoid issues with libraries to ready for it +- `[jest-worker]` Disable `worker_threads` to avoid issues with libraries to ready for it ([#7681](https://github.com/facebook/jest/pull/7681)) - `[docs]` Fix message property in custom matcher example to return a function instead of a constant. ([#7426](https://github.com/facebook/jest/pull/7426)) - `[jest-circus]` Standardize file naming in `jest-circus` ([#7301](https://github.com/facebook/jest/pull/7301)) - `[docs]` Add synchronous test.each setup ([#7150](https://github.com/facebook/jest/pull/7150))