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 5 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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

### Fixes

- `[jest-circus]` Setup globals before emitting `init`, and include Jest globals in the `init` payload ([#10598](https://github.com/facebook/jest/pull/10598))

### Chore & Maintenance

### Performance
Expand Down
11 changes: 2 additions & 9 deletions packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,24 +43,17 @@ const jestAdapter = async (
const {globals, snapshotState} = await initialize({
config,
environment,
expect,
getBabelTraverse,
getPrettier,
globalConfig,
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
Original file line number Diff line number Diff line change
Expand Up @@ -35,29 +35,38 @@ import {getTestID} from '../utils';
import run from '../run';
import testCaseReportHandler from '../testCaseReportHandler';
import globals from '..';
import type {Expect} from './jestExpect';

type Process = NodeJS.Process;

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

export const initialize = async ({
config,
environment,
expect,
getPrettier,
getBabelTraverse,
globalConfig,
localRequire,
parentProcess,
testPath,
sendMessageToJest,
setGlobalsForRuntime,
testPath,
}: {
config: Config.ProjectConfig;
environment: JestEnvironment;
expect: Expect;
getPrettier: () => null | any;
getBabelTraverse: () => typeof BabelTraverse;
globalConfig: Config.GlobalConfig;
localRequire: <T = unknown>(path: Config.Path) => T;
testPath: Config.Path;
parentProcess: Process;
sendMessageToJest?: TestFileEvent;
setGlobalsForRuntime?: (globals: JestGlobals) => void;
}): Promise<{
globals: Global.TestFrameworkGlobals;
snapshotState: SnapshotStateType;
Expand Down Expand Up @@ -120,9 +129,19 @@ export const initialize = async ({
addEventHandler(environment.handleTestEvent.bind(environment));
}

const runtimeGlobals: JestGlobals = {expect, ...globalsObject};
// 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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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