From c70fc235e789e8296d4e06af6e60873de7d2367d Mon Sep 17 00:00:00 2001 From: Nico Jansen Date: Sat, 16 Mar 2024 00:23:39 +0100 Subject: [PATCH] refactor(real-time): move update logic to metrics package Make `mutation-testing-metrics` responsible for the real-time calculations in the report. This has a couple of advantages: - Easier to reason about: the metrics are always up-to-date. - Easier to maintain: no ad-hoc calculations are needed anymore - Can be reused from StrykerJS itself, for example, in the console reporter Note: this refactor is needed to enable fine-grained reactivity. --- .../src/components/app/app.component.ts | 113 ++++------------ .../unit/components/app.component.spec.ts | 43 ------ .../drawer-mutant.component.spec.ts | 23 ++-- .../elements/test/unit/helpers/factory.ts | 12 +- packages/metrics/src/calculateMetrics.ts | 114 ++++++++++------ packages/metrics/src/index.ts | 2 +- packages/metrics/src/model/index.ts | 2 +- packages/metrics/src/model/metrics-result.ts | 85 ++++-------- packages/metrics/src/model/mutant-model.ts | 67 +++++----- .../src/model/mutation-test-metrics-result.ts | 64 ++++++++- packages/metrics/src/model/test-model.ts | 46 ++++--- packages/metrics/test/helpers/factories.ts | 2 +- .../test/unit/calculateMetrics.spec.ts | 10 +- .../test/unit/model/mutant-model.spec.ts | 5 +- .../mutation-test-metrics-result.spec.ts | 123 ++++++++++++++++++ .../test/unit/model/test-model.spec.ts | 2 +- workspace.code-workspace | 11 +- 17 files changed, 421 insertions(+), 303 deletions(-) create mode 100644 packages/metrics/test/unit/model/mutation-test-metrics-result.spec.ts diff --git a/packages/elements/src/components/app/app.component.ts b/packages/elements/src/components/app/app.component.ts index 171962911..4b469f68d 100644 --- a/packages/elements/src/components/app/app.component.ts +++ b/packages/elements/src/components/app/app.component.ts @@ -7,7 +7,7 @@ import type { MetricsResult, MutantModel, TestModel } from 'mutation-testing-met import { calculateMutationTestMetrics } from 'mutation-testing-metrics'; import { tailwind, globals } from '../../style/index.js'; import { locationChange$, View } from '../../lib/router.js'; -import type { Subscription } from 'rxjs'; +import { Subscription } from 'rxjs'; import { fromEvent, sampleTime } from 'rxjs'; import theme from './theme.scss?inline'; import { createCustomEvent } from '../../lib/custom-events.js'; @@ -133,9 +133,6 @@ export class MutationTestReportAppComponent extends RealTimeElement { } } - private mutants = new Map(); - private tests = new Map(); - public updated(changedProperties: PropertyValues) { if (changedProperties.has('theme') && this.theme) { this.dispatchEvent( @@ -163,29 +160,6 @@ export class MutationTestReportAppComponent extends RealTimeElement { private updateModel(report: MutationTestResult) { this.rootModel = calculateMutationTestMetrics(report); - collectForEach((file, metric) => { - file.result = metric; - file.mutants.forEach((mutant) => this.mutants.set(mutant.id, mutant)); - })(this.rootModel?.systemUnderTestMetrics); - - collectForEach((file, metric) => { - file.result = metric; - file.tests.forEach((test) => this.tests.set(test.id, test)); - })(this.rootModel?.testMetrics); - - this.rootModel.systemUnderTestMetrics.updateParent(); - this.rootModel.testMetrics?.updateParent(); - - function collectForEach(collect: (file: TFile, metrics: MetricsResult) => void) { - return function forEachMetric(metrics: MetricsResult | undefined): void { - if (metrics?.file) { - collect(metrics.file, metrics); - } - metrics?.childResults.forEach((child) => { - forEachMetric(child); - }); - }; - } } private updateContext() { @@ -225,18 +199,16 @@ export class MutationTestReportAppComponent extends RealTimeElement { public static styles = [globals, unsafeCSS(theme), tailwind]; - public readonly subscriptions: Subscription[] = []; + private readonly subscription = new Subscription(); + private readonly sseSubscriptions = new Set(); public connectedCallback() { super.connectedCallback(); - this.subscriptions.push(locationChange$.subscribe((path) => (this.path = path))); + this.subscription.add(locationChange$.subscribe((path) => (this.path = path))); this.initializeSse(); } private source: EventSource | undefined; - private sseSubscriptions = new Set(); - private theMutant?: MutantModel; - private theTest?: TestModel; private initializeSse() { if (!this.sse) { @@ -245,73 +217,34 @@ export class MutationTestReportAppComponent extends RealTimeElement { this.source = new EventSource(this.sse); - const modifySubscription = fromEvent(this.source, 'mutant-tested').subscribe((event) => { - const newMutantData = JSON.parse(event.data as string) as Partial & Pick; - if (!this.report) { - return; - } - - const mutant = this.mutants.get(newMutantData.id); - if (mutant === undefined) { - return; - } - this.theMutant = mutant; - - for (const [prop, val] of Object.entries(newMutantData)) { - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - (this.theMutant as any)[prop] = val; - } - - if (newMutantData.killedBy) { - newMutantData.killedBy.forEach((killedByTestId) => { - const test = this.tests.get(killedByTestId)!; - if (test === undefined) { - return; - } - this.theTest = test; - test.addKilled(this.theMutant!); - this.theMutant!.addKilledBy(test); - }); - } - - if (newMutantData.coveredBy) { - newMutantData.coveredBy.forEach((coveredByTestId) => { - const test = this.tests.get(coveredByTestId)!; - if (test === undefined) { - return; - } - this.theTest = test; - test.addCovered(this.theMutant!); - this.theMutant!.addCoveredBy(test); - }); - } - }); - - const applySubscription = fromEvent(this.source, 'mutant-tested') - .pipe(sampleTime(UPDATE_CYCLE_TIME)) - .subscribe(() => { - this.applyChanges(); - }); - - this.sseSubscriptions.add(modifySubscription); - this.sseSubscriptions.add(applySubscription); + this.sseSubscriptions.add( + fromEvent(this.source, 'mutant-tested').subscribe((event) => { + const newMutantData = JSON.parse(event.data as string) as Partial & Pick; + if (!this.report) { + return; + } + this.rootModel?.updateMutant(newMutantData); + }), + ); + + this.sseSubscriptions.add( + fromEvent(this.source, 'mutant-tested') + .pipe(sampleTime(UPDATE_CYCLE_TIME)) + .subscribe(() => { + mutantChanges.next(); + }), + ); this.source.addEventListener('finished', () => { this.source?.close(); - this.applyChanges(); this.sseSubscriptions.forEach((s) => s.unsubscribe()); }); } - private applyChanges() { - this.theMutant?.update(); - this.theTest?.update(); - mutantChanges.next(); - } - public disconnectedCallback() { super.disconnectedCallback(); - this.subscriptions.forEach((subscription) => subscription.unsubscribe()); + this.subscription.unsubscribe(); + this.sseSubscriptions.forEach((s) => s.unsubscribe()); } private renderTitle() { diff --git a/packages/elements/test/unit/components/app.component.spec.ts b/packages/elements/test/unit/components/app.component.spec.ts index efdf2a2cb..93e5db1c9 100644 --- a/packages/elements/test/unit/components/app.component.spec.ts +++ b/packages/elements/test/unit/components/app.component.spec.ts @@ -371,49 +371,6 @@ describe(MutationTestReportAppComponent.name, () => { expect(file.mutants[0].status).to.be.equal('Killed'); }); - it('should update every mutant field when given in an SSE event', async () => { - // Arrange - const report = createReport(); - const mutant = createMutantResult(); - mutant.status = 'Pending'; - report.files['foobar.js'].mutants = [mutant]; - sut.element.report = report; - sut.element.sse = 'http://localhost:8080/sse'; - sut.connect(); - await sut.whenStable(); - - // Act - const newMutantData = JSON.stringify({ - id: '1', - status: 'Killed', - description: 'test description', - coveredBy: ['test 1'], - duration: 100, - killedBy: ['test 1'], - replacement: 'test-r', - static: true, - statusReason: 'test reason', - testsCompleted: 12, - location: { start: { line: 12, column: 1 }, end: { line: 13, column: 2 } }, - mutatorName: 'test mutator', - }); - const message = new MessageEvent('mutant-tested', { data: newMutantData }); - eventSource.dispatchEvent(message); - - // Assert - const theMutant = sut.element.rootModel!.systemUnderTestMetrics.childResults[0].file!.mutants[0]; - expect(theMutant.description).to.be.equal('test description'); - expect(theMutant.coveredBy).to.have.same.members(['test 1']); - expect(theMutant.duration).to.be.equal(100); - expect(theMutant.killedBy).to.have.same.members(['test 1']); - expect(theMutant.replacement).to.be.equal('test-r'); - expect(theMutant.static).to.be.true; - expect(theMutant.statusReason).to.be.equal('test reason'); - expect(theMutant.testsCompleted).to.be.equal(12); - expect(theMutant.location).to.deep.equal({ start: { line: 12, column: 1 }, end: { line: 13, column: 2 } }); - expect(theMutant.mutatorName).to.be.equal('test mutator'); - }); - it('should close the SSE process when the final event comes in', async () => { // Arrange const message = new MessageEvent('finished', { data: '' }); diff --git a/packages/elements/test/unit/components/drawer-mutant.component.spec.ts b/packages/elements/test/unit/components/drawer-mutant.component.spec.ts index a11cdc71b..fde8e1019 100644 --- a/packages/elements/test/unit/components/drawer-mutant.component.spec.ts +++ b/packages/elements/test/unit/components/drawer-mutant.component.spec.ts @@ -53,17 +53,15 @@ describe(MutationTestReportDrawerMutant.name, () => { }); it('should render the first killedBy test', async () => { - mutant.killedByTests = [new TestModel(createTestDefinition({ name: 'foo should bar' }))]; + mutant.addKilledBy(new TestModel(createTestDefinition({ name: 'foo should bar' }))); sut.element.mutant = mutant; await sut.whenStable(); expect(summaryText()).contain('🎯 Killed by: foo should bar'); }); it('should mention more killedBy tests when they exist', async () => { - mutant.killedByTests = [ - new TestModel(createTestDefinition({ id: '1', name: 'foo should bar' })), - new TestModel(createTestDefinition({ id: '2' })), - ]; + mutant.addKilledBy(new TestModel(createTestDefinition({ id: '1', name: 'foo should bar' }))); + mutant.addKilledBy(new TestModel(createTestDefinition({ id: '2' }))); sut.element.mutant = mutant; await sut.whenStable(); expect(summaryText()).contain('(and 1 more)'); @@ -71,7 +69,8 @@ describe(MutationTestReportDrawerMutant.name, () => { it('should mention when one test covered the mutant', async () => { mutant.status = 'Killed'; - mutant.coveredByTests = [new TestModel(createTestDefinition())]; + mutant.addCoveredBy(new TestModel(createTestDefinition())); + sut.element.mutant = mutant; await sut.whenStable(); const summary = summaryText(); @@ -82,7 +81,7 @@ describe(MutationTestReportDrawerMutant.name, () => { it('should not mentioned that a killed mutant still survived', async () => { mutant.status = 'Killed'; - mutant.coveredByTests = [new TestModel(createTestDefinition())]; + mutant.addCoveredBy(new TestModel(createTestDefinition())); sut.element.mutant = mutant; await sut.whenStable(); const summary = summaryText(); @@ -90,7 +89,8 @@ describe(MutationTestReportDrawerMutant.name, () => { }); it('should mention when two tests covered the mutant', async () => { - mutant.coveredByTests = [new TestModel(createTestDefinition({ id: '1' })), new TestModel(createTestDefinition({ id: '2' }))]; + mutant.addCoveredBy(new TestModel(createTestDefinition({ id: '1' }))); + mutant.addCoveredBy(new TestModel(createTestDefinition({ id: '2' }))); sut.element.mutant = mutant; await sut.whenStable(); const summary = summaryText(); @@ -138,8 +138,11 @@ describe(MutationTestReportDrawerMutant.name, () => { it('should render the tests', async () => { const test1 = new TestModel(createTestDefinition({ id: '1', name: 'foo should bar' })); const test2 = new TestModel(createTestDefinition({ id: '2', name: 'baz should qux' })); - mutant.killedByTests = [test1, test2]; - mutant.coveredByTests = [test1, test2, new TestModel(createTestDefinition({ id: '3', name: 'quux should corge' }))]; + mutant.addKilledBy(test1); + mutant.addKilledBy(test2); + mutant.addCoveredBy(test1); + mutant.addCoveredBy(test2); + mutant.addCoveredBy(new TestModel(createTestDefinition({ id: '3', name: 'quux should corge' }))); sut.element.mutant = mutant; await sut.whenStable(); const listItems = sut.$$('[slot="detail"] ul li'); diff --git a/packages/elements/test/unit/helpers/factory.ts b/packages/elements/test/unit/helpers/factory.ts index 870bb76a9..da46c27cb 100644 --- a/packages/elements/test/unit/helpers/factory.ts +++ b/packages/elements/test/unit/helpers/factory.ts @@ -1,6 +1,7 @@ import type { Metrics, TestMetrics } from 'mutation-testing-metrics'; import { MetricsResult, TestFileModel } from 'mutation-testing-metrics'; import type { FileResult, Location, MutantResult, MutationTestResult, TestDefinition, TestFile } from 'mutation-testing-report-schema/api'; +import { accumulateFileUnderTestMetrics, accumulateTestMetrics, countFileUnderTestMetrics, countTestFileMetrics } from '../../../../metrics/src/calculateMetrics.js'; export function createMutantResult(overrides?: Partial): MutantResult { const defaults: MutantResult = { @@ -46,7 +47,7 @@ export function createFileResult(overrides?: Partial): FileResult { } export function createMetricsResult(overrides?: Partial): MetricsResult { - const result = new MetricsResult('foo', [], createMetrics()); + const result = new MetricsResult('foo', [], accumulateFileUnderTestMetrics, countFileUnderTestMetrics); if (overrides?.file) { result.file = overrides.file; } @@ -56,25 +57,18 @@ export function createMetricsResult(overrides?: Partial): Metrics if (overrides?.name) { result.name = overrides.name; } - if (overrides?.metrics) { - result.metrics = overrides.metrics; - } return result; } export function createTestMetricsResult(overrides?: Partial>): MetricsResult { - const result = new MetricsResult('foo', [], createTestMetrics(), new TestFileModel(createTestFile(), '')); + const result = new MetricsResult('foo', [], accumulateTestMetrics, countTestFileMetrics, overrides?.file ?? new TestFileModel(createTestFile(), '')); if (overrides?.childResults) { result.childResults = overrides.childResults; } if (overrides?.name) { result.name = overrides.name; } - if (overrides?.metrics) { - result.metrics = overrides.metrics; - } - return result; } diff --git a/packages/metrics/src/calculateMetrics.ts b/packages/metrics/src/calculateMetrics.ts index f58b09a61..b6732519b 100644 --- a/packages/metrics/src/calculateMetrics.ts +++ b/packages/metrics/src/calculateMetrics.ts @@ -1,7 +1,8 @@ import { compareNames, normalize, groupBy } from './helpers/index.js'; import type { FileResult, MutationTestResult } from 'mutation-testing-report-schema'; import type { MutantStatus } from 'mutation-testing-report-schema'; -import type { Metrics, MutantModel, MutationTestMetricsResult, TestMetrics, TestModel } from './model/index.js'; +import type { Metrics, MutantModel, TestMetrics, TestModel } from './model/index.js'; +import { MutationTestMetricsResult } from './model/index.js'; import { FileUnderTestModel, MetricsResult, TestFileModel } from './model/index.js'; import { TestStatus } from './model/test-model.js'; const DEFAULT_SCORE = NaN; @@ -15,7 +16,7 @@ const ROOT_NAME_TESTS = 'All tests'; */ export function calculateMetrics(files: Record): MetricsResult { const normalizedFiles = normalize(files, '', (input, name) => new FileUnderTestModel(input, name)); - return calculateDirectoryMetrics(ROOT_NAME, normalizedFiles, countFileMetrics); + return calculateDirectoryMetrics(ROOT_NAME, normalizedFiles, accumulateFileUnderTestMetrics, countFileUnderTestMetrics); } /** @@ -26,27 +27,25 @@ export function calculateMetrics(files: Record): MetricsResu export function calculateMutationTestMetrics(result: MutationTestResult): MutationTestMetricsResult { const { files, testFiles, projectRoot = '' } = result; const fileModelsUnderTest = normalize(files, projectRoot, (input, name) => new FileUnderTestModel(input, name)); + const systemUnderTestMetrics = calculateRootMetrics(ROOT_NAME, fileModelsUnderTest, accumulateFileUnderTestMetrics, countFileUnderTestMetrics); + if (testFiles && Object.keys(testFiles).length) { const testFileModels = normalize(testFiles, projectRoot, (input, name) => new TestFileModel(input, name)); relate( Object.values(fileModelsUnderTest).flatMap((file) => file.mutants), Object.values(testFileModels).flatMap((file) => file.tests), ); - return { - systemUnderTestMetrics: calculateRootMetrics(ROOT_NAME, fileModelsUnderTest, countFileMetrics), - testMetrics: calculateRootMetrics(ROOT_NAME_TESTS, testFileModels, countTestFileMetrics), - }; + const testMetrics = calculateRootMetrics(ROOT_NAME_TESTS, testFileModels, accumulateTestMetrics, countTestFileMetrics); + return new MutationTestMetricsResult(systemUnderTestMetrics, testMetrics); } - return { - systemUnderTestMetrics: calculateRootMetrics(ROOT_NAME, fileModelsUnderTest, countFileMetrics), - testMetrics: undefined, - }; + return new MutationTestMetricsResult(systemUnderTestMetrics, undefined); } function calculateRootMetrics( name: string, files: Record, - calculateMetrics: (files: TFileModel[]) => TMetrics, + accumulateMetrics: (metrics: TMetrics[]) => TMetrics, + toMetrics: (files: TFileModel) => TMetrics, ) { const fileNames = Object.keys(files); /** @@ -54,33 +53,35 @@ function calculateRootMetrics( * it will put those tests in a 'dummy' file with an empty string as name. */ if (fileNames.length === 1 && fileNames[0] === '') { - return calculateFileMetrics(name, files[fileNames[0]], calculateMetrics); + return calculateFileMetrics(name, files[fileNames[0]], accumulateMetrics, toMetrics); } else { - return calculateDirectoryMetrics(name, files, calculateMetrics); + return calculateDirectoryMetrics(name, files, accumulateMetrics, toMetrics); } } function calculateDirectoryMetrics( name: string, files: Record, - calculateMetrics: (files: TFileModel[]) => TMetrics, + accumulateMetrics: (metrics: TMetrics[]) => TMetrics, + toMetrics: (files: TFileModel) => TMetrics, ): MetricsResult { - const metrics = calculateMetrics(Object.values(files)); - const childResults = toChildModels(files, calculateMetrics); - return new MetricsResult(name, childResults, metrics); + const childResults = toChildModels(files, accumulateMetrics, toMetrics); + return new MetricsResult(name, childResults, accumulateMetrics, toMetrics); } export function calculateFileMetrics( fileName: string, file: TFileModel, - calculateMetrics: (files: TFileModel[]) => TMetrics, + accumulateMetrics: (metrics: TMetrics[]) => TMetrics, + toMetrics: (files: TFileModel) => TMetrics, ): MetricsResult { - return new MetricsResult(fileName, [], calculateMetrics([file]), file); + return new MetricsResult(fileName, [], accumulateMetrics, toMetrics, file); } function toChildModels( files: Record, - calculateMetrics: (files: TFileModel[]) => TMetrics, + accumulateMetrics: (metrics: TMetrics[]) => TMetrics, + toMetrics: (files: TFileModel) => TMetrics, ): MetricsResult[] { const filesByDirectory = groupBy(Object.entries(files), (namedFile) => namedFile[0].split('/')[0]); return Object.keys(filesByDirectory) @@ -88,10 +89,10 @@ function toChildModels( if (filesByDirectory[directoryName].length > 1 || filesByDirectory[directoryName][0][0] !== directoryName) { const directoryFiles: Record = {}; filesByDirectory[directoryName].forEach((file) => (directoryFiles[file[0].substr(directoryName.length + 1)] = file[1])); - return calculateDirectoryMetrics(directoryName, directoryFiles, calculateMetrics); + return calculateDirectoryMetrics(directoryName, directoryFiles, accumulateMetrics, toMetrics); } else { const [fileName, file] = filesByDirectory[directoryName][0]; - return calculateFileMetrics(fileName, file, calculateMetrics); + return calculateFileMetrics(fileName, file, accumulateMetrics, toMetrics); } }) .sort(compareNames); @@ -121,34 +122,73 @@ function relate(mutants: MutantModel[], tests: TestModel[]) { } } -export function countTestFileMetrics(testFile: TestFileModel[]): TestMetrics { - const tests = testFile.flatMap((_) => _.tests); - const count = (status: TestStatus) => tests.filter((_) => _.status === status).length; +export function countTestFileMetrics(testFile: TestFileModel): TestMetrics { + const count = (status: TestStatus) => testFile.tests.filter((_) => _.status === status).length; return { - total: tests.length, + total: testFile.tests.length, killing: count(TestStatus.Killing), covering: count(TestStatus.Covering), notCovering: count(TestStatus.NotCovering), }; } -export function countFileMetrics(fileResult: FileUnderTestModel[]): Metrics { - const mutants = fileResult.flatMap((_) => _.mutants); - const count = (status: MutantStatus) => mutants.filter((_) => _.status === status).length; - const pending = count('Pending'); - const killed = count('Killed'); - const timeout = count('Timeout'); - const survived = count('Survived'); - const noCoverage = count('NoCoverage'); - const runtimeErrors = count('RuntimeError'); - const compileErrors = count('CompileError'); - const ignored = count('Ignored'); +export function accumulateTestMetrics(metrics: readonly Readonly[]): Readonly { + if (metrics.length === 1) { + return metrics[0]; + } + const reduceMetricValue = (property: keyof TestMetrics) => metrics.reduce((previous, current) => previous + current[property], 0); + return { + total: reduceMetricValue('total'), + killing: reduceMetricValue('killing'), + covering: reduceMetricValue('covering'), + notCovering: reduceMetricValue('notCovering'), + }; +} + +type BaseMetrics = Pick; + +export function countFileUnderTestMetrics(fileResult: FileUnderTestModel): Metrics { + const count = (status: MutantStatus) => fileResult.mutants.filter((_) => _.status === status).length; + return deriveMetrics({ + pending: count('Pending'), + compileErrors: count('CompileError'), + ignored: count('Ignored'), + killed: count('Killed'), + noCoverage: count('NoCoverage'), + survived: count('Survived'), + timeout: count('Timeout'), + runtimeErrors: count('RuntimeError'), + }); +} + +export function accumulateFileUnderTestMetrics(metrics: readonly Readonly[]): Readonly { + if (metrics.length === 1) { + return metrics[0]; + } + + const reduceMetricValues = (property: keyof BaseMetrics) => metrics.reduce((previous, current) => previous + current[property], 0); + + const baseMetrics: BaseMetrics = { + pending: reduceMetricValues('pending'), + compileErrors: reduceMetricValues('compileErrors'), + ignored: reduceMetricValues('ignored'), + killed: reduceMetricValues('killed'), + survived: reduceMetricValues('survived'), + timeout: reduceMetricValues('timeout'), + runtimeErrors: reduceMetricValues('runtimeErrors'), + noCoverage: reduceMetricValues('noCoverage'), + }; + return deriveMetrics(baseMetrics); +} + +function deriveMetrics({ pending, compileErrors, ignored, killed, noCoverage, survived, timeout, runtimeErrors }: BaseMetrics) { const totalDetected = timeout + killed; const totalUndetected = survived + noCoverage; const totalCovered = totalDetected + survived; const totalValid = totalUndetected + totalDetected; const totalInvalid = runtimeErrors + compileErrors; + return { pending, killed, diff --git a/packages/metrics/src/index.ts b/packages/metrics/src/index.ts index 83dcd35d0..aa53d3a1d 100644 --- a/packages/metrics/src/index.ts +++ b/packages/metrics/src/index.ts @@ -2,4 +2,4 @@ export { calculateMetrics, calculateMutationTestMetrics } from './calculateMetri export { aggregateResultsByModule } from './aggregate.js'; export { normalizeFileNames } from './helpers/index.js'; export { MetricsResult, TestModel, FileUnderTestModel, TestFileModel, MutantModel, TestStatus } from './model/index.js'; -export type { Metrics, TestMetrics, MutationTestMetricsResult } from './model/index.js'; +export { type Metrics, type TestMetrics, MutationTestMetricsResult } from './model/index.js'; diff --git a/packages/metrics/src/model/index.ts b/packages/metrics/src/model/index.ts index a0c21784a..082d41cfe 100644 --- a/packages/metrics/src/model/index.ts +++ b/packages/metrics/src/model/index.ts @@ -2,7 +2,7 @@ export { FileUnderTestModel } from './file-under-test-model.js'; export { MetricsResult } from './metrics-result.js'; export type { Metrics } from './metrics.js'; export { MutantModel } from './mutant-model.js'; -export type { MutationTestMetricsResult } from './mutation-test-metrics-result.js'; +export { MutationTestMetricsResult } from './mutation-test-metrics-result.js'; export { TestFileModel } from './test-file-model.js'; export type { TestMetrics } from './test-metrics.js'; export { TestModel, TestStatus } from './test-model.js'; diff --git a/packages/metrics/src/model/metrics-result.ts b/packages/metrics/src/model/metrics-result.ts index a0d719ac7..eed6035ed 100644 --- a/packages/metrics/src/model/metrics-result.ts +++ b/packages/metrics/src/model/metrics-result.ts @@ -1,7 +1,6 @@ -import { countFileMetrics, countTestFileMetrics } from '../calculateMetrics.js'; +import { isNotNullish } from '../helpers/is-not-nullish.js'; import type { FileUnderTestModel } from './file-under-test-model.js'; import type { Metrics } from './metrics.js'; -import type { TestFileModel } from './test-file-model.js'; /** * A metrics result of T for a directory or file @@ -12,7 +11,7 @@ export class MetricsResult { /** * The parent of this result (if it has one) */ - parent: MetricsResult | undefined; + protected parent?: MetricsResult; /** * The name of this result */ @@ -25,72 +24,44 @@ export class MetricsResult { * The the child results */ childResults: MetricsResult[]; + /** + * The way to calculate metrics for this result + */ + #accumulateMetrics; + /** + * The way to calculate metrics from a file + */ + #toMetrics; /** * The actual metrics */ - metrics: TMetrics; + #metrics?: TMetrics; - constructor(name: string, childResults: MetricsResult[], metrics: TMetrics, file?: TFile) { + constructor( + name: string, + childResults: MetricsResult[], + accumulateMetrics: (metrics: TMetrics[]) => TMetrics, + toMetrics: (file: TFile) => TMetrics, + file?: TFile, + ) { this.name = name; this.childResults = childResults; - this.metrics = metrics; + this.#accumulateMetrics = accumulateMetrics; + this.#toMetrics = toMetrics; this.file = file; } - public updateParent(value?: MetricsResult) { - this.parent = value; - this.childResults.forEach((result) => result.updateParent(this)); - } - - public updateAllMetrics() { - if (this.parent !== undefined) { - this.parent.updateAllMetrics(); - return; - } - - this.updateMetrics(); + public invalidate() { + this.#metrics = undefined; + this.parent?.invalidate(); } - public updateMetrics() { - if (this.file === undefined) { - this.childResults.forEach((childResult) => { - childResult.updateMetrics(); - }); - - const files = this.#getFileModelsRecursively(this.childResults); - if (files.length === 0) { - return; - } - if ((files[0] as TestFileModel).tests) { - this.metrics = countTestFileMetrics(files as TestFileModel[]) as TMetrics; - } else { - this.metrics = countFileMetrics(files as FileUnderTestModel[]) as TMetrics; - } - - return; - } - - if ((this.file as TestFileModel).tests) { - this.metrics = countTestFileMetrics([this.file as TestFileModel]) as TMetrics; - } else { - this.metrics = countFileMetrics([this.file as FileUnderTestModel]) as TMetrics; + get metrics(): TMetrics { + if (this.#metrics === undefined) { + const myFileMetrics = this.file ? this.#toMetrics(this.file) : undefined; + this.#metrics = this.#accumulateMetrics([myFileMetrics, ...this.childResults.map((child) => child.metrics)].filter(isNotNullish)); } + return this.#metrics; } - #getFileModelsRecursively(childResults: MetricsResult[]): TFile[] { - const flattenedFiles: TFile[] = []; - if (childResults.length === 0) { - return flattenedFiles; - } - - childResults.forEach((child) => { - if (child.file) { - flattenedFiles.push(child.file); - return; - } - flattenedFiles.push(...this.#getFileModelsRecursively(child.childResults)); - }); - - return flattenedFiles; - } } diff --git a/packages/metrics/src/model/mutant-model.ts b/packages/metrics/src/model/mutant-model.ts index 9292447aa..8aa349f7f 100644 --- a/packages/metrics/src/model/mutant-model.ts +++ b/packages/metrics/src/model/mutant-model.ts @@ -13,12 +13,9 @@ function assertSourceFileDefined(sourceFile: FileUnderTestModel | undefined): as */ export class MutantModel implements MutantResult { // MutantResult properties - - public coveredBy: string[] | undefined; public description: string | undefined; public duration: number | undefined; public id: string; - public killedBy: string[] | undefined; public location: Location; public mutatorName: string; public replacement: string | undefined; @@ -27,35 +24,49 @@ export class MutantModel implements MutantResult { public statusReason: string | undefined; public testsCompleted: number | undefined; - // New fields + #coveredByTests?: TestModel[]; get coveredByTests(): TestModel[] | undefined { - if (this.#coveredByTests.size) { - return Array.from(this.#coveredByTests.values()); - } else return undefined; - } - set coveredByTests(tests: TestModel[]) { - this.#coveredByTests = new Map(tests.map((test) => [test.id, test])); + if (!this.#coveredByTests && this.#coveredByTestsById.size) { + this.#coveredByTests = Array.from(this.#coveredByTestsById.values()); + } + return this.#coveredByTests; } + #killedByTests?: TestModel[]; get killedByTests(): TestModel[] | undefined { - if (this.#killedByTests.size) { - return Array.from(this.#killedByTests.values()); - } else return undefined; + if (!this.#killedByTests && this.#killedByTestsById.size) { + this.#killedByTests = Array.from(this.#killedByTestsById.values()); + } + return this.#killedByTests; } - set killedByTests(tests: TestModel[]) { - this.#killedByTests = new Map(tests.map((test) => [test.id, test])); + + #coveredBy?: string[]; + public get coveredBy() { + if (!this.#coveredBy && this.#coveredByTestsById) { + this.#coveredBy = Array.from(this.#coveredByTestsById.keys()); + } + return this.#coveredBy; } + + #killedBy?: string[]; + public get killedBy() { + if (!this.#killedBy && this.#killedByTestsById) { + this.#killedBy = Array.from(this.#killedByTestsById.keys()); + } + return this.#killedBy; + } + public declare sourceFile: FileUnderTestModel | undefined; - #coveredByTests = new Map(); - #killedByTests = new Map(); + #coveredByTestsById = new Map(); + #killedByTestsById = new Map(); constructor(input: MutantResult) { - this.coveredBy = input.coveredBy; this.description = input.description; this.duration = input.duration; this.id = input.id; - this.killedBy = input.killedBy; + this.#coveredBy = input.coveredBy; + this.#killedBy = input.killedBy; this.location = input.location; this.mutatorName = input.mutatorName; this.replacement = input.replacement; @@ -66,11 +77,17 @@ export class MutantModel implements MutantResult { } public addCoveredBy(test: TestModel) { - this.#coveredByTests.set(test.id, test); + this.#coveredByTestsById.set(test.id, test); + // invalidate cache + this.#coveredBy = undefined; + this.#coveredByTests = undefined; } public addKilledBy(test: TestModel) { - this.#killedByTests.set(test.id, test); + this.#killedByTestsById.set(test.id, test); + // invalidate cache + this.#killedBy = undefined; + this.#killedByTests = undefined; } /** @@ -97,12 +114,4 @@ export class MutantModel implements MutantResult { assertSourceFileDefined(this.sourceFile); return this.sourceFile.name; } - - // TODO: https://github.com/stryker-mutator/mutation-testing-elements/pull/2453#discussion_r1178769871 - public update(): void { - if (!this.sourceFile?.result?.file) { - return; - } - this.sourceFile.result.updateAllMetrics(); - } } diff --git a/packages/metrics/src/model/mutation-test-metrics-result.ts b/packages/metrics/src/model/mutation-test-metrics-result.ts index 89b62fe78..481963dd5 100644 --- a/packages/metrics/src/model/mutation-test-metrics-result.ts +++ b/packages/metrics/src/model/mutation-test-metrics-result.ts @@ -3,8 +3,66 @@ import type { FileUnderTestModel } from './file-under-test-model.js'; import type { TestMetrics } from './test-metrics.js'; import type { Metrics } from './metrics.js'; import type { TestFileModel } from './test-file-model.js'; +import type { MutantModel } from './mutant-model.js'; +import type { TestModel } from './test-model.js'; +import type { MutantResult } from 'mutation-testing-report-schema'; -export interface MutationTestMetricsResult { - systemUnderTestMetrics: MetricsResult; - testMetrics: MetricsResult | undefined; +export class MutationTestMetricsResult { + systemUnderTestMetrics; + testMetrics; + + #mutants = new Map(); + #tests = new Map(); + + constructor( + systemUnderTestMetrics: MetricsResult, + testMetrics: MetricsResult | undefined, + ) { + this.systemUnderTestMetrics = systemUnderTestMetrics; + this.testMetrics = testMetrics; + collectForEach(this.systemUnderTestMetrics, ({ file }) => file?.mutants.forEach((m) => this.#mutants.set(m.id, m))); + collectForEach(this.testMetrics, ({ file }) => file?.tests.forEach((t) => this.#tests.set(t.id, t))); + } + + updateMutant(mutantUpdate: Partial & Pick) { + const mutant = this.#mutants.get(mutantUpdate.id); + if (!mutant) { + throw new Error(`Mutant with id "${mutantUpdate.id}" not found for update.`); + } + + if (mutantUpdate.killedBy) { + mutantUpdate.killedBy.forEach((killedByTestId) => { + const test = this.#tests.get(killedByTestId)!; + if (test === undefined) { + return; + } + test.addKilled(mutant); + mutant.addKilledBy(test); + }); + } + + if (mutantUpdate.coveredBy) { + mutantUpdate.coveredBy.forEach((coveredByTestId) => { + const test = this.#tests.get(coveredByTestId)!; + if (test === undefined) { + return; + } + test.addCovered(mutant); + mutant.addCoveredBy(test); + }); + } + mutant.status = mutantUpdate.status; + } +} + +function collectForEach( + metrics: MetricsResult | undefined, + collect: (result: MetricsResult) => void, +) { + if (metrics) { + collect(metrics); + metrics.childResults.forEach((child) => { + collectForEach(child, collect); + }); + } } diff --git a/packages/metrics/src/model/test-model.ts b/packages/metrics/src/model/test-model.ts index f3a7357ba..b0908a7c4 100644 --- a/packages/metrics/src/model/test-model.ts +++ b/packages/metrics/src/model/test-model.ts @@ -24,28 +24,36 @@ export class TestModel implements TestDefinition { public name!: string; public location?: OpenEndLocation | undefined; + #killedMutants?: MutantModel[]; get killedMutants() { - if (this.#killedMutants.size) { - return Array.from(this.#killedMutants.values()); - } else return undefined; + if (!this.#killedMutants && this.#killedMutantMyId.size) { + this.#killedMutants = Array.from(this.#killedMutantMyId.values()); + } + return this.#killedMutants; } + #coveredMutants?: MutantModel[]; get coveredMutants() { - if (this.#coveredMutants.size) { - return Array.from(this.#coveredMutants.values()); - } else return undefined; + if (!this.#coveredMutants && this.#coveredMutantById.size) { + this.#coveredMutants = Array.from(this.#coveredMutantById.values()); + } + return this.#coveredMutants; } public declare sourceFile: TestFileModel | undefined; - #killedMutants = new Map(); - #coveredMutants = new Map(); + #killedMutantMyId = new Map(); + #coveredMutantById = new Map(); public addCovered(mutant: MutantModel) { - this.#coveredMutants.set(mutant.id, mutant); + this.#coveredMutantById.set(mutant.id, mutant); + this.#coveredMutants = undefined; // invalidate cache + this.#invalidateMetrics(); } public addKilled(mutant: MutantModel) { - this.#killedMutants.set(mutant.id, mutant); + this.#killedMutantMyId.set(mutant.id, mutant); + this.#killedMutants = undefined; // invalidate cache + this.#invalidateMetrics(); } constructor(input: TestDefinition) { @@ -75,19 +83,23 @@ export class TestModel implements TestDefinition { } public get status(): TestStatus { - if (this.#killedMutants.size) { + if (this.#killedMutantMyId.size) { return TestStatus.Killing; - } else if (this.#coveredMutants.size) { + } else if (this.#coveredMutantById.size) { return TestStatus.Covering; } else { return TestStatus.NotCovering; } } - public update(): void { - if (!this.sourceFile?.result?.file) { - return; - } - this.sourceFile.result.updateAllMetrics(); + #invalidateMetrics() { + this.sourceFile?.result?.invalidate(); } + + // public update(): void { + // if (!this.sourceFile?.result?.file) { + // return; + // } + // this.sourceFile.result.updateAllMetrics(); + // } } diff --git a/packages/metrics/test/helpers/factories.ts b/packages/metrics/test/helpers/factories.ts index c78c8c87f..d8a4e6942 100644 --- a/packages/metrics/test/helpers/factories.ts +++ b/packages/metrics/test/helpers/factories.ts @@ -58,7 +58,7 @@ export function createFileResult(overrides?: Partial): FileResult { export function createTestDefinition(overrides?: Partial): TestDefinition { return { id: '52', - name: 'foo should be bar', + name: overrides?.id ?? 'foo should be bar', ...overrides, }; } diff --git a/packages/metrics/test/unit/calculateMetrics.spec.ts b/packages/metrics/test/unit/calculateMetrics.spec.ts index f88020446..2e940389b 100644 --- a/packages/metrics/test/unit/calculateMetrics.spec.ts +++ b/packages/metrics/test/unit/calculateMetrics.spec.ts @@ -1,7 +1,7 @@ import { expect } from 'chai'; import type { FileResultDictionary } from 'mutation-testing-report-schema'; import { calculateMetrics, calculateMutationTestMetrics } from '../../src/calculateMetrics.js'; -import type { FileUnderTestModel, Metrics, MetricsResult, TestMetrics } from '../../src/model/index.js'; +import type { MutationTestMetricsResult, TestMetrics } from '../../src/model/index.js'; import { TestFileModel } from '../../src/model/index.js'; import { createFileResult, createMutantResult, createMutationTestResult, createTestDefinition, createTestFile } from '../helpers/factories.js'; @@ -304,7 +304,13 @@ describe(calculateMutationTestMetrics.name, () => { const output = calculateMutationTestMetrics(input); // Assert - function collectFileNames(metricsResult: MetricsResult): string[] { + + interface NamedResult { + file?: { name: string }; + childResults: NamedResult[]; + } + + function collectFileNames(metricsResult: NamedResult): string[] { if (metricsResult.file) { return [metricsResult.file.name]; } else { diff --git a/packages/metrics/test/unit/model/mutant-model.spec.ts b/packages/metrics/test/unit/model/mutant-model.spec.ts index 76d8626c7..d91d1b73e 100644 --- a/packages/metrics/test/unit/model/mutant-model.spec.ts +++ b/packages/metrics/test/unit/model/mutant-model.spec.ts @@ -19,7 +19,10 @@ describe(MutantModel.name, () => { statusReason: 'Foo should have been "bar" but was "baz"', testsCompleted: 45, }; - expect(new MutantModel(mutantResult)).deep.eq(mutantResult); + const actual = new MutantModel(mutantResult); + Object.entries(mutantResult).forEach(([key, value]) => { + expect(actual[key as keyof MutantModel], `Field "${key}" not copied over`).deep.eq(value); + }); }); it('should initialize killedByTests and coveredByTests as undefined', () => { diff --git a/packages/metrics/test/unit/model/mutation-test-metrics-result.spec.ts b/packages/metrics/test/unit/model/mutation-test-metrics-result.spec.ts new file mode 100644 index 000000000..283ea1ac2 --- /dev/null +++ b/packages/metrics/test/unit/model/mutation-test-metrics-result.spec.ts @@ -0,0 +1,123 @@ +import { expect } from 'chai'; +import { calculateMutationTestMetrics, MutationTestMetricsResult, TestStatus } from '../../../src/index.js'; +import { createFileResult, createMutantResult, createMutationTestResult, createTestDefinition, createTestFile } from '../../helpers/factories.js'; + +describe(MutationTestMetricsResult.name, () => { + describe(MutationTestMetricsResult.prototype.updateMutant.name, () => { + let sut: MutationTestMetricsResult; + let mutant1; + + beforeEach(() => { + mutant1 = createMutantResult({ id: 'mutant1', status: 'Pending' }); + sut = calculateMutationTestMetrics( + createMutationTestResult({ + files: { + 'foo/bar.js': createFileResult({ + mutants: [mutant1, createMutantResult({ id: 'mutant2', status: 'Pending' })], + }), + 'foo/baz.js': createFileResult({ + mutants: [createMutantResult({ id: 'mutant3', status: 'Pending' }), createMutantResult({ id: 'mutant4', status: 'Pending' })], + }), + 'bar/qux.js': createFileResult({ + mutants: [createMutantResult({ id: 'mutant5', status: 'Pending' })], + }), + 'corge.js': createFileResult({ mutants: [createMutantResult({ id: 'mutant6', status: 'Pending' })] }), + }, + testFiles: { + 'foo/bar.spec.js': createTestFile({ tests: [createTestDefinition({ id: 'test-1' }), createTestDefinition({ id: 'test-2' })] }), + 'foo/baz.spec.js': createTestFile({ tests: [createTestDefinition({ id: 'test-3' })] }), + 'bar/qux.spec.js': createTestFile({ tests: [createTestDefinition({ id: 'test-4' })] }), + 'corge.spec.js': createTestFile({ tests: [createTestDefinition({ id: 'test-5' })] }), + }, + }), + ); + }); + + it('should update the mutant itself', () => { + // Arrange + const actualMutant = sut.systemUnderTestMetrics.childResults + .find(({ name }) => name === 'foo') + ?.childResults.find(({ name }) => name === 'bar.js') + ?.file?.mutants.find((m) => m.id === 'mutant1'); + + // Act + sut.updateMutant({ id: 'mutant1', status: 'Killed' }); + + // Assert + expect(actualMutant?.status).eq('Killed'); + }); + + it('should update the correlated tests', () => { + // Arrange + const actualSpecFile = sut.testMetrics?.childResults + .find(({ name }) => name === 'foo') + ?.childResults.find(({ name }) => name === 'bar.spec.js'); + const actualTest1 = actualSpecFile?.file?.tests.find((t) => t.id === 'test-1'); + const actualTest2 = actualSpecFile?.file?.tests.find((t) => t.id === 'test-2'); + + // Act + sut.updateMutant({ id: 'mutant1', status: 'Killed', coveredBy: ['test-1', 'test-2'], killedBy: ['test-2'] }); + + // Assert + expect(actualTest1?.status).eq(TestStatus.Covering); + expect(actualTest2?.status).eq(TestStatus.Killing); + }); + + it('should update the parent metrics', () => { + // Arrange + const actualFileResult = sut.systemUnderTestMetrics.childResults + .find(({ name }) => name === 'foo') + ?.childResults.find(({ name }) => name === 'bar.js'); + + // Act + sut.updateMutant({ id: 'mutant1', status: 'Killed' }); + sut.updateMutant({ id: 'mutant2', status: 'Survived' }); + + // Assert + expect(actualFileResult?.metrics.killed).eq(1); + expect(actualFileResult?.metrics.survived).eq(1); + expect(actualFileResult?.metrics.mutationScore).eq(50); + }) + + it('should update the root system under test metrics', () => { + // Act + sut.updateMutant({ id: 'mutant1', status: 'Killed' }); + sut.updateMutant({ id: 'mutant2', status: 'Survived' }); + + // Assert + expect(sut.systemUnderTestMetrics.metrics.killed).eq(1); + expect(sut.systemUnderTestMetrics.metrics.survived).eq(1); + expect(sut.systemUnderTestMetrics.metrics.mutationScore).eq(50); + }) + + it('should update the parent test metrics', () => { + // Arrange + const actualBarSpecFile = sut.testMetrics?.childResults + .find(({ name }) => name === 'foo') + ?.childResults.find(({ name }) => name === 'bar.spec.js'); + const actualQuxSpecFile = sut.testMetrics?.childResults + .find(({ name }) => name === 'bar') + ?.childResults.find(({ name }) => name === 'qux.spec.js'); + + // Act + sut.updateMutant({ id: 'mutant1', status: 'Killed', coveredBy: ['test-1', 'test-2', 'test-5'], killedBy: ['test-2'] }); + sut.updateMutant({ id: 'mutant2', status: 'Survived', coveredBy: ['test-3', 'test-4']}); + + // Assert + expect(actualBarSpecFile?.metrics.killing).eq(1); + expect(actualBarSpecFile?.metrics.covering).eq(1); + expect(actualQuxSpecFile?.metrics.covering).eq(1); + }) + + it('should update the root test metrics', () => { + // Act + sut.updateMutant({ id: 'mutant1', status: 'Killed', coveredBy: ['test-1', 'test-2', 'test-5'], killedBy: ['test-2'] }); + sut.updateMutant({ id: 'mutant2', status: 'Survived', coveredBy: ['test-3', 'test-4']}); + + // Assert + expect(sut.testMetrics?.metrics.killing).eq(1); + expect(sut.testMetrics?.metrics.covering).eq(4); + expect(sut.testMetrics?.metrics.total).eq(5); + }) + }); +}); diff --git a/packages/metrics/test/unit/model/test-model.spec.ts b/packages/metrics/test/unit/model/test-model.spec.ts index 1c7f9108d..6d291eb59 100644 --- a/packages/metrics/test/unit/model/test-model.spec.ts +++ b/packages/metrics/test/unit/model/test-model.spec.ts @@ -5,7 +5,7 @@ import { TestStatus } from '../../../src/model/test-model.js'; import { createLocation, createMutantResult, createTestDefinition, createTestFileModel } from '../../helpers/factories.js'; describe(TestModel.name, () => { - it('should copy over all values from mutant result', () => { + it('should copy over all values from test result', () => { const test: Required = { id: 'test1', location: createLocation(), diff --git a/workspace.code-workspace b/workspace.code-workspace index aa7314d6f..19caee62b 100644 --- a/workspace.code-workspace +++ b/workspace.code-workspace @@ -50,7 +50,16 @@ "eslint.rules.customizations": [{ "rule": "*", "severity": "warn" }], "typescript.tsdk": "./node_modules/typescript/lib", "task.autoDetect": "off", - "cSpell.words": ["autoloader", "bootswatch", "clazz", "clike", "prismjs", "templating", "Theming"], + "cSpell.words": [ + "autoloader", + "bootswatch", + "clazz", + "clike", + "corge", + "prismjs", + "templating", + "Theming" + ], "[json]": { "editor.defaultFormatter": "esbenp.prettier-vscode", },