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

refactor(jest-worker)!: allow only absolute workerPath #12343

Merged
merged 2 commits into from Feb 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Expand Up @@ -2,9 +2,10 @@

### Features

- `[jest-environment-jsdom]` [**BREAKING**] Add default `browser` condtion to `exportConditions` for `jsdom` environment ([#11924](https://github.com/facebook/jest/pull/11924))
- `[jest-environment-jsdom]` [**BREAKING**] Add default `browser` condition to `exportConditions` for `jsdom` environment ([#11924](https://github.com/facebook/jest/pull/11924))
- `[jest-environment-node]` [**BREAKING**] Add default `node` and `node-addon` conditions to `exportConditions` for `node` environment ([#11924](https://github.com/facebook/jest/pull/11924))
- `[@jest/expect-utils]` New module exporting utils for `expect` ([#12323](https://github.com/facebook/jest/pull/12323))
- `[jest-worker]` [**BREAKING**] Allow only absolute `workerPath` ([#12343](https://github.com/facebook/jest/pull/12343))

### Fixes

Expand Down
2 changes: 1 addition & 1 deletion packages/jest-worker/README.md
Expand Up @@ -2,7 +2,7 @@

Module for executing heavy tasks under forked processes in parallel, by providing a `Promise` based interface, minimum overhead, and bound workers.

The module works by providing an absolute path of the module to be loaded in all forked processes. Files relative to a node module are also accepted. All methods are exposed on the parent process as promises, so they can be `await`'ed. Child (worker) methods can either be synchronous or asynchronous.
The module works by providing an absolute path of the module to be loaded in all forked processes. All methods are exposed on the parent process as promises, so they can be `await`'ed. Child (worker) methods can either be synchronous or asynchronous.

The module also implements support for bound workers. Binding a worker means that, based on certain parameters, the same task will always be executed by the same worker. The way bound workers work is by using the returned string of the `computeWorkerKey` method. If the string was used before for a task, the call will be queued to the related worker that processed the task earlier; if not, it will be executed by the first available worker, then sticked to the worker that executed it; so the next time it will be processed by the same worker. If you have no preference on the worker executing the task, but you have defined a `computeWorkerKey` method because you want _some_ of the tasks to be sticked, you can return `null` from it.

Expand Down
7 changes: 7 additions & 0 deletions packages/jest-worker/src/__tests__/index.test.js
Expand Up @@ -61,6 +61,13 @@ afterEach(() => {
jest.resetModules();
});

it('makes a non-existing relative worker throw', () => {
expect(() => {
// eslint-disable-next-line no-new
new Farm('./relative/worker-module.js');
}).toThrow("'workerPath' must be absolute");
});

it('exposes the right API using default working', () => {
const farm = new Farm('/tmp/baz.js', {
exposedMethods: ['foo', 'bar'],
Expand Down
5 changes: 0 additions & 5 deletions packages/jest-worker/src/base/BaseWorkerPool.ts
Expand Up @@ -5,7 +5,6 @@
* LICENSE file in the root directory of this source tree.
*/

import * as path from 'path';
import mergeStream = require('merge-stream');
import {
CHILD_MESSAGE_END,
Expand All @@ -32,10 +31,6 @@ export default class BaseWorkerPool {
this._options = options;
this._workers = new Array(options.numWorkers);

if (!path.isAbsolute(workerPath)) {
workerPath = require.resolve(workerPath);
}

const stdout = mergeStream();
const stderr = mergeStream();

Expand Down
10 changes: 0 additions & 10 deletions packages/jest-worker/src/base/__tests__/BaseWorkerPool.test.js
Expand Up @@ -102,16 +102,6 @@ describe('BaseWorkerPool', () => {
});
});

it('makes a non-existing relative worker throw', () => {
expect(() => {
// eslint-disable-next-line no-new
new MockWorkerPool('./baz.js', {
exposedMethods: [],
numWorkers: 1,
});
}).toThrow();
});

it('create multiple workers with unique worker ids', () => {
// eslint-disable-next-line no-new
new MockWorkerPool('/tmp/baz.js', {
Expand Down
5 changes: 5 additions & 0 deletions packages/jest-worker/src/index.ts
Expand Up @@ -6,6 +6,7 @@
*/

import {cpus} from 'os';
import {isAbsolute} from 'path';
import Farm from './Farm';
import WorkerPool from './WorkerPool';
import type {
Expand Down Expand Up @@ -79,6 +80,10 @@ export class Worker {
this._options = {...options};
this._ending = false;

if (!isAbsolute(workerPath)) {
throw new Error(`'workerPath' must be absolute, got '${workerPath}'`);
}

const workerPoolOptions: WorkerPoolOptions = {
enableWorkerThreads: this._options.enableWorkerThreads ?? false,
forkOptions: this._options.forkOptions ?? {},
Expand Down