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

fix(circus): make sure to install globals before emitting setup event #10598

Merged
merged 8 commits into from Oct 6, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Expand Up @@ -4,7 +4,8 @@

### Fixes

- `[jest-mock]` Fix typings for `mockResolvedValue`, `mockResolvedValueOnce`, `mockRejectedValue` and `mockRejectedValueOnce`
- `[jest-circus]` Setup globals before emitting `setup`, and include Jest globals in the `setup` payload ([#10598](https://github.com/facebook/jest/pull/10598))
- `[jest-mock]` Fix typings for `mockResolvedValue`, `mockResolvedValueOnce`, `mockRejectedValue` and `mockRejectedValueOnce` ([#10600](https://github.com/facebook/jest/pull/10600))

### Chore & Maintenance

Expand Down
18 changes: 2 additions & 16 deletions packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapter.ts
Expand Up @@ -5,7 +5,6 @@
* LICENSE file in the root directory of this source tree.
*/

import * as path from 'path';
import type {Config} from '@jest/types';
import type {JestEnvironment} from '@jest/environment';
import type {TestResult} from '@jest/test-result';
Expand All @@ -14,8 +13,7 @@ import type {RuntimeType as Runtime} from 'jest-runtime';
import type {SnapshotStateType} from 'jest-snapshot';
import {deepCyclicCopy} from 'jest-util';

const FRAMEWORK_INITIALIZER = path.resolve(__dirname, './jestAdapterInit.js');
const EXPECT_INITIALIZER = path.resolve(__dirname, './jestExpect.js');
const FRAMEWORK_INITIALIZER = require.resolve('./jestAdapterInit');

const jestAdapter = async (
globalConfig: Config.GlobalConfig,
Expand All @@ -32,10 +30,6 @@ const jestAdapter = async (
FRAMEWORK_INITIALIZER,
);

const expect = runtime
.requireInternalModule<typeof import('./jestExpect')>(EXPECT_INITIALIZER)
.default(globalConfig);

const getPrettier = () =>
config.prettierPath ? require(config.prettierPath) : null;
const getBabelTraverse = () => require('@babel/traverse').default;
Expand All @@ -49,18 +43,10 @@ const jestAdapter = async (
localRequire: runtime.requireModule.bind(runtime),
parentProcess: process,
sendMessageToJest,
setGlobalsForRuntime: runtime.setGlobalsForRuntime?.bind(runtime),
testPath,
});

const runtimeGlobals = {expect, ...globals};
// TODO: `jest-circus` might be newer than `jest-runtime` - remove `?.` for Jest 27
runtime.setGlobalsForRuntime?.(runtimeGlobals);

// TODO: `jest-circus` might be newer than `jest-config` - remove `??` for Jest 27
if (config.injectGlobals ?? true) {
Object.assign(environment.global, runtimeGlobals);
}

if (config.timers === 'fake' || config.timers === 'legacy') {
// during setup, this cannot be null (and it's fine to explode if it is)
environment.fakeTimers!.useFakeTimers();
Expand Down
Expand Up @@ -35,9 +35,14 @@ import {getTestID} from '../utils';
import run from '../run';
import testCaseReportHandler from '../testCaseReportHandler';
import globals from '..';
import createExpect, {Expect} from './jestExpect';

type Process = NodeJS.Process;

interface JestGlobals extends Global.TestFrameworkGlobals {
expect: Expect;
}

export const initialize = async ({
config,
environment,
Expand All @@ -46,8 +51,9 @@ export const initialize = async ({
globalConfig,
localRequire,
parentProcess,
testPath,
sendMessageToJest,
setGlobalsForRuntime,
testPath,
}: {
config: Config.ProjectConfig;
environment: JestEnvironment;
Expand All @@ -58,6 +64,7 @@ export const initialize = async ({
testPath: Config.Path;
parentProcess: Process;
sendMessageToJest?: TestFileEvent;
setGlobalsForRuntime?: (globals: JestGlobals) => void;
}): Promise<{
globals: Global.TestFrameworkGlobals;
snapshotState: SnapshotStateType;
Expand Down Expand Up @@ -120,9 +127,22 @@ export const initialize = async ({
addEventHandler(environment.handleTestEvent.bind(environment));
}

const runtimeGlobals: JestGlobals = {
...globalsObject,
expect: createExpect(globalConfig),
};
// TODO: `jest-circus` might be newer than `jest-runtime` - remove `?.` for Jest 27
setGlobalsForRuntime?.(runtimeGlobals);

// TODO: `jest-circus` might be newer than `jest-config` - remove `??` for Jest 27
if (config.injectGlobals ?? true) {
Object.assign(environment.global, runtimeGlobals);
}

await dispatch({
name: 'setup',
parentProcess,
runtimeGlobals,
Copy link
Collaborator

Choose a reason for hiding this comment

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

shall we document this somehow?

Copy link
Member Author

Choose a reason for hiding this comment

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

we probably should, but the events as of now are not documented so I'd rather do that separately at some point

testNamePattern: globalConfig.testNamePattern,
});

Expand Down
Expand Up @@ -16,7 +16,9 @@ import {
toThrowErrorMatchingSnapshot,
} from 'jest-snapshot';

export default (config: Pick<Config.GlobalConfig, 'expand'>): typeof expect => {
export type Expect = typeof expect;

export default (config: Pick<Config.GlobalConfig, 'expand'>): Expect => {
expect.setState({expand: config.expand});
expect.extend({
toMatchInlineSnapshot,
Expand Down
6 changes: 6 additions & 0 deletions packages/jest-types/src/Circus.ts
Expand Up @@ -38,6 +38,11 @@ export interface EventHandler {

export type Event = SyncEvent | AsyncEvent;

interface JestGlobals extends Global.TestFrameworkGlobals {
// we cannot type `expect` properly as it'd create circular dependencies
expect: unknown;
}

export type SyncEvent =
| {
asyncError: Error;
Expand Down Expand Up @@ -77,6 +82,7 @@ export type AsyncEvent =
// first action to dispatch. Good time to initialize all settings
name: 'setup';
testNamePattern?: string;
runtimeGlobals: JestGlobals;
parentProcess: Process;
}
| {
Expand Down
15 changes: 6 additions & 9 deletions packages/jest-util/src/globsToMatcher.ts
Expand Up @@ -9,12 +9,11 @@ import micromatch = require('micromatch');
import type {Config} from '@jest/types';
import replacePathSepForGlob from './replacePathSepForGlob';

type Matcher = (str: Config.Path) => boolean;
Copy link
Member Author

Choose a reason for hiding this comment

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

totally unrelated, just snuck these in 🤘


const globsToMatchersMap = new Map<
string,
{
isMatch: (str: string) => boolean;
negated: boolean;
}
{isMatch: Matcher; negated: boolean}
>();

const micromatchOptions = {dot: true};
Expand All @@ -36,13 +35,11 @@ const micromatchOptions = {dot: true};
* isMatch('pizza.js'); // true
* isMatch('pizza.test.js'); // false
*/
export default function globsToMatcher(
globs: Array<Config.Glob>,
): (path: Config.Path) => boolean {
export default function globsToMatcher(globs: Array<Config.Glob>): Matcher {
if (globs.length === 0) {
// Since there were no globs given, we can simply have a fast path here and
// return with a very simple function.
return (_: Config.Path): boolean => false;
return () => false;
}

const matchers = globs.map(glob => {
Expand All @@ -62,7 +59,7 @@ export default function globsToMatcher(
return globsToMatchersMap.get(glob)!;
});

return (path: Config.Path): boolean => {
return path => {
const replacedPath = replacePathSepForGlob(path);
let kept = undefined;
let negatives = 0;
Expand Down