Skip to content

fix: implement CallbackTestRunnerInterface instead of extending CallbackTestRunner class #134

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Aug 9, 2022
Merged

fix: implement CallbackTestRunnerInterface instead of extending CallbackTestRunner class #134

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 15 additions & 8 deletions lib/createJestRunner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { TestResult } from '@jest/test-result';
import {
CallbackTestRunner,
import type {
CallbackTestRunnerInterface,
Config,
OnTestFailure,
OnTestStart,
OnTestSuccess,
Expand Down Expand Up @@ -32,8 +33,14 @@ export default function createRunner<
>(
runPath: string,
{ getExtraOptions }: CreateRunnerOptions<ExtraOptions> = {},
): typeof CallbackTestRunner {
return class BaseTestRunner extends CallbackTestRunner {
) {
return class BaseTestRunner implements CallbackTestRunnerInterface {
#globalConfig: Config.GlobalConfig;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TS is fine with #, but does not allow to have private _globalConfig: "Property '_globalConfig' of exported class expression may not be private or protected."

Hm.. Should Node 12 support be dropped before merging this change?

create-jest-runner/.github/workflows/nodejs.yml

Lines 36 to 37 in 445b564

matrix:
node-version: [12.x, 14.x, 16.x, 17.x]

Copy link
Member

Choose a reason for hiding this comment

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

do we need to use # now, or is that just a refactoring?

Copy link
Contributor Author

@mrazauskas mrazauskas Aug 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Can be public as well. Is that fine?

private was not allowed by TS, so I went for #. But before #113 this field was marked public. Can stay like that too.

Copy link
Member

Choose a reason for hiding this comment

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

Private is fine 🙂 this class isn't used by consumers anyways

Copy link
Member

Choose a reason for hiding this comment

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

@mrazauskas landed #135 fwiw


constructor(globalConfig: Config.GlobalConfig) {
this.#globalConfig = globalConfig;
}

runTests(
tests: Array<Test>,
watcher: TestWatcher,
Expand Down Expand Up @@ -84,7 +91,7 @@ export default function createRunner<
const runner = require(runPath);
const baseOptions = {
config: test.context.config,
globalConfig: this._globalConfig,
globalConfig: this.#globalConfig,
testPath: test.path,
rawModuleMap: watcher.isWatchMode()
? test.context.moduleMap.getRawModuleMap()
Expand Down Expand Up @@ -118,13 +125,13 @@ export default function createRunner<
): Promise<void> {
const worker = new Worker(runPath, {
exposedMethods: ['default'],
numWorkers: this._globalConfig.maxWorkers,
numWorkers: this.#globalConfig.maxWorkers,
forkOptions: { stdio: 'inherit' },
}) as JestWorkerFarm<{
default: (runTestOptions: RunTestOptions) => TestResult;
}>;

const mutex = throat(this._globalConfig.maxWorkers);
const mutex = throat(this.#globalConfig.maxWorkers);

const runTestInWorker = (test: Test) =>
mutex(() => {
Expand All @@ -135,7 +142,7 @@ export default function createRunner<
return onStart(test).then(() => {
const runTestOptions: RunTestOptions = {
config: test.context.config,
globalConfig: this._globalConfig,
globalConfig: this.#globalConfig,
testPath: test.path,
rawModuleMap: watcher.isWatchMode()
? test.context.moduleMap.getRawModuleMap()
Expand Down