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

chore: stabilize event emitter runners #12641

Merged
merged 2 commits into from Apr 7, 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 @@ -14,8 +14,9 @@
- `[jest-config]` [**BREAKING**] Rename `moduleLoader` to `runtime` ([#10817](https://github.com/facebook/jest/pull/10817))
- `[jest-config]` [**BREAKING**] Rename `extraGlobals` to `sandboxInjectedGlobals` ([#10817](https://github.com/facebook/jest/pull/10817))
- `[jest-config]` [**BREAKING**] Throw an error instead of showing a warning if multiple configs are used ([#12510](https://github.com/facebook/jest/pull/12510))
- `[jest-core]` Pass project config to `globalSetup`/`globalTeardown` function as second argument ([#12440](https://github.com/facebook/jest/pull/12440))
- `[jest-cli, jest-core]` Add `--ignoreProjects` CLI argument to ignore test suites by project name ([#12620](https://github.com/facebook/jest/pull/12620))
- `[jest-core]` Pass project config to `globalSetup`/`globalTeardown` function as second argument ([#12440](https://github.com/facebook/jest/pull/12440))
- `[jest-core]` Stabilize test runners with event emitters ([#12641](https://github.com/facebook/jest/pull/12641))
- `[jest-environment-jsdom]` [**BREAKING**] Upgrade jsdom to 19.0.0 ([#12290](https://github.com/facebook/jest/pull/12290))
- `[jest-environment-jsdom]` [**BREAKING**] Add default `browser` condition to `exportConditions` for `jsdom` environment ([#11924](https://github.com/facebook/jest/pull/11924))
- `[jest-environment-jsdom]` [**BREAKING**] Pass global config to Jest environment constructor for `jsdom` environment ([#12461](https://github.com/facebook/jest/pull/12461))
Expand Down
6 changes: 1 addition & 5 deletions packages/jest-core/src/TestScheduler.ts
Expand Up @@ -242,11 +242,7 @@ class TestScheduler {
serial: runInBand || Boolean(testRunner.isSerial),
};

/**
* Test runners with event emitters are still not supported
* for third party test runners.
*/
if (testRunner.__PRIVATE_UNSTABLE_API_supportsEventEmitters__) {
if (testRunner.supportsEventEmitters) {
const unsubscribes = [
testRunner.on('test-file-start', ([test]) =>
onTestFileStart(test),
Expand Down
104 changes: 23 additions & 81 deletions packages/jest-runner/src/index.ts
Expand Up @@ -47,7 +47,7 @@ export default class TestRunner {
private readonly _globalConfig: Config.GlobalConfig;
private readonly _context: TestRunnerContext;
private readonly eventEmitter = new Emittery<TestEvents>();
readonly __PRIVATE_UNSTABLE_API_supportsEventEmitters__: boolean = true;
readonly supportsEventEmitters: boolean = true;

readonly isSerial?: boolean;

Expand All @@ -59,29 +59,19 @@ export default class TestRunner {
async runTests(
tests: Array<Test>,
watcher: TestWatcher,
onStart: OnTestStart | undefined,
onResult: OnTestSuccess | undefined,
onFailure: OnTestFailure | undefined,
// keep these three as they're still passed and should be in the types,
// even if this particular runner doesn't use them
_onStart: OnTestStart | undefined,
_onResult: OnTestSuccess | undefined,
_onFailure: OnTestFailure | undefined,
options: TestRunnerOptions,
): Promise<void> {
return await (options.serial
? this._createInBandTestRun(tests, watcher, onStart, onResult, onFailure)
: this._createParallelTestRun(
tests,
watcher,
onStart,
onResult,
onFailure,
));
? this._createInBandTestRun(tests, watcher)
: this._createParallelTestRun(tests, watcher));
}

private async _createInBandTestRun(
tests: Array<Test>,
watcher: TestWatcher,
onStart?: OnTestStart,
onResult?: OnTestSuccess,
onFailure?: OnTestFailure,
) {
private async _createInBandTestRun(tests: Array<Test>, watcher: TestWatcher) {
process.env.JEST_WORKER_ID = '1';
const mutex = throat(1);
return tests.reduce(
Expand All @@ -92,19 +82,6 @@ export default class TestRunner {
if (watcher.isInterrupted()) {
throw new CancelRun();
}
// Remove `if(onStart)` in Jest 27
if (onStart) {
await onStart(test);

return runTest(
test.path,
this._globalConfig,
test.context.config,
test.context.resolver,
this._context,
undefined,
);
}

// `deepCyclicCopy` used here to avoid mem-leak
const sendMessageToJest: TestFileEvent = (eventName, args) =>
Expand All @@ -124,23 +101,11 @@ export default class TestRunner {
sendMessageToJest,
);
})
.then(result => {
if (onResult) {
return onResult(test, result);
}

return this.eventEmitter.emit('test-file-success', [
test,
result,
]);
})
.catch(err => {
if (onFailure) {
return onFailure(test, err);
}

return this.eventEmitter.emit('test-file-failure', [test, err]);
}),
.then(
result =>
this.eventEmitter.emit('test-file-success', [test, result]),
err => this.eventEmitter.emit('test-file-failure', [test, err]),
),
),
Promise.resolve(),
);
Expand All @@ -149,9 +114,6 @@ export default class TestRunner {
private async _createParallelTestRun(
tests: Array<Test>,
watcher: TestWatcher,
onStart?: OnTestStart,
onResult?: OnTestSuccess,
onFailure?: OnTestFailure,
) {
const resolvers: Map<string, SerializableResolver> = new Map();
for (const test of tests) {
Expand All @@ -168,11 +130,7 @@ export default class TestRunner {
forkOptions: {stdio: 'pipe'},
maxRetries: 3,
numWorkers: this._globalConfig.maxWorkers,
setupArgs: [
{
serializableResolvers: Array.from(resolvers.values()),
},
],
setupArgs: [{serializableResolvers: Array.from(resolvers.values())}],
}) as WorkerInterface;

if (worker.getStdout()) worker.getStdout().pipe(process.stdout);
Expand All @@ -188,12 +146,7 @@ export default class TestRunner {
return Promise.reject();
}

// Remove `if(onStart)` in Jest 27
if (onStart) {
await onStart(test);
} else {
await this.eventEmitter.emit('test-file-start', [test]);
}
await this.eventEmitter.emit('test-file-start', [test]);

const promise = worker.worker({
config: test.context.config,
Expand All @@ -212,9 +165,9 @@ export default class TestRunner {

if (promise.UNSTABLE_onCustomMessage) {
// TODO: Get appropriate type for `onCustomMessage`
promise.UNSTABLE_onCustomMessage(([event, payload]: any) => {
this.eventEmitter.emit(event, payload);
});
promise.UNSTABLE_onCustomMessage(([event, payload]: any) =>
this.eventEmitter.emit(event, payload),
);
}

return promise;
Expand All @@ -230,21 +183,10 @@ export default class TestRunner {

const runAllTests = Promise.all(
tests.map(test =>
runTestInWorker(test)
.then(result => {
if (onResult) {
return onResult(test, result);
}

return this.eventEmitter.emit('test-file-success', [test, result]);
})
.catch(error => {
if (onFailure) {
return onFailure(test, error);
}

return this.eventEmitter.emit('test-file-failure', [test, error]);
}),
runTestInWorker(test).then(
result => this.eventEmitter.emit('test-file-success', [test, result]),
error => this.eventEmitter.emit('test-file-failure', [test, error]),
),
),
);

Expand Down