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

Unify worker module map transmission w/ small perf benefit. #8237

Merged
merged 6 commits into from Mar 29, 2019
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -28,6 +28,7 @@
- `[jest-resolve]`: Remove internal peer dependencies ([#8215](https://github.com/facebook/jest/pull/8215))
- `[jest-snapshot]`: Remove internal peer dependencies ([#8215](https://github.com/facebook/jest/pull/8215))
- `[jest-resolve]` Fix requireActual with moduleNameMapper ([#8210](https://github.com/facebook/jest/pull/8210))
- `[jest-haste-map]` Fix haste map duplicate detection in watch mode ([#8237](https://github.com/facebook/jest/pull/8237))

### Chore & Maintenance

Expand Down
35 changes: 31 additions & 4 deletions packages/jest-haste-map/src/ModuleMap.ts
Expand Up @@ -12,7 +12,6 @@ import {
ModuleMetaData,
RawModuleMap,
ModuleMapData,
DuplicatesIndex,
MockData,
} from './types';

Expand All @@ -25,7 +24,7 @@ const EMPTY_MAP = new Map();
type ValueType<T> = T extends Map<string, infer V> ? V : never;

export type SerializableModuleMap = {
duplicates: ReadonlyArray<[string, ValueType<DuplicatesIndex>]>;
duplicates: ReadonlyArray<[string, [string, [string, [string, number]]]]>;
map: ReadonlyArray<[string, ValueType<ModuleMapData>]>;
mocks: ReadonlyArray<[string, ValueType<MockData>]>;
rootDir: Config.Path;
Expand All @@ -36,6 +35,30 @@ export default class ModuleMap {
private readonly _raw: RawModuleMap;
private json: SerializableModuleMap | undefined;

private static mapToArrayRecursive(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're probably wondering: what is this?

Turns out the ModuleMap wasn't being serialized in watch mode correctly but tests didn't catch it because we had 2 code paths.

1 code path = bug caught and fixed.

map: Map<any, any>,
): Array<[string, unknown]> {
let arr = Array.from(map);
if (arr[0] && arr[0][1] instanceof Map) {
arr = arr.map(
el => [el[0], this.mapToArrayRecursive(el[1])] as [string, unknown],
);
}
return arr;
}

private static mapFromArrayRecursive(
arr: ReadonlyArray<[string, unknown]>,
): Map<string, unknown> {
if (arr[0] && Array.isArray(arr[1])) {
Copy link

Choose a reason for hiding this comment

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

Why do you check for arr[1]? Is there some constraint that there are always at least 2 items in arr when going recursive?
Checking for arr[0][1] would make more sense.

arr = arr.map(el => [
el[0],
this.mapFromArrayRecursive(el[1] as Array<[string, unknown]>),
]) as Array<[string, unknown]>;
}
return new Map(arr);
}

constructor(raw: RawModuleMap) {
this._raw = raw;
}
Expand Down Expand Up @@ -87,7 +110,9 @@ export default class ModuleMap {
toJSON(): SerializableModuleMap {
if (!this.json) {
this.json = {
duplicates: Array.from(this._raw.duplicates),
duplicates: ModuleMap.mapToArrayRecursive(
this._raw.duplicates,
) as SerializableModuleMap['duplicates'],
map: Array.from(this._raw.map),
mocks: Array.from(this._raw.mocks),
rootDir: this._raw.rootDir,
Expand All @@ -98,7 +123,9 @@ export default class ModuleMap {

static fromJSON(serializableModuleMap: SerializableModuleMap) {
return new ModuleMap({
duplicates: new Map(serializableModuleMap.duplicates),
duplicates: ModuleMap.mapFromArrayRecursive(
serializableModuleMap.duplicates,
) as RawModuleMap['duplicates'],
map: new Map(serializableModuleMap.map),
mocks: new Map(serializableModuleMap.mocks),
rootDir: serializableModuleMap.rootDir,
Expand Down
37 changes: 0 additions & 37 deletions packages/jest-runner/src/__tests__/testRunner.test.js
Expand Up @@ -65,43 +65,6 @@ test('injects the serializable module map into each worker in watch mode', () =>
});
});

test('does not inject the serializable module map in serial mode', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

pretty sure this code path no longer exists with my changes, please sanity check my assumption

const globalConfig = {maxWorkers: 1, watch: false};
const config = {rootDir: '/path/'};
const context = {config};
const runContext = {};

return new TestRunner(globalConfig, runContext)
.runTests(
[{context, path: './file.test.js'}, {context, path: './file2.test.js'}],
new TestWatcher({isWatchMode: globalConfig.watch}),
() => {},
() => {},
() => {},
{serial: false},
)
.then(() => {
expect(mockWorkerFarm.worker.mock.calls).toEqual([
[
{
config,
context: runContext,
globalConfig,
path: './file.test.js',
},
],
[
{
config,
context: runContext,
globalConfig,
path: './file2.test.js',
},
],
]);
});
});

test('assign process.env.JEST_WORKER_ID = 1 when in runInBand mode', () => {
const globalConfig = {maxWorkers: 1, watch: false};
const config = {rootDir: '/path/'};
Expand Down
29 changes: 12 additions & 17 deletions packages/jest-runner/src/index.ts
Expand Up @@ -103,16 +103,13 @@ class TestRunner {
onResult: OnTestSuccess,
onFailure: OnTestFailure,
) {
let resolvers: Map<string, SerializableResolver> | undefined = undefined;
if (watcher.isWatchMode()) {
resolvers = new Map();
for (const test of tests) {
if (!resolvers.has(test.context.config.name)) {
resolvers.set(test.context.config.name, {
config: test.context.config,
serializableModuleMap: test.context.moduleMap.toJSON(),
});
}
const resolvers: Map<string, SerializableResolver> = new Map();
for (const test of tests) {
if (!resolvers.has(test.context.config.name)) {
resolvers.set(test.context.config.name, {
config: test.context.config,
serializableModuleMap: test.context.moduleMap.toJSON(),
});
}
}

Expand All @@ -121,13 +118,11 @@ class TestRunner {
forkOptions: {stdio: 'pipe'},
maxRetries: 3,
numWorkers: this._globalConfig.maxWorkers,
setupArgs: resolvers
? [
{
serializableResolvers: Array.from(resolvers.values()),
},
]
: undefined,
setupArgs: [
{
serializableResolvers: Array.from(resolvers.values()),
},
],
}) as WorkerInterface;

if (worker.getStdout()) worker.getStdout().pipe(process.stdout);
Expand Down
38 changes: 14 additions & 24 deletions packages/jest-runner/src/testWorker.ts
Expand Up @@ -8,7 +8,7 @@

import {Config} from '@jest/types';
import {SerializableError, TestResult} from '@jest/test-result';
import HasteMap, {ModuleMap, SerializableModuleMap} from 'jest-haste-map';
import HasteMap, {SerializableModuleMap} from 'jest-haste-map';
import exit from 'exit';
import {separateMessageFromStack} from 'jest-message-util';
import Runtime from 'jest-runtime';
Expand Down Expand Up @@ -53,34 +53,24 @@ const formatError = (error: string | ErrorWithCode): SerializableError => {
};

const resolvers = new Map<string, Resolver>();
const getResolver = (config: Config.ProjectConfig, moduleMap?: ModuleMap) => {
const name = config.name;
if (moduleMap || !resolvers.has(name)) {
resolvers.set(
name,
Runtime.createResolver(
config,
moduleMap || Runtime.createHasteMap(config).readModuleMap(),
),
);
const getResolver = (config: Config.ProjectConfig) => {
const resolver = resolvers.get(config.name);
if (!resolver) {
throw new Error('Cannot find resolver for: ' + config.name);
}
return resolvers.get(name)!;
return resolver;
};

export function setup(setupData?: {
export function setup(setupData: {
serializableResolvers: Array<SerializableResolver>;
}) {
// Setup data is only used in watch mode to pass the latest version of all
// module maps that will be used during the test runs. Otherwise, module maps
// are loaded from disk as needed.
if (setupData) {
for (const {
config,
serializableModuleMap,
} of setupData.serializableResolvers) {
const moduleMap = HasteMap.ModuleMap.fromJSON(serializableModuleMap);
getResolver(config, moduleMap);
}
// Module maps that will be needed for the test runs are passed.
for (const {
config,
serializableModuleMap,
} of setupData.serializableResolvers) {
const moduleMap = HasteMap.ModuleMap.fromJSON(serializableModuleMap);
resolvers.set(config.name, Runtime.createResolver(config, moduleMap));
}
}

Expand Down