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: move execution of setupFiles to jest-runner #9596

Merged
merged 2 commits into from
Feb 19, 2020
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

- `[docs]` Warn about unexpected behavior / bug of node-notifier when using the `notify` options.
- `[jest-resolver]` Use `resolve` package to implement custom module resolution ([#9520](https://github.com/facebook/jest/pull/9520))
- `[jest-runtime]` Move execution of `setupFiles` to `jest-runner` ([#9596](https://github.com/facebook/jest/pull/9596))
- `[@jest/reporters]` Remove unused dependencies and type exports ([#9462](https://github.com/facebook/jest/pull/9462))
- `[website]` Update pictures of reports when matchers fail ([#9214](https://github.com/facebook/jest/pull/9214))

Expand Down
4 changes: 1 addition & 3 deletions packages/jest-jasmine2/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,7 @@ async function jasmine2(
testPath,
});

config.setupFilesAfterEnv.forEach((path: Config.Path) =>
runtime.requireModule(path),
);
config.setupFilesAfterEnv.forEach(path => runtime.requireModule(path));

if (globalConfig.enabledTestsMap) {
env.specFilter = (spec: Spec) => {
Expand Down
2 changes: 2 additions & 0 deletions packages/jest-runner/src/runTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,8 @@ async function runTestInternal(

const start = Date.now();

config.setupFiles.forEach(path => runtime!.requireModule(path));
Copy link
Member Author

@SimenB SimenB Feb 19, 2020

Choose a reason for hiding this comment

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

I do this up here since it happened in the constructor previously. However, I wonder if it makes more sense to do it after we call the environment's setup function further down? If not, maybe right before? Seems like they belong together

https://github.com/facebook/jest/blob/9fbe3c5bd07002d569a1b4037d53556244a728cd/packages/jest-runner/src/runTest.ts#L230

Copy link
Collaborator

Choose a reason for hiding this comment

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

We can mark it as a todo for later, to group similar features together. However for now, to avoid unintended breakages, I'd stay with what's closer to previous version

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, makes sense


const sourcemapOptions: sourcemapSupport.Options = {
environment: 'node',
handleUncaughtExceptions: false,
Expand Down
42 changes: 23 additions & 19 deletions packages/jest-runtime/src/__mocks__/createRuntime.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import path from 'path';

module.exports = function createRuntime(filename, config) {
module.exports = async function createRuntime(filename, config) {
const NodeEnvironment = require('jest-environment-node');
const Runtime = require('../');

Expand Down Expand Up @@ -37,22 +37,26 @@ module.exports = function createRuntime(filename, config) {

const environment = new NodeEnvironment(config);
environment.global.console = console;
return Runtime.createHasteMap(config, {maxWorkers: 1, resetCache: false})
.build()
.then(hasteMap => {
const runtime = new Runtime(
config,
environment,
Runtime.createResolver(config, hasteMap.moduleMap),
);

runtime.__mockRootPath = path.join(config.rootDir, 'root.js');
runtime.__mockSubdirPath = path.join(
config.rootDir,
'subdir2',
'module_dir',
'module_dir_module.js',
);
return runtime;
});

const hasteMap = await Runtime.createHasteMap(config, {
maxWorkers: 1,
resetCache: false,
}).build();

const runtime = new Runtime(
config,
environment,
Runtime.createResolver(config, hasteMap.moduleMap),
);

config.setupFiles.forEach(path => runtime.requireModule(path));

runtime.__mockRootPath = path.join(config.rootDir, 'root.js');
runtime.__mockSubdirPath = path.join(
config.rootDir,
'subdir2',
'module_dir',
'module_dir_module.js',
);
return runtime;
};
3 changes: 3 additions & 0 deletions packages/jest-runtime/src/cli/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ export async function run(
setGlobal(environment.global, 'jestGlobalConfig', globalConfig);

const runtime = new Runtime(config, environment, hasteMap.resolver);

config.setupFiles.forEach(path => runtime.requireModule(path));

runtime.requireModule(filePath);
} catch (e) {
console.error(chalk.red(e.stack || e));
Expand Down
6 changes: 0 additions & 6 deletions packages/jest-runtime/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,12 +185,6 @@ class Runtime {
}

this.resetModules();

if (config.setupFiles.length) {
for (let i = 0; i < config.setupFiles.length; i++) {
this.requireModule(config.setupFiles[i]);
}
}
}

static shouldInstrument = shouldInstrument;
Expand Down
44 changes: 14 additions & 30 deletions packages/jest-util/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,36 +5,20 @@
* LICENSE file in the root directory of this source tree.
*/

import clearLine from './clearLine';
import createDirectory from './createDirectory';
import ErrorWithStack from './ErrorWithStack';
import installCommonGlobals from './installCommonGlobals';
import interopRequireDefault from './interopRequireDefault';
import isInteractive from './isInteractive';
import isPromise from './isPromise';
import setGlobal from './setGlobal';
import deepCyclicCopy from './deepCyclicCopy';
import convertDescriptorToString from './convertDescriptorToString';
export {default as clearLine} from './clearLine';
Copy link
Member Author

@SimenB SimenB Feb 19, 2020

Choose a reason for hiding this comment

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

snuck this in

export {default as createDirectory} from './createDirectory';
export {default as ErrorWithStack} from './ErrorWithStack';
export {default as installCommonGlobals} from './installCommonGlobals';
export {default as interopRequireDefault} from './interopRequireDefault';
export {default as isInteractive} from './isInteractive';
export {default as isPromise} from './isPromise';
export {default as setGlobal} from './setGlobal';
export {default as deepCyclicCopy} from './deepCyclicCopy';
export {default as convertDescriptorToString} from './convertDescriptorToString';
import * as specialChars from './specialChars';
import replacePathSepForGlob from './replacePathSepForGlob';
import testPathPatternToRegExp from './testPathPatternToRegExp';
export {default as replacePathSepForGlob} from './replacePathSepForGlob';
export {default as testPathPatternToRegExp} from './testPathPatternToRegExp';
import * as preRunMessage from './preRunMessage';
import pluralize from './pluralize';
export {default as pluralize} from './pluralize';

export {
ErrorWithStack,
clearLine,
convertDescriptorToString,
createDirectory,
deepCyclicCopy,
installCommonGlobals,
interopRequireDefault,
isInteractive,
isPromise,
pluralize,
preRunMessage,
replacePathSepForGlob,
setGlobal,
specialChars,
testPathPatternToRegExp,
};
export {preRunMessage, specialChars};