From 47af38f87e6a04fc116f41e8e42ad90ea800abd3 Mon Sep 17 00:00:00 2001 From: Yaroslav Serhieiev Date: Sat, 30 Sep 2023 17:46:31 +0300 Subject: [PATCH] fix: duplicate calls for multiple jest-metadata reporters (#36) --- packages/library/src/debug.ts | 6 +- .../src/jest-reporter/AssociateMetadata.ts | 14 ---- .../library/src/jest-reporter/FallbackAPI.ts | 32 ++++--- .../src/jest-reporter/QueryMetadata.ts | 3 +- .../src/jest-reporter/ReporterServer.ts | 84 +++++++++++++++++++ packages/library/src/jest-reporter/index.ts | 1 + packages/library/src/realms/BaseRealm.ts | 13 ++- .../library/src/realms/ChildProcessRealm.ts | 2 +- .../library/src/realms/ParentProcessRealm.ts | 11 ++- packages/library/src/realms/index.ts | 2 + packages/library/src/reporter.ts | 53 ++++-------- packages/recorder/jest.config.js | 1 + packages/recorder/mockReporter.js | 4 +- 13 files changed, 151 insertions(+), 75 deletions(-) create mode 100644 packages/library/src/jest-reporter/ReporterServer.ts diff --git a/packages/library/src/debug.ts b/packages/library/src/debug.ts index da9429f..ed7db95 100644 --- a/packages/library/src/debug.ts +++ b/packages/library/src/debug.ts @@ -1,4 +1,4 @@ -import { realm } from './realms'; +import { ParentProcessRealm, realm } from './realms'; export { aggregateLogs } from './utils'; @@ -7,7 +7,9 @@ export function isEnabled() { } export function isFallback() { - return realm.fallbackAPI.enabled; + return realm.type === 'parent_process' + ? (realm as ParentProcessRealm).fallbackAPI.enabled + : undefined; } export const events = realm.events; diff --git a/packages/library/src/jest-reporter/AssociateMetadata.ts b/packages/library/src/jest-reporter/AssociateMetadata.ts index e59e0f3..6826608 100644 --- a/packages/library/src/jest-reporter/AssociateMetadata.ts +++ b/packages/library/src/jest-reporter/AssociateMetadata.ts @@ -1,5 +1,3 @@ -import path from 'path'; - import type { TestCaseResult } from '@jest/reporters'; import { JestMetadataError } from '../errors'; @@ -8,24 +6,12 @@ import type { GlobalMetadata, Metadata, TestFileMetadata, TestEntryMetadata } fr export class AssociateMetadata { private readonly _map = new Map(); - constructor(private readonly _cwd: string) {} - filePath(value: string, metadata: TestFileMetadata): void { if (!value) { throw new JestMetadataError('Cannot associate metadata with an empty file path'); } this._map.set(value, metadata); - - if (path.isAbsolute(value)) { - this._map.set(path.relative(this._cwd, value), metadata); - } else { - this._map.set(path.resolve(value), metadata); - } - } - - testCaseName(nameIdentifier: string[], metadata: TestEntryMetadata): void { - this._map.set(nameIdentifier.join('\u001F'), metadata); } testCaseResult(testCaseResult: TestCaseResult, metadata: TestEntryMetadata): void { diff --git a/packages/library/src/jest-reporter/FallbackAPI.ts b/packages/library/src/jest-reporter/FallbackAPI.ts index 74c1375..0eed29f 100644 --- a/packages/library/src/jest-reporter/FallbackAPI.ts +++ b/packages/library/src/jest-reporter/FallbackAPI.ts @@ -1,7 +1,13 @@ import type { TestCaseResult, TestResult } from '@jest/reporters'; import { JestMetadataError } from '../errors'; -import type { GlobalMetadata, MetadataEventEmitter, TestEntryMetadata } from '../metadata'; -import { memoizeLast, Rotator } from '../utils'; +import type { + GlobalMetadata, + MetadataEventEmitter, + TestDoneEvent, + TestEntryMetadata, + TestSkipEvent, +} from '../metadata'; +import { Rotator } from '../utils'; export class FallbackAPI { private _fallbackMode: boolean | undefined = undefined; @@ -10,10 +16,7 @@ export class FallbackAPI { constructor( private readonly globalMetadata: GlobalMetadata, private readonly eventEmitter: MetadataEventEmitter, - ) { - this.reportTestFile = memoizeLast(this.reportTestFile.bind(this)); - this.reportTestCase = memoizeLast(this.reportTestCase.bind(this)); - } + ) {} public get enabled() { return this._fallbackMode ?? true; @@ -24,16 +27,18 @@ export class FallbackAPI { type: 'add_test_file', testFilePath, }); + + return this.globalMetadata.getTestFileMetadata(testFilePath); } - reportTestCase(testFilePath: string, testCaseResult: TestCaseResult) { + reportTestCase(testFilePath: string, testCaseResult: TestCaseResult): TestEntryMetadata { const file = this.globalMetadata.getTestFileMetadata(testFilePath); if (this._fallbackMode === undefined) { this._fallbackMode = !file.rootDescribeBlock; } if (!this._fallbackMode) { - return; + return file.lastTestEntry!; } if (!file.rootDescribeBlock) { @@ -88,7 +93,9 @@ export class FallbackAPI { type: this._getCompletionEventType(testCaseResult), testFilePath, testId, - }); + } as TestDoneEvent | TestSkipEvent | TestDoneEvent); + + return lastChild; } else { const tests = this._cache.get(nameIdentifier)!; const info = tests.find((t) => t.testCaseResult.status === 'failed')!; @@ -104,7 +111,9 @@ export class FallbackAPI { type: this._getCompletionEventType(testCaseResult), testFilePath: info.testFilePath, testId: info.testId, - }); + } as TestDoneEvent | TestSkipEvent | TestDoneEvent); + + return info.testEntryMetadata; } } @@ -148,7 +157,7 @@ export class FallbackAPI { type: this._getCompletionEventType(testCaseResult), testFilePath, testId, - }); + } as TestDoneEvent | TestSkipEvent | TestDoneEvent); } result.push(info ? info.testEntryMetadata : file.lastTestEntry!); @@ -188,5 +197,6 @@ type TestEntryInfo = { testId: string; testFilePath: string; testEntryMetadata: TestEntryMetadata; + /** Only or the last invocation */ testCaseResult: TestCaseResult; }; diff --git a/packages/library/src/jest-reporter/QueryMetadata.ts b/packages/library/src/jest-reporter/QueryMetadata.ts index 1b3b6bd..9ae0263 100644 --- a/packages/library/src/jest-reporter/QueryMetadata.ts +++ b/packages/library/src/jest-reporter/QueryMetadata.ts @@ -4,7 +4,6 @@ import { JestMetadataError } from '../errors'; import type { GlobalMetadata, MetadataChecker, - InstanceOfMetadataChecker, TestFileMetadata, TestEntryMetadata, } from '../metadata'; @@ -18,7 +17,7 @@ export class QueryMetadata { private readonly [_associate]: AssociateMetadata; private readonly [_checker]: MetadataChecker; - constructor(associate: AssociateMetadata, checker: InstanceOfMetadataChecker) { + constructor(associate: AssociateMetadata, checker: MetadataChecker) { this[_associate] = associate; this[_checker] = checker; } diff --git a/packages/library/src/jest-reporter/ReporterServer.ts b/packages/library/src/jest-reporter/ReporterServer.ts new file mode 100644 index 0000000..c7fe168 --- /dev/null +++ b/packages/library/src/jest-reporter/ReporterServer.ts @@ -0,0 +1,84 @@ +/* eslint-disable @typescript-eslint/no-empty-function,unicorn/no-for-loop */ +import type { Test, TestCaseResult, TestResult } from '@jest/reporters'; +import memoize from 'lodash.memoize'; +import type { IPCServer } from '../ipc'; +import { logger } from '../utils'; +import type { AssociateMetadata } from './AssociateMetadata'; +import type { FallbackAPI } from './FallbackAPI'; + +export type ReporterServerConfig = { + ipc: IPCServer; + fallbackAPI: FallbackAPI; + associate: AssociateMetadata; +}; + +/** + * @implements {import('@jest/reporters').Reporter} + */ +export class ReporterServer { + #log = logger.child({ cat: 'reporter', tid: 'reporter' }); + #associate: AssociateMetadata; + #fallbackAPI: FallbackAPI; + #ipc: IPCServer; + + constructor(config: ReporterServerConfig) { + this.#associate = config.associate; + this.#fallbackAPI = config.fallbackAPI; + this.#ipc = config.ipc; + + // We are memoizing all methods because there might be + // multiple reporters based on jest-metadata, so we need to + // make sure that we are calling every method only once per + // a given test case result. + // + // Unfortunately, we can't use a quicker `memoizeLast` because + // of obscure race conditions in Jest's ReporterDispatcher. + // This might be a good candidate for a future optimization. + + this.onRunStart = memoize(this.onRunStart.bind(this)); + this.onTestFileStart = memoize(this.onTestFileStart.bind(this)); + this.onTestCaseStart = memoize(this.onTestCaseStart.bind(this)); + this.onTestCaseResult = memoize(this.onTestCaseResult.bind(this)); + this.onTestFileResult = memoize(this.onTestFileResult.bind(this)); + this.onRunComplete = memoize(this.onRunComplete.bind(this)); + } + + async onRunStart(): Promise { + await this.#ipc.start(); + } + + onTestFileStart(testPath: string): void { + this.#log.debug.begin({ tid: ['reporter', testPath] }, testPath); + const testFileMetadata = this.#fallbackAPI.reportTestFile(testPath); + this.#associate.filePath(testPath, testFileMetadata); + } + + onTestCaseStart(test: Test): void { + this.#log.debug({ tid: ['reporter', test.path] }, 'onTestCaseStart'); + // We cannot use the fallback API here because `testCaseStartInfo` + // does not contain information, whether this is a retry or not. + // That's why we might end up with multiple test entries for the same test, + // so better to ignore this event, rather than distort the data. + } + + onTestCaseResult(test: Test, testCaseResult: TestCaseResult): void { + this.#log.debug({ tid: ['reporter', test.path] }, 'onTestCaseResult'); + + const lastTestEntry = this.#fallbackAPI.reportTestCase(test.path, testCaseResult); + this.#associate.testCaseResult(testCaseResult, lastTestEntry); + } + + onTestFileResult(test: Test, testResult: TestResult): void { + const allTestEntries = this.#fallbackAPI.reportTestFileResult(testResult); + const testResults = testResult.testResults; + for (let i = 0; i < testResults.length; i++) { + this.#associate.testCaseResult(testResults[i], allTestEntries[i]); + } + + this.#log.debug.end({ tid: ['reporter', test.path] }); + } + + async onRunComplete(): Promise { + await this.#ipc.stop(); + } +} diff --git a/packages/library/src/jest-reporter/index.ts b/packages/library/src/jest-reporter/index.ts index cec4e4e..5d93753 100644 --- a/packages/library/src/jest-reporter/index.ts +++ b/packages/library/src/jest-reporter/index.ts @@ -1,3 +1,4 @@ export { AssociateMetadata } from './AssociateMetadata'; export { FallbackAPI } from './FallbackAPI'; export { QueryMetadata } from './QueryMetadata'; +export { ReporterServer } from './ReporterServer'; diff --git a/packages/library/src/realms/BaseRealm.ts b/packages/library/src/realms/BaseRealm.ts index 64bef03..b09284d 100644 --- a/packages/library/src/realms/BaseRealm.ts +++ b/packages/library/src/realms/BaseRealm.ts @@ -1,6 +1,6 @@ import { EnvironmentEventHandler } from '../jest-environment'; -import { AssociateMetadata, FallbackAPI, QueryMetadata } from '../jest-reporter'; +import { AssociateMetadata, QueryMetadata } from '../jest-reporter'; import { GlobalMetadataRegistry, MetadataDSL, @@ -15,13 +15,13 @@ import { import { AggregatedEmitter, SerialSyncEmitter } from '../utils'; export abstract class BaseRealm { - readonly coreEmitter: MetadataEventEmitter = new SerialSyncEmitter('core').on( + readonly coreEmitter = new SerialSyncEmitter('core').on( '*', (event: MetadataEvent) => { this.metadataHandler.handle(event); }, - ); - readonly setEmitter: SetMetadataEventEmitter = new SerialSyncEmitter('set'); + ) as MetadataEventEmitter; + readonly setEmitter = new SerialSyncEmitter('set') as SetMetadataEventEmitter; readonly events = new AggregatedEmitter('events').add(this.coreEmitter); readonly metadataRegistry = new GlobalMetadataRegistry(); @@ -34,11 +34,10 @@ export abstract class BaseRealm { globalMetadata: this.globalMetadata, metadataRegistry: this.metadataRegistry, }); - readonly associate = new AssociateMetadata(process.cwd()); - readonly query = new QueryMetadata(this.associate, this.metadataFactory.checker); - readonly fallbackAPI = new FallbackAPI(this.globalMetadata, this.coreEmitter); readonly metadataDSL = new MetadataDSL( this.coreEmitter, () => this.globalMetadata.currentMetadata, ); + readonly associate = new AssociateMetadata(); + readonly query = new QueryMetadata(this.associate, this.metadataFactory.checker); } diff --git a/packages/library/src/realms/ChildProcessRealm.ts b/packages/library/src/realms/ChildProcessRealm.ts index 5cbf850..1a3d99f 100644 --- a/packages/library/src/realms/ChildProcessRealm.ts +++ b/packages/library/src/realms/ChildProcessRealm.ts @@ -6,7 +6,7 @@ import { BaseRealm } from './BaseRealm'; import { getClientId, getServerId } from './detect'; export class ChildProcessRealm extends BaseRealm { - readonly type = 'child_process'; + readonly type = 'child_process' as const; readonly environmentHandler: EnvironmentEventHandler = new EnvironmentEventHandler({ emitter: this.coreEmitter, diff --git a/packages/library/src/realms/ParentProcessRealm.ts b/packages/library/src/realms/ParentProcessRealm.ts index 0fa7823..6c7a051 100644 --- a/packages/library/src/realms/ParentProcessRealm.ts +++ b/packages/library/src/realms/ParentProcessRealm.ts @@ -1,11 +1,12 @@ import { IPCServer } from '../ipc'; +import { FallbackAPI, ReporterServer } from '../jest-reporter'; import { getVersion } from '../utils'; import { BaseRealm } from './BaseRealm'; import { registerServerId } from './detect'; export class ParentProcessRealm extends BaseRealm { - readonly type = 'parent_process'; + readonly type = 'parent_process' as const; readonly ipc = new IPCServer({ appspace: `jest-metadata@${getVersion()}-`, @@ -13,6 +14,14 @@ export class ParentProcessRealm extends BaseRealm { emitter: this.coreEmitter, }); + readonly fallbackAPI = new FallbackAPI(this.globalMetadata, this.coreEmitter); + + readonly reporterServer = new ReporterServer({ + associate: this.associate, + fallbackAPI: this.fallbackAPI, + ipc: this.ipc, + }); + constructor() { super(); diff --git a/packages/library/src/realms/index.ts b/packages/library/src/realms/index.ts index be16065..56007a1 100644 --- a/packages/library/src/realms/index.ts +++ b/packages/library/src/realms/index.ts @@ -1,2 +1,4 @@ export { injectRealmIntoSandbox } from './detect'; export { default as realm } from './realm'; +export type { ParentProcessRealm } from './ParentProcessRealm'; +export type { ChildProcessRealm } from './ChildProcessRealm'; diff --git a/packages/library/src/reporter.ts b/packages/library/src/reporter.ts index 504929b..c864a1b 100644 --- a/packages/library/src/reporter.ts +++ b/packages/library/src/reporter.ts @@ -1,6 +1,7 @@ /* eslint-disable @typescript-eslint/no-empty-function,unicorn/no-for-loop */ import type { AggregatedResult, + Config, Reporter, ReporterOnStartOptions, Test, @@ -8,29 +9,28 @@ import type { TestContext, TestResult, } from '@jest/reporters'; -import { realm } from './realms'; -import { logger } from './utils'; +import { JestMetadataError } from './errors'; +import { realm as unknownRealm } from './realms'; +import type { ParentProcessRealm } from './realms'; -export type JestMetadataServerReporterConfig = { - // empty for now -}; - -export const query = realm.query; +const realm = unknownRealm as ParentProcessRealm; /** * @implements {import('@jest/reporters').Reporter} */ export class JestMetadataReporter implements Reporter { - #log = logger.child({ cat: 'reporter', tid: 'reporter' }); - - constructor(_globalConfig: unknown, _options: JestMetadataServerReporterConfig) {} + constructor(_globalConfig: Config.GlobalConfig) { + if (realm.type !== 'parent_process') { + throw new JestMetadataError(`JestMetadataReporter can be used only in the parent process`); + } + } getLastError(): Error | void { return undefined; } - async onRunStart(_results: AggregatedResult, _options: ReporterOnStartOptions): Promise { - await realm.ipc.start(); + onRunStart(_results: AggregatedResult, _options: ReporterOnStartOptions): Promise { + return realm.reporterServer.onRunStart(); } /** @@ -41,10 +41,7 @@ export class JestMetadataReporter implements Reporter { } onTestFileStart(test: Test): void { - this.#log.debug.begin({ tid: ['reporter', test.path] }, test.path); - realm.fallbackAPI.reportTestFile(test.path); - const testFileMetadata = realm.globalMetadata.getTestFileMetadata(test.path); - realm.associate.filePath(test.path, testFileMetadata); + return realm.reporterServer.onTestFileStart(test.path); } /** @@ -52,19 +49,11 @@ export class JestMetadataReporter implements Reporter { * @see {import('@jest/types').Circus.TestCaseStartInfo} */ onTestCaseStart(test: Test, _testCaseStartInfo: /* for type safety */ unknown): void { - this.#log.debug({ tid: ['reporter', test.path] }, 'onTestCaseStart'); - // We cannot use the fallback API here because `testCaseStartInfo` - // does not contain information, whether this is a retry or not. - // That's why we might end up with multiple test entries for the same test, - // so better to ignore this event, rather than distort the data. + return realm.reporterServer.onTestCaseStart(test); } onTestCaseResult(test: Test, testCaseResult: TestCaseResult): void { - this.#log.debug({ tid: ['reporter', test.path] }, 'onTestCaseResult'); - - realm.fallbackAPI.reportTestCase(test.path, testCaseResult); - const lastTestEntry = realm.query.test(test)!.lastTestEntry!; - realm.associate.testCaseResult(testCaseResult, lastTestEntry); + return realm.reporterServer.onTestCaseResult(test, testCaseResult); } /** @@ -75,17 +64,11 @@ export class JestMetadataReporter implements Reporter { } onTestFileResult(test: Test, testResult: TestResult, _aggregatedResult: AggregatedResult): void { - const allTestEntries = realm.fallbackAPI.reportTestFileResult(testResult); - const testResults = testResult.testResults; - for (let i = 0; i < testResults.length; i++) { - realm.associate.testCaseResult(testResults[i], allTestEntries[i]); - } - - this.#log.debug.end({ tid: ['reporter', test.path] }); + return realm.reporterServer.onTestFileResult(test, testResult); } - async onRunComplete(_testContexts: Set, _results: AggregatedResult): Promise { - await realm.ipc.stop(); + onRunComplete(_testContexts: Set, _results: AggregatedResult): Promise { + return realm.reporterServer.onRunComplete(); } } diff --git a/packages/recorder/jest.config.js b/packages/recorder/jest.config.js index 3b35503..2d5d350 100644 --- a/packages/recorder/jest.config.js +++ b/packages/recorder/jest.config.js @@ -76,6 +76,7 @@ module.exports = { reporters: [ 'default', + 'jest-metadata/reporter', '/mockReporter' ], setupFilesAfterEnv: [ diff --git a/packages/recorder/mockReporter.js b/packages/recorder/mockReporter.js index 61514af..f998360 100644 --- a/packages/recorder/mockReporter.js +++ b/packages/recorder/mockReporter.js @@ -17,8 +17,8 @@ class MockReporter extends JestMetadataReporter { _single = false; _bail = false; - constructor(globalConfig, reporterConfig) { - super(globalConfig, reporterConfig); + constructor(globalConfig) { + super(globalConfig); this._single = globalConfig.maxWorkers === 1; this._bail = globalConfig.bail > 0;