Skip to content

feat: migrate lib to typescript #40

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

Merged
merged 7 commits into from
Sep 1, 2021
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .babelrc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@
"node": "6"
}
}
],
[
"@babel/preset-typescript",
{
"allowDeclareFields": true
}
]
]
}
2 changes: 0 additions & 2 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
**/node_modules
build
.yarn/
# remove after migrating to TS
types/
28 changes: 23 additions & 5 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,26 @@
"extends": [
"airbnb-base",
"plugin:@typescript-eslint/recommended",
"plugin:import/typescript",
"plugin:prettier/recommended"
],
"plugins": ["jest"],
"rules": {
"no-underscore-dangle": "off",
"import/export": "off",
"import/extensions": [
"error",
"ignorePackages",
{
"js": "never",
"mjs": "never",
"jsx": "never",
"ts": "never"
}
],
"max-classes-per-file": "off",
"@typescript-eslint/prefer-ts-expect-error": "error",
// TODO: turn on after migrating to TS
"@typescript-eslint/no-var-requires": "off",
"@typescript-eslint/explicit-module-boundary-types": "off"
"no-underscore-dangle": "off",
"no-useless-constructor": "off",
"@typescript-eslint/prefer-ts-expect-error": "error"
},
"env": {
"jest/globals": true
Expand All @@ -28,6 +38,14 @@
"rules": {
"no-console": "off"
}
},
{
// TODO: remove after migrating to TS
"files": "**/*.js",
"rules": {
"@typescript-eslint/no-var-requires": "off",
"@typescript-eslint/explicit-module-boundary-types": "off"
}
}
]
}
6 changes: 6 additions & 0 deletions .github/workflows/nodejs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ jobs:
run: yarn
- name: build
run: yarn build
- name: build types
run: yarn build-types
- name: run eslint
run: yarn lint
- name: run prettier
Expand All @@ -46,6 +48,8 @@ jobs:
run: yarn
- name: build
run: yarn build
- name: build types
run: yarn build-types
- name: run tests
run: yarn test
env:
Expand All @@ -68,6 +72,8 @@ jobs:
run: yarn
- name: build
run: yarn build
- name: build types
run: yarn build-types
- name: run tests
run: yarn test
env:
Expand Down
1 change: 1 addition & 0 deletions integrationTests/runner/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// eslint-disable-next-line import/extensions, import/no-unresolved -- ignore build artifact
const { createJestRunner } = require('../..');

module.exports = createJestRunner(require.resolve('./run'));
1 change: 1 addition & 0 deletions integrationTests/runner/run.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const fs = require('fs');
// eslint-disable-next-line import/extensions, import/no-unresolved -- ignore build artifact
const { pass, fail, skip, todo } = require('../..');

module.exports = ({ testPath }) => {
Expand Down
88 changes: 54 additions & 34 deletions lib/createJestRunner.js → lib/createJestRunner.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,37 @@
import throat from 'throat';
import type * as JestResult from '@jest/test-result';
import type { Config } from '@jest/types';
import type * as JestRunner from 'jest-runner';
import { Worker } from 'jest-worker';
import throat from 'throat';
import type { CreateRunnerOptions, Path, TestRunner } from './types';

class CancelRun extends Error {
constructor(message) {
constructor(message?: string) {
super(message);
this.name = 'CancelRun';
}
}

const createRunner = (runPath, { getExtraOptions } = {}) => {
class BaseTestRunner {
constructor(globalConfig) {
this._globalConfig = globalConfig;
}

runTests(tests, watcher, onStart, onResult, onFailure, options) {
export default function createRunner<
ExtraOptionsType extends Record<string, unknown>,
>(
runPath: Path,
{ getExtraOptions }: CreateRunnerOptions<ExtraOptionsType> = {},
): typeof TestRunner {
return class BaseTestRunner implements TestRunner {
constructor(
public readonly _globalConfig: Config.GlobalConfig,
public readonly _context: JestRunner.TestRunnerContext = {},
) {}

runTests(
tests: Array<JestRunner.Test>,
watcher: JestRunner.TestWatcher,
onStart: JestRunner.OnTestStart,
onResult: JestRunner.OnTestSuccess,
onFailure: JestRunner.OnTestFailure,
options: JestRunner.TestRunnerOptions,
): Promise<void> {
return options.serial
? this._createInBandTestRun(
tests,
Expand All @@ -35,13 +52,13 @@ const createRunner = (runPath, { getExtraOptions } = {}) => {
}

_createInBandTestRun(
tests,
watcher,
onStart,
onResult,
onFailure,
options,
) {
tests: Array<JestRunner.Test>,
watcher: JestRunner.TestWatcher,
onStart: JestRunner.OnTestStart,
onResult: JestRunner.OnTestSuccess,
onFailure: JestRunner.OnTestFailure,
options: JestRunner.TestRunnerOptions,
): Promise<void> {
const mutex = throat(1);
return tests.reduce(
(promise, test) =>
Expand All @@ -53,7 +70,7 @@ const createRunner = (runPath, { getExtraOptions } = {}) => {
}

return onStart(test).then(() => {
// eslint-disable-next-line import/no-dynamic-require, global-require
// eslint-disable-next-line import/no-dynamic-require, global-require, @typescript-eslint/no-var-requires
const runner = require(runPath);
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should do import(runPath) here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can be done by changing it to:

onStart(test).then(() => import(runPath)).then(runner => { /* */ })

But babel would emit more code

onStart(test)
  .then(() => Promise.resolve(`${runPath}`)
  .then(s => _interopRequireWildcard(require(s))))
  .then(runner => { /* */ })

Instead of what it currently emits:

onStart(test).then(() => {
  const runner = require(runPath);
  /* */
})

Copy link
Member

Choose a reason for hiding this comment

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

we should change when we drop node 10

const baseOptions = {
config: test.context.config,
Expand Down Expand Up @@ -81,13 +98,13 @@ const createRunner = (runPath, { getExtraOptions } = {}) => {
}

_createParallelTestRun(
tests,
watcher,
onStart,
onResult,
onFailure,
options,
) {
tests: Array<JestRunner.Test>,
watcher: JestRunner.TestWatcher,
onStart: JestRunner.OnTestStart,
onResult: JestRunner.OnTestSuccess,
onFailure: JestRunner.OnTestFailure,
options: JestRunner.TestRunnerOptions,
): Promise<void> {
const worker = new Worker(runPath, {
exposedMethods: ['default'],
numWorkers: this._globalConfig.maxWorkers,
Expand All @@ -96,7 +113,7 @@ const createRunner = (runPath, { getExtraOptions } = {}) => {

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

const runTestInWorker = test =>
const runTestInWorker = (test: JestRunner.Test) =>
mutex(() => {
if (watcher.isInterrupted()) {
throw new CancelRun();
Expand All @@ -114,12 +131,16 @@ const createRunner = (runPath, { getExtraOptions } = {}) => {
extraOptions: getExtraOptions ? getExtraOptions() : {},
};

// @ts-expect-error -- the required module should have a default export
return worker.default(baseOptions);
});
});

const onError = (err, test) =>
onFailure(test, err).then(() => {
const onError = (
err: JestResult.SerializableError,
test: JestRunner.Test,
) => {
return onFailure(test, err).then(() => {
if (err.type === 'ProcessTerminatedError') {
// eslint-disable-next-line no-console
console.error(
Expand All @@ -129,6 +150,7 @@ const createRunner = (runPath, { getExtraOptions } = {}) => {
process.exit(1);
}
});
};

const onInterrupt = new Promise((_, reject) => {
watcher.on('change', state => {
Expand All @@ -146,13 +168,11 @@ const createRunner = (runPath, { getExtraOptions } = {}) => {
),
);

const cleanup = () => worker.end();
const cleanup = () => {
worker.end();
};

return Promise.race([runAllTests, onInterrupt]).then(cleanup, cleanup);
}
}

return BaseTestRunner;
};

export default createRunner;
};
}
18 changes: 0 additions & 18 deletions lib/fail.js

This file was deleted.

52 changes: 52 additions & 0 deletions lib/fail.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import type { TestResult } from '@jest/test-result';
import toTestResult from './toTestResult';
import type { Path } from './types';

interface Options {
start: number;
end: number;
test: { title: string; path: Path; errorMessage?: string };
errorMessage?: string;
}

export default function fail(options: {
start: number;
end: number;
test: { title: string; path: Path; errorMessage: string };
}): TestResult;

export default function fail(options: {
start: number;
end: number;
test: { title: string; path: Path };
errorMessage: string;
}): TestResult;

export default function fail({
start,
end,
test,
errorMessage,
}: Options): TestResult {
// TODO: Currently the `fail` function allows 2 ways to pass an error message.
// Both methods are currently in used by downstream packages.
// The current behaviour is to favour `errorMessage` over `test.errorMessage`.
const actualErrorMessage = errorMessage || test.errorMessage;

return toTestResult({
errorMessage: actualErrorMessage,
stats: {
failures: 1,
pending: 0,
passes: 0,
todo: 0,
start,
end,
},
skipped: false,
tests: [
{ duration: end - start, ...test, errorMessage: actualErrorMessage },
],
jestTestPath: test.path,
});
}
File renamed without changes.
17 changes: 0 additions & 17 deletions lib/pass.js

This file was deleted.

25 changes: 25 additions & 0 deletions lib/pass.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import type { TestResult } from '@jest/test-result';
import toTestResult from './toTestResult';
import type { TestDetail } from './types';

interface Options {
start: number;
end: number;
test: TestDetail;
}

export default function pass({ start, end, test }: Options): TestResult {
return toTestResult({
stats: {
failures: 0,
pending: 0,
passes: 1,
todo: 0,
start,
end,
},
skipped: false,
tests: [{ duration: end - start, ...test }],
jestTestPath: test.path,
});
}
18 changes: 0 additions & 18 deletions lib/skip.js

This file was deleted.

Loading