Skip to content

Commit

Permalink
fix(diff): last test generation (#3910)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@nicojs
nicojs authored Dec 18, 2022
1 parent 2d9e1a5 commit f88b038
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 17 deletions.
24 changes: 14 additions & 10 deletions packages/core/src/mutants/incremental-differ.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -576,5 +576,9 @@ function isClosed(test: Required<schema.TestDefinition>): 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 };
79 changes: 72 additions & 7 deletions packages/core/test/unit/mutants/incremental-differ.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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' () => {
Expand All @@ -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' () => {
Expand Down Expand Up @@ -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) });

Expand All @@ -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) }),
Expand Down Expand Up @@ -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);
});

Expand Down

0 comments on commit f88b038

Please sign in to comment.