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

Setup before tests but after framework loads #7119

Merged
merged 19 commits into from Oct 21, 2018
Merged
Show file tree
Hide file tree
Changes from 6 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -2,6 +2,7 @@

### Features

- `[jest-jasmine2/jest-circus/jest-cli]` Setup before tests but after framework loads ([#7119](https://github.com/facebook/jest/pull/7119))
SimenB marked this conversation as resolved.
Show resolved Hide resolved
- `[jest-jasmine2/jest-circus/jest-cli]` Add test.todo ([#6996](https://github.com/facebook/jest/pull/6996))
- `[pretty-format]` Option to not escape strings in diff messages ([#5661](https://github.com/facebook/jest/pull/5661))
- `[jest-haste-map]` Add `getFileIterator` to `HasteFS` for faster file iteration ([#7010](https://github.com/facebook/jest/pull/7010)).
Expand Down
1 change: 1 addition & 0 deletions TestUtils.js
Expand Up @@ -100,6 +100,7 @@ const DEFAULT_PROJECT_CONFIG: ProjectConfig = {
runner: 'jest-runner',
setupFiles: [],
setupTestFrameworkScriptFile: null,
setupTestsAfterJest: [],
skipFilter: false,
skipNodeResolution: false,
snapshotResolver: null,
Expand Down
1 change: 1 addition & 0 deletions e2e/__tests__/__snapshots__/show_config.test.js.snap
Expand Up @@ -45,6 +45,7 @@ exports[`--showConfig outputs config info and exits 1`] = `
\\"runner\\": \\"jest-runner\\",
\\"setupFiles\\": [],
\\"setupTestFrameworkScriptFile\\": null,
\\"setupTestsAfterJest\\": [],
\\"skipFilter\\": false,
\\"snapshotSerializers\\": [],
\\"testEnvironment\\": \\"jest-environment-jsdom\\",
Expand Down
60 changes: 60 additions & 0 deletions e2e/__tests__/setup_tests_after_jest_config.test.js
@@ -0,0 +1,60 @@
/**
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/
'use strict';

import path from 'path';
import {json as runWithJson} from '../runJest';
import {writeFiles} from '../Utils';

const DIR = path.resolve(__dirname, '../setup-tests-after-jest-config');

describe('setupTestsAfterJest', () => {
it('requires multiple setup files before each file in the suite', () => {
const pkgJson = {
jest: {
setupTestsAfterJest: ['./setup1.js', './setup2.js'],
},
};

writeFiles(DIR, {
'package.json': JSON.stringify(pkgJson, null, 2),
});

const result = runWithJson('setup-tests-after-jest-config', [
'test1.test.js',
'test2.test.js',
]);

expect(result.json.numTotalTests).toBe(2);
expect(result.json.numPassedTests).toBe(2);
expect(result.json.testResults.length).toBe(2);
expect(result.status).toBe(0);
});

it('requires setup files *after* the test runners are required', () => {
const pkgJson = {
jest: {
setupTestsAfterJest: ['./setup_hooks_into_runner.js'],
},
};

writeFiles(DIR, {
'package.json': JSON.stringify(pkgJson, null, 2),
});

const result = runWithJson('setup-tests-after-jest-config', [
'runner_patch.test.js',
]);

expect(result.json.numTotalTests).toBe(1);
expect(result.json.numPassedTests).toBe(1);
expect(result.json.testResults.length).toBe(1);
expect(result.status).toBe(0);
});
});
13 changes: 13 additions & 0 deletions e2e/setup-tests-after-jest-config/__tests__/runner_patch.test.js
@@ -0,0 +1,13 @@
/**
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
'use strict';

describe('setupFile', () => {
it('patches jasmine in setup file', () => {
expect(global.describeDefined).toBe(true);
});
});
13 changes: 13 additions & 0 deletions e2e/setup-tests-after-jest-config/__tests__/test1.test.js
@@ -0,0 +1,13 @@
/**
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
'use strict';

describe('test', () => {
it('has predefined global variable', () => {
expect(global.definedInSetupFile).toEqual(true);
});
});
13 changes: 13 additions & 0 deletions e2e/setup-tests-after-jest-config/__tests__/test2.test.js
@@ -0,0 +1,13 @@
/**
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
'use strict';

describe('test', () => {
it('has predefined global variable', () => {
expect(global.definedInSetupFile).toEqual(true);
});
});
7 changes: 7 additions & 0 deletions e2e/setup-tests-after-jest-config/package.json
@@ -0,0 +1,7 @@
{
"jest": {
"setupTestsAfterJest": [
"./setup_hooks_into_runner.js"
]
}
}
8 changes: 8 additions & 0 deletions e2e/setup-tests-after-jest-config/setup1.js
@@ -0,0 +1,8 @@
/**
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

global.definedInSetupFile = true;
8 changes: 8 additions & 0 deletions e2e/setup-tests-after-jest-config/setup2.js
@@ -0,0 +1,8 @@
/**
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

global.definedInSetupFile = true;
8 changes: 8 additions & 0 deletions e2e/setup-tests-after-jest-config/setup_hooks_into_runner.js
@@ -0,0 +1,8 @@
/**
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

global.describeDefined = !!global.describe;
Expand Up @@ -77,6 +77,12 @@ const jestAdapter = async (
runtime.requireModule(config.setupTestFrameworkScriptFile);
}

if (config.setupTestsAfterJest.length) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code will run setup files after any file set in setupTestFrameworkScriptFile. I wasn't sure how we want to handle this. Here are some ideas.

  1. Add details in the docs stating that the files set in the new config will run after that old option.
  2. In preparation for the deprecation of setupTestFrameworkScriptFile, only run the new config if set.

Copy link
Contributor

@wsmd wsmd Oct 8, 2018

Choose a reason for hiding this comment

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

Hmm.. you bring up a good point! I lean more towards option 2 since both serve the same purpose. I can't think of a case where I would have both set in my configuration.

With that, another idea I'd like to suggest is that if both are set, we could log a warning indicating that setupTestsAfterJest is taking precedence, and that setupTestFrameworkScriptFile is being ignored.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, how's the behaviour like if you pass in a wrong path to a file? Is that error handled or is that handled by the runtime. It would be good to gracefully handle that scenario

Copy link
Member

Choose a reason for hiding this comment

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

What about changing line 77 to:

config.setupTestsAfterJest.push(config.setupTestFrameworkScriptFile);

Copy link
Contributor Author

@ddruker ddruker Oct 14, 2018

Choose a reason for hiding this comment

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

@rickhanlonii Awesome. We would need to include info in the docs that mentions the module set using setupTestFrameworkScriptFile will run after any code set using the new configuration. If I do that, we should probably remove the original implementation of setupTestFrameworkScriptFile in the lines above. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah agreed, can probably add it to the doocs here https://jestjs.io/docs/en/configuration#setuptestframeworkscriptfile-string

What's the original implementation? Do you mean just line 77?

for (let i = 0; i < config.setupTestsAfterJest.length; i++) {
runtime.requireModule(config.setupTestsAfterJest[i]);
}
}

runtime.requireModule(testPath);
const results = await runAndTransformResultsToJestFormat({
config,
Expand Down
6 changes: 6 additions & 0 deletions packages/jest-cli/src/cli/args.js
Expand Up @@ -511,6 +511,12 @@ export const options = {
'set up the testing framework before each test.',
type: 'string',
},
setupTestsAfterJest: {
description:
'The paths to modules that run some code to configure or ' +
'set up the testing framework after each test. ',
type: 'array',
},
showConfig: {
default: undefined,
description: 'Print your jest config and then exits.',
Expand Down
Expand Up @@ -141,6 +141,9 @@ module.exports = {
// The path to a module that runs some code to configure or set up the testing framework before each test
// setupTestFrameworkScriptFile: null,

// The paths to modules that run some code to configure or set up the testing environment after each test
// setupTestsAfterJest: [],

// A list of paths to snapshot serializer modules Jest should use for snapshot testing
// snapshotSerializers: [],

Expand Down
1 change: 1 addition & 0 deletions packages/jest-config/src/Defaults.js
Expand Up @@ -61,6 +61,7 @@ export default ({
runner: 'jest-runner',
setupFiles: [],
setupTestFrameworkScriptFile: null,
setupTestsAfterJest: [],
skipFilter: false,
snapshotSerializers: [],
testEnvironment: 'jest-environment-jsdom',
Expand Down
2 changes: 2 additions & 0 deletions packages/jest-config/src/Descriptions.js
Expand Up @@ -63,6 +63,8 @@ export default ({
'The paths to modules that run some code to configure or set up the testing environment before each test',
setupTestFrameworkScriptFile:
'The path to a module that runs some code to configure or set up the testing framework before each test',
setupTestsAfterJest:
'The paths to modules that run some code to configure or set up the testing environment after each test',
snapshotSerializers:
'A list of paths to snapshot serializer modules Jest should use for snapshot testing',
testEnvironment: 'The test environment that will be used for testing',
Expand Down
1 change: 1 addition & 0 deletions packages/jest-config/src/ValidConfig.js
Expand Up @@ -88,6 +88,7 @@ export default ({
runner: 'jest-runner',
setupFiles: ['<rootDir>/setup.js'],
setupTestFrameworkScriptFile: '<rootDir>/testSetupFile.js',
setupTestsAfterJest: ['<rootDir>/testSetupFile.js'],
silent: true,
skipFilter: false,
skipNodeResolution: false,
Expand Down
47 changes: 47 additions & 0 deletions packages/jest-config/src/__tests__/normalize.test.js
Expand Up @@ -330,6 +330,53 @@ describe('haste', () => {
});
});

describe('setupTestsAfterJest', () => {
let Resolver;
beforeEach(() => {
Resolver = require('jest-resolve');
Resolver.findNodeModule = jest.fn(
name =>
name.startsWith('/') ? name : '/root/path/foo' + path.sep + name,
);
});

it('normalizes the path according to rootDir', () => {
const {options} = normalize(
{
rootDir: '/root/path/foo',
setupTestsAfterJest: ['bar/baz'],
},
{},
);

expect(options.setupTestsAfterJest).toEqual([expectedPathFooBar]);
});

it('does not change absolute paths', () => {
const {options} = normalize(
{
rootDir: '/root/path/foo',
setupTestsAfterJest: ['/an/abs/path'],
},
{},
);

expect(options.setupTestsAfterJest).toEqual([expectedPathAbs]);
});

it('substitutes <rootDir> tokens', () => {
const {options} = normalize(
{
rootDir: '/root/path/foo',
setupTestsAfterJest: ['<rootDir>/bar/baz'],
},
{},
);

expect(options.setupTestsAfterJest).toEqual([expectedPathFooBar]);
});
});

describe('setupTestFrameworkScriptFile', () => {
let Resolver;
beforeEach(() => {
Expand Down
1 change: 1 addition & 0 deletions packages/jest-config/src/index.js
Expand Up @@ -183,6 +183,7 @@ const getConfigs = (
runner: options.runner,
setupFiles: options.setupFiles,
setupTestFrameworkScriptFile: options.setupTestFrameworkScriptFile,
setupTestsAfterJest: options.setupTestsAfterJest,
skipFilter: options.skipFilter,
skipNodeResolution: options.skipNodeResolution,
snapshotResolver: options.snapshotResolver,
Expand Down
6 changes: 6 additions & 0 deletions packages/jest-config/src/normalize.js
Expand Up @@ -109,6 +109,11 @@ const setupPreset = (
if (options.setupFiles) {
options.setupFiles = (preset.setupFiles || []).concat(options.setupFiles);
}

if (!options.setupTestsAfterJest) {
options.setupTestsAfterJest = [];
}

if (options.modulePathIgnorePatterns && preset.modulePathIgnorePatterns) {
options.modulePathIgnorePatterns = preset.modulePathIgnorePatterns.concat(
options.modulePathIgnorePatterns,
Expand Down Expand Up @@ -414,6 +419,7 @@ export default function normalize(options: InitialOptions, argv: Argv) {
value = normalizeCollectCoverageOnlyFrom(options, key);
break;
case 'setupFiles':
case 'setupTestsAfterJest':
case 'snapshotSerializers':
value =
options[key] &&
Expand Down
6 changes: 6 additions & 0 deletions packages/jest-jasmine2/src/index.js
Expand Up @@ -131,6 +131,12 @@ async function jasmine2(
runtime.requireModule(config.setupTestFrameworkScriptFile);
}

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

if (globalConfig.enabledTestsMap) {
env.specFilter = spec => {
const suiteMap =
Expand Down
Expand Up @@ -108,6 +108,7 @@ const validConfig = {
roots: ['<rootDir>'],
setupFiles: ['<rootDir>/setup.js'],
setupTestFrameworkScriptFile: '<rootDir>/testSetupFile.js',
setupTestsAfterJest: ['<rootDir>/testSetupFile.js'],
silent: true,
snapshotSerializers: ['my-serializer-module'],
testEnvironment: 'jest-environment-jsdom',
Expand Down
1 change: 1 addition & 0 deletions types/Argv.js
Expand Up @@ -72,6 +72,7 @@ export type Argv = {|
runInBand: boolean,
setupFiles: Array<string>,
setupTestFrameworkScriptFile: string,
setupTestsAfterJest: Array<string>,
showConfig: boolean,
silent: boolean,
snapshotSerializers: Array<string>,
Expand Down
3 changes: 3 additions & 0 deletions types/Config.js
Expand Up @@ -65,6 +65,7 @@ export type DefaultOptions = {|
runTestsByPath: boolean,
setupFiles: Array<Path>,
setupTestFrameworkScriptFile: ?Path,
setupTestsAfterJest: Array<Path>,
skipFilter: boolean,
snapshotSerializers: Array<Path>,
testEnvironment: string,
Expand Down Expand Up @@ -150,6 +151,7 @@ export type InitialOptions = {
scriptPreprocessor?: string,
setupFiles?: Array<Path>,
setupTestFrameworkScriptFile?: Path,
setupTestsAfterJest?: Array<Path>,
silent?: boolean,
skipFilter?: boolean,
skipNodeResolution?: boolean,
Expand Down Expand Up @@ -272,6 +274,7 @@ export type ProjectConfig = {|
runner: string,
setupFiles: Array<Path>,
setupTestFrameworkScriptFile: ?Path,
setupTestsAfterJest: Array<Path>,
skipFilter: boolean,
skipNodeResolution: boolean,
snapshotResolver: ?Path,
Expand Down