-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Changes from all commits
5168af8
1485c55
2128266
85fa793
8d3a28f
262eaf5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,6 @@ import { | |
ModuleMetaData, | ||
RawModuleMap, | ||
ModuleMapData, | ||
DuplicatesIndex, | ||
MockData, | ||
} from './types'; | ||
|
||
|
@@ -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; | ||
|
@@ -36,6 +35,30 @@ export default class ModuleMap { | |
private readonly _raw: RawModuleMap; | ||
private json: SerializableModuleMap | undefined; | ||
|
||
private static mapToArrayRecursive( | ||
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])) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you check for |
||
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; | ||
} | ||
|
@@ -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, | ||
|
@@ -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, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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/'}; | ||
|
There was a problem hiding this comment.
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.