Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions packages/jest-runner/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@
"jest-message-util": "^21.2.1",
"jest-runtime": "^21.2.1",
"jest-util": "^21.2.1",
"pify": "^3.0.0",
"throat": "^4.0.0",
"worker-farm": "^1.3.1"
"jest-worker": "^21.2.1",
"throat": "^4.0.0"
}
}
36 changes: 14 additions & 22 deletions packages/jest-runner/src/__tests__/test_runner.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,17 @@
const TestRunner = require('../index');
const {TestWatcher} = require('jest-cli');

let workerFarmMock;
let mockWorkerFarm;

jest.mock('worker-farm', () => {
const mock = jest.fn(
(options, worker) =>
(workerFarmMock = jest.fn((data, callback) =>
require(worker)(data, callback),
)),
);
mock.end = jest.fn();
return mock;
jest.mock('jest-worker', () => {
return jest.fn(worker => {
return (mockWorkerFarm = {
default: jest.fn((data, callback) => require(worker)(data, callback)),
end: jest.fn(),
getStderr: jest.fn(),
getStdout: jest.fn(),
});
});
});

jest.mock('../test_worker', () => {});
Expand All @@ -44,15 +44,9 @@ test('injects the rawModuleMap into each worker in watch mode', () => {
{serial: false},
)
.then(() => {
expect(workerFarmMock.mock.calls).toEqual([
[
{config, globalConfig, path: './file.test.js', rawModuleMap},
expect.any(Function),
],
[
{config, globalConfig, path: './file2.test.js', rawModuleMap},
expect.any(Function),
],
expect(mockWorkerFarm.default.mock.calls).toEqual([
[{config, globalConfig, path: './file.test.js', rawModuleMap}],
[{config, globalConfig, path: './file2.test.js', rawModuleMap}],
]);
});
});
Expand All @@ -72,15 +66,14 @@ test('does not inject the rawModuleMap in serial mode', () => {
{serial: false},
)
.then(() => {
expect(workerFarmMock.mock.calls).toEqual([
expect(mockWorkerFarm.default.mock.calls).toEqual([
[
{
config,
globalConfig,
path: './file.test.js',
rawModuleMap: null,
},
expect.any(Function),
],
[
{
Expand All @@ -89,7 +82,6 @@ test('does not inject the rawModuleMap in serial mode', () => {
path: './file2.test.js',
rawModuleMap: null,
},
expect.any(Function),
],
]);
});
Expand Down
33 changes: 18 additions & 15 deletions packages/jest-runner/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/

import type {GlobalConfig} from 'types/Config';
import type {TestResult} from 'types/TestResult';
import type {
OnTestFailure,
OnTestStart,
Expand All @@ -17,13 +18,18 @@ import type {
TestWatcher,
} from 'types/TestRunner';

import pify from 'pify';
import type {WorkerData} from './test_worker';

import runTest from './run_test';
import throat from 'throat';
import workerFarm from 'worker-farm';
import Worker from 'jest-worker';

const TEST_WORKER_PATH = require.resolve('./test_worker');

type CoverageWorker = Worker & {
default: (WorkerData) => Promise<TestResult>,
};

class TestRunner {
_globalConfig: GlobalConfig;

Expand Down Expand Up @@ -89,17 +95,12 @@ class TestRunner {
onResult: OnTestSuccess,
onFailure: OnTestFailure,
) {
const farm = workerFarm(
{
autoStart: true,
maxConcurrentCallsPerWorker: 1,
maxConcurrentWorkers: this._globalConfig.maxWorkers,
maxRetries: 2, // Allow for a couple of transient errors.
},
TEST_WORKER_PATH,
);
// $FlowFixMe: "default" property will be exposed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be cool if default (as long as __esModule: true) would be the actual default export instead of attached to the default key (so you could just do worker(stuff)).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have been thinking about this, and I'm happy to see feedback on that direction. The reason of the default comes from the fact that a class instance is returned from new. The change here would be to make constructor return a method, but then instanceof JestWorker wouldn't work.

I will think more about it and see what's the best way of fixing the issue; but generally speaking, I agree with your comment.

const worker: CoverageWorker = new Worker(TEST_WORKER_PATH, {
numWorkers: this._globalConfig.maxWorkers,
});

const mutex = throat(this._globalConfig.maxWorkers);
const worker = pify(farm);

// Send test suites to workers continuously instead of all at once to track
// the start time of individual tests.
Expand All @@ -108,14 +109,16 @@ class TestRunner {
if (watcher.isInterrupted()) {
return Promise.reject();
}

await onStart(test);
return worker({

return worker.default({
config: test.context.config,
globalConfig: this._globalConfig,
path: test.path,
rawModuleMap: watcher.isWatchMode()
? test.context.moduleMap.getRawModuleMap()
: null,
: undefined,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is causing the single test failure

});
});

Expand Down Expand Up @@ -146,7 +149,7 @@ class TestRunner {
),
);

const cleanup = () => workerFarm.end(farm);
const cleanup = () => worker.end();
return Promise.race([runAllTests, onInterrupt]).then(cleanup, cleanup);
}
}
Expand Down
43 changes: 14 additions & 29 deletions packages/jest-runner/src/test_worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,13 @@ import {separateMessageFromStack} from 'jest-message-util';
import Runtime from 'jest-runtime';
import runTest from './run_test';

type WorkerData = {|
export type WorkerData = {|
config: ProjectConfig,
globalConfig: GlobalConfig,
path: Path,
rawModuleMap?: RawModuleMap,
|};

type WorkerCallback = (error: ?SerializableError, result?: TestResult) => void;

const formatError = (error: string | Error): SerializableError => {
if (typeof error === 'string') {
const {message, stack} = separateMessageFromStack(error);
Expand Down Expand Up @@ -69,33 +67,20 @@ const getResolver = (config, rawModuleMap) => {
}
};

// Cannot be ESM export because of worker-farm
module.exports = (
{config, globalConfig, path, rawModuleMap}: WorkerData,
callback: WorkerCallback,
) => {
let parentExited = false;
const disconnectCallback = () => (parentExited = true);
const removeListener = () =>
process.removeListener('disconnect', disconnectCallback);
process.on('disconnect', disconnectCallback);

export default async function({
config,
globalConfig,
path,
rawModuleMap,
}: WorkerData): Promise<TestResult> {
try {
runTest(path, globalConfig, config, getResolver(config, rawModuleMap)).then(
result => {
removeListener();
if (!parentExited) {
callback(null, result);
}
},
error => {
removeListener();
if (!parentExited) {
callback(formatError(error));
}
},
return await runTest(
path,
globalConfig,
config,
getResolver(config, rawModuleMap),
);
} catch (error) {
callback(formatError(error));
} catch (err) {
throw formatError(err);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any particular reason formatError return something not an actual error? I realise that is not new in this PR, it just struck me as odd

}
};
2 changes: 1 addition & 1 deletion packages/jest-worker/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "jest-worker",
"version": "21.1.0",
"version": "21.2.1",
"repository": {
"type": "git",
"url": "https://github.com/facebook/jest.git"
Expand Down