From f88b03811001f2e393134c51b1603b315d892ecb Mon Sep 17 00:00:00 2001 From: Nico Jansen Date: Sun, 18 Dec 2022 12:22:16 +0100 Subject: [PATCH] fix(diff): last test generation (#3910) Fix a bug in the diffing algorithm where, running in `--incremental` mode, the test file would always be marked as 'changed' when it contained tests generated using a function, which was the last in the file. --- .../core/src/mutants/incremental-differ.ts | 24 +++--- .../unit/mutants/incremental-differ.spec.ts | 79 +++++++++++++++++-- 2 files changed, 86 insertions(+), 17 deletions(-) diff --git a/packages/core/src/mutants/incremental-differ.ts b/packages/core/src/mutants/incremental-differ.ts index 1065d6e688..db6ad89f18 100644 --- a/packages/core/src/mutants/incremental-differ.ts +++ b/packages/core/src/mutants/incremental-differ.ts @@ -530,24 +530,24 @@ function closeLocations(testFile: schema.TestFile): LocatedTest[] { if (openEndedTests.length) { // Sort the opened tests in order to close their locations openEndedTests.sort((a, b) => a.location.start.line - b.location.start.line); - + const openEndedTestSet = new Set(openEndedTests); const startPositions = uniqueStartPositions(openEndedTests); let currentPositionIndex = 0; - let currentPosition = startPositions[currentPositionIndex]; - openEndedTests.forEach((test) => { - if (currentPosition && test.location.start.line === currentPosition.line && test.location.start.column === currentPosition.column) { + openEndedTestSet.forEach((test) => { + if (eqPosition(test.location.start, startPositions[currentPositionIndex])) { currentPositionIndex++; - currentPosition = startPositions[currentPositionIndex]; } - if (currentPosition) { - locatedTests.push({ ...test, location: { start: test.location.start, end: currentPosition } }); + if (startPositions[currentPositionIndex]) { + locatedTests.push({ ...test, location: { start: test.location.start, end: startPositions[currentPositionIndex] } }); + openEndedTestSet.delete(test); } }); - // Don't forget about the last test - const lastTest = openEndedTests[openEndedTests.length - 1]; - locatedTests.push({ ...lastTest, location: { start: lastTest.location.start, end: { line: Number.POSITIVE_INFINITY, column: 0 } } }); + // Don't forget about the last tests + openEndedTestSet.forEach((lastTest) => { + locatedTests.push({ ...lastTest, location: { start: lastTest.location.start, end: { line: Number.POSITIVE_INFINITY, column: 0 } } }); + }); } return locatedTests; @@ -576,5 +576,9 @@ function isClosed(test: Required): test is LocatedTest { return !!test.location.end; } +function eqPosition(start: Position, end?: Position): boolean { + return start.column === end?.column && start.line === end.line; +} + type LocatedTest = schema.TestDefinition & { location: Location }; type OpenEndedTest = schema.TestDefinition & { location: schema.OpenEndLocation }; diff --git a/packages/core/test/unit/mutants/incremental-differ.spec.ts b/packages/core/test/unit/mutants/incremental-differ.spec.ts index c0e3ad9859..5ddaf50279 100644 --- a/packages/core/test/unit/mutants/incremental-differ.spec.ts +++ b/packages/core/test/unit/mutants/incremental-differ.spec.ts @@ -37,7 +37,7 @@ describe('add' () => { }); }); `; -const testAddContentWithTestGeneration = `import { expect } from 'chai'; +const testAddContentWithTestGenerationAndAdditionalTest = `import { expect } from 'chai'; import { add } from '../src/add.js'; describe('add' () => { @@ -52,7 +52,18 @@ describe('add' () => { }); `; -const testAddContentWithTestGenerationUpdated = `import { expect } from 'chai'; +const testAddContentWithTestGeneration = `import { add } from '../add.js'; + +test.each\` + x | y | expected + ${1} | ${1} | ${2} + ${1} | ${2} | ${3} + ${2} | ${2} | ${4} +\`("add($x, $y) = $expected", ({ x, y, expected }) => { + expect(add(x, y)).toBe(expected); +});`; + +const testAddContentWithTestGenerationAndAdditionalTestUpdated = `import { expect } from 'chai'; import { add } from '../src/add.js'; describe('add' () => { @@ -254,8 +265,35 @@ class ScenarioBuilder { return this; } - public withUpdatedTestGeneration(): this { - this.currentFiles.set(testAdd, testAddContentWithTestGenerationUpdated); + public withTestGeneration(): this { + this.currentFiles.set(testAdd, testAddContentWithTestGeneration); + const generateTest = (id: string, a: number, b: number, answer: number) => + factory.testResult({ id, fileName: testAdd, name: `add(${a}, ${b}) = ${answer}`, startPosition: pos(8, 3) }); + + this.testCoverage.clear(); + this.testCoverage.addTest(generateTest('spec1', 1, 1, 2), generateTest('spec2', 1, 2, 3), generateTest('spec3', 2, 2, 4)); + this.testCoverage.addCoverage(this.mutantId, ['spec1', 'spec2', 'spec3']); + return this; + } + + public withTestGenerationIncrementalReport(): this { + this.incrementalTestFiles[testAdd].source = testAddContentWithTestGeneration; + const generateTest = (id: string, a: number, b: number, answer: number) => + factory.mutationTestReportSchemaTestDefinition({ + id, + name: `add(${a}, ${b}) = ${answer}`, + location: loc(8, 3), + }); + // clear all tests + while (this.incrementalTestFiles[testAdd].tests.shift()) {} + this.incrementalTestFiles[testAdd].tests.push(generateTest('spec4', 1, 1, 2), generateTest('spec5', 1, 2, 3), generateTest('spec6', 2, 2, 4)); + this.incrementalFiles[srcAdd].mutants[0].coveredBy = ['spec4', 'spec5', 'spec6']; + this.incrementalFiles[srcAdd].mutants[0].killedBy = ['spec4']; + return this; + } + + public withTestGenerationAndAdditionalTest(): this { + this.currentFiles.set(testAdd, testAddContentWithTestGenerationAndAdditionalTest); const createAddWithTestGenerationTestResult = (a: number, b: number, answer: number) => factory.testResult({ id: `spec${a}`, fileName: testAdd, name: `should result in ${answer} for ${a} and ${b}`, startPosition: pos(5, 4) }); @@ -267,14 +305,30 @@ class ScenarioBuilder { this.testCoverage.addCoverage(this.mutantId, ['new-spec-2', gen1.id, gen2.id]); return this; } - public withTestGenerationIncrementalReport(): this { - this.incrementalTestFiles[testAdd].source = testAddContentWithTestGeneration; + + public withUpdatedTestGenerationAndAdditionalTest(): this { + this.currentFiles.set(testAdd, testAddContentWithTestGenerationAndAdditionalTestUpdated); + const createAddWithTestGenerationTestResult = (a: number, b: number, answer: number) => + factory.testResult({ id: `spec${a}`, fileName: testAdd, name: `should result in ${answer} for ${a} and ${b}`, startPosition: pos(5, 4) }); + + this.testCoverage.clear(); + this.testCoverage.addTest(factory.testResult({ id: 'new-spec-2', fileName: testAdd, name: 'should have name "add"', startPosition: pos(9, 2) })); + const gen1 = createAddWithTestGenerationTestResult(40, 2, 42); + const gen2 = createAddWithTestGenerationTestResult(45, -3, 42); + this.testCoverage.addTest(gen1, gen2); + this.testCoverage.addCoverage(this.mutantId, ['new-spec-2', gen1.id, gen2.id]); + return this; + } + + public withTestGenerationAndAdditionalTestIncrementalReport(): this { + this.incrementalTestFiles[testAdd].source = testAddContentWithTestGenerationAndAdditionalTest; const createAddWithTestGenerationTestDefinition = (id: string, a: number, b: number, answer: number) => factory.mutationTestReportSchemaTestDefinition({ id, name: `should result in ${answer} for ${a} and ${b}`, location: loc(5, 4), }); + // clear all tests while (this.incrementalTestFiles[testAdd].tests.shift()) {} this.incrementalTestFiles[testAdd].tests.push( factory.mutationTestReportSchemaTestDefinition({ id: 'spec3', name: 'should have name "add"', location: loc(9, 2) }), @@ -649,7 +703,18 @@ describe(IncrementalDiffer.name, () => { it('should close locations for tests on the same location in the incremental report', () => { // Test cases can generate tests, make sure the correct end position is chosen in those cases - const actualDiff = new ScenarioBuilder().withMathProjectExample().withUpdatedTestGeneration().withTestGenerationIncrementalReport().act(); + const actualDiff = new ScenarioBuilder() + .withMathProjectExample() + .withUpdatedTestGenerationAndAdditionalTest() + .withTestGenerationAndAdditionalTestIncrementalReport() + .act(); + expect(actualDiff[0].status).eq(MutantStatus.Killed); + }); + + // See #3909 + it('should close locations for tests on the same location in the incremental report when they are the last tests', () => { + // Test cases can generate tests, make sure the correct end position is chosen in those cases + const actualDiff = new ScenarioBuilder().withMathProjectExample().withTestGeneration().withTestGenerationIncrementalReport().act(); expect(actualDiff[0].status).eq(MutantStatus.Killed); });