Skip to content

Commit

Permalink
feat(mutant-matcher): lower memory usage (#1794)
Browse files Browse the repository at this point in the history
Don't fill an array with all test references
when all tests need to run for a mutant.
Instead, use the `runAllTests` flag on a mutant.
  • Loading branch information
nicojs authored and simondel committed Oct 22, 2019
1 parent 3405c0e commit 16294e5
Show file tree
Hide file tree
Showing 13 changed files with 134 additions and 53 deletions.
22 changes: 22 additions & 0 deletions packages/api/src/report/MatchedMutant.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,31 @@
interface MatchedMutant {
/**
* The ID of the mutant
*/
readonly id: string;
/**
* The Mutator name
*/
readonly mutatorName: string;
/**
* Whether or not all tests will run for this mutant
*/
readonly runAllTests: boolean;
/**
* If not all tests will run for this mutant, this array will contain the ids of the tests that will run.
*/
readonly scopedTestIds: number[];
/**
* The time spent on the tests that will run in initial test run
*/
readonly timeSpentScopedTests: number;
/**
* The file name that contains the mutant
*/
readonly fileName: string;
/**
* The replacement code to be inserted
*/
readonly replacement: string;
}

Expand Down
12 changes: 6 additions & 6 deletions packages/core/src/Sandbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,13 @@ export default class Sandbox {
`Transpile error occurred: "${transpiledMutant.transpileResult.error}" during transpiling of mutant ${transpiledMutant.mutant.toString()}`
);
}
const result = transpiledMutant.mutant.result(MutantStatus.TranspileError, []);
const result = transpiledMutant.mutant.createResult(MutantStatus.TranspileError, []);
return result;
} else if (!transpiledMutant.mutant.selectedTests.length) {
const result = transpiledMutant.mutant.result(MutantStatus.NoCoverage, []);
} else if (!transpiledMutant.mutant.runAllTests && !transpiledMutant.mutant.selectedTests.length) {
const result = transpiledMutant.mutant.createResult(MutantStatus.NoCoverage, []);
return result;
} else if (!transpiledMutant.changedAnyTranspiledFiles) {
const result = transpiledMutant.mutant.result(MutantStatus.Survived, []);
const result = transpiledMutant.mutant.createResult(MutantStatus.Survived, []);
return result;
} else {
// No early result possible, need to run in the sandbox later
Expand All @@ -115,7 +115,7 @@ export default class Sandbox {
const error = runResult.errorMessages ? runResult.errorMessages.toString() : '(undefined)';
this.log.debug('A runtime error occurred: %s during execution of mutant: %s', error, mutant.toString());
}
return mutant.result(status, testNames);
return mutant.createResult(status, testNames);
}

private determineMutantState(runResult: RunResult): MutantStatus {
Expand Down Expand Up @@ -195,7 +195,7 @@ export default class Sandbox {
}

private getFilterTestsHooks(mutant: TestableMutant): string | undefined {
if (this.testFramework) {
if (this.testFramework && !mutant.runAllTests) {
return wrapInClosure(this.testFramework.filter(mutant.selectedTests));
} else {
return undefined;
Expand Down
35 changes: 27 additions & 8 deletions packages/core/src/TestableMutant.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,39 @@ export enum TestSelectionResult {
Success
}

class TestFilter {
public timeSpentScopedTests = 0;
public runAllTests = false;
public selectedTests: TestSelection[] = [];

public selectAllTests(runResult: RunResult) {
this.timeSpentScopedTests = runResult.tests.reduce((time, test) => time + test.timeSpentMs, this.timeSpentScopedTests);
this.runAllTests = true;
}

public selectTest(testResult: TestResult, index: number) {
this.selectedTests.push({ id: index, name: testResult.name });
this.timeSpentScopedTests += testResult.timeSpentMs;
this.runAllTests = false;
}
}

export default class TestableMutant {
private readonly _selectedTests: TestSelection[] = [];
public specsRan: string[] = [];
private _timeSpentScopedTests = 0;
private readonly filter = new TestFilter();
private _location: Location;
public testSelectionResult = TestSelectionResult.Success;

public get selectedTests(): TestSelection[] {
return this._selectedTests;
return this.filter.selectedTests;
}

public get runAllTests(): boolean {
return this.filter.runAllTests;
}

public get timeSpentScopedTests() {
return this._timeSpentScopedTests;
return this.filter.timeSpentScopedTests;
}

public get fileName() {
Expand Down Expand Up @@ -59,13 +79,12 @@ export default class TestableMutant {
}

public selectAllTests(runResult: RunResult, testSelectionResult: TestSelectionResult) {
this.filter.selectAllTests(runResult);
this.testSelectionResult = testSelectionResult;
runResult.tests.forEach((testResult, id) => this.selectTest(testResult, id));
}

public selectTest(testResult: TestResult, index: number) {
this._selectedTests.push({ id: index, name: testResult.name });
this._timeSpentScopedTests += testResult.timeSpentMs;
this.filter.selectTest(testResult, index);
}

constructor(public readonly id: string, public mutant: Mutant, public sourceFile: SourceFile) {}
Expand Down Expand Up @@ -96,7 +115,7 @@ export default class TestableMutant {
return [startIndexLines, endIndexLines];
}

public result(status: MutantStatus, testsRan: string[]): MutantResult {
public createResult(status: MutantStatus, testsRan: string[]): MutantResult {
return freezeRecursively({
id: this.id,
location: this.location,
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/mutants/MutantTestMatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { Mutant } from '@stryker-mutator/api/mutant';
import { commonTokens, tokens } from '@stryker-mutator/api/plugin';
import { MatchedMutant } from '@stryker-mutator/api/report';
import { CoverageCollection, CoveragePerTestResult, CoverageResult, StatementMap } from '@stryker-mutator/api/test_runner';
import * as _ from 'lodash';
import { coreTokens } from '../di';
import InputFileCollection from '../input/InputFileCollection';
import { InitialTestRunResult } from '../process/InitialTestExecutor';
Expand Down Expand Up @@ -143,14 +142,15 @@ export class MutantTestMatcher {
* @returns The MatchedMutant
*/
private mapMutantOnMatchedMutant(testableMutant: TestableMutant): MatchedMutant {
const matchedMutant = _.cloneDeep({
const matchedMutant: MatchedMutant = {
fileName: testableMutant.mutant.fileName,
id: testableMutant.id,
mutatorName: testableMutant.mutant.mutatorName,
replacement: testableMutant.mutant.replacement,
runAllTests: testableMutant.runAllTests,
scopedTestIds: testableMutant.selectedTests.map(testSelection => testSelection.id),
timeSpentScopedTests: testableMutant.timeSpentScopedTests
});
};
return Object.freeze(matchedMutant);
}

Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/reporters/ProgressKeeper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ abstract class ProgressKeeper implements Reporter {

public onAllMutantsMatchedWithTests(matchedMutants: readonly MatchedMutant[]): void {
this.timer = new Timer();
this.mutantIdsWithoutCoverage = matchedMutants.filter(m => m.scopedTestIds.length === 0).map(m => m.id);
this.mutantIdsWithoutCoverage = matchedMutants.filter(m => !m.runAllTests && m.scopedTestIds.length === 0).map(m => m.id);
this.progress.total = matchedMutants.length - this.mutantIdsWithoutCoverage.length;
}

Expand Down
8 changes: 0 additions & 8 deletions packages/core/src/utils/objectUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,6 @@ export function filterEmpty<T>(input: Array<T | null | void>) {
return input.filter(item => item !== undefined && item !== null) as T[];
}

export function copy<T>(obj: T, deep?: boolean) {
if (deep) {
return _.cloneDeep(obj);
} else {
return _.clone(obj);
}
}

export function wrapInClosure(codeFragment: string) {
return `
(function (window) {
Expand Down
34 changes: 34 additions & 0 deletions packages/core/test/unit/Sandbox.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,28 @@ describe(Sandbox.name, () => {
it('should not filter any tests when testFramework = null', async () => {
const sut = await createSut();
const mutant = new TestableMutant('2', createMutant(), new SourceFile(new File('', '')));
await sut.runMutant(new TranspiledMutant(mutant, { outputFiles: [new File(expectedTargetFileToMutate, '')], error: null }, true));
expect(fileUtils.writeFile).not.calledWith(expectedTestFrameworkHooksFile);
});

it('should not filter any tests when runAllTests = true', async () => {
// Arrange
while (transpiledMutant.mutant.selectedTests.pop());
transpiledMutant.mutant.selectAllTests(runResult, TestSelectionResult.Success);
const sut = await createSut();

// Act
await sut.runMutant(transpiledMutant);

// Assert
expect(fileUtils.writeFile).not.calledWith(expectedTestFrameworkHooksFile);
expect(testRunner.run).called;
});

it('should not filter any tests when runAllTests = true', async () => {
const sut = await createSut();
const mutant = new TestableMutant('2', createMutant(), new SourceFile(new File('', '')));
mutant.selectAllTests(runResult, TestSelectionResult.Failed);
sut.runMutant(new TranspiledMutant(mutant, { outputFiles: [new File(expectedTargetFileToMutate, '')], error: null }, true));
expect(fileUtils.writeFile).not.calledWith(expectedTestFrameworkHooksFile);
});
Expand Down Expand Up @@ -281,6 +303,18 @@ describe(Sandbox.name, () => {
expect(mutantResult.status).eq(MutantStatus.TranspileError);
});

it('should report an early result when there are no files scoped', async () => {
// Arrange
while (transpiledMutant.mutant.selectedTests.pop());

// Act
const sut = await createSut();

// Assert
const mutantResult = await sut.runMutant(transpiledMutant);
expect(mutantResult.status).eq(MutantStatus.NoCoverage);
});

it('should report an early result when there are no file changes', async () => {
transpiledMutant.changedAnyTranspiledFiles = false;
const sut = await createSut();
Expand Down
13 changes: 11 additions & 2 deletions packages/core/test/unit/TestableMutant.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { expect } from 'chai';
import SourceFile from '../../src/SourceFile';
import TestableMutant, { TestSelectionResult } from '../../src/TestableMutant';

describe('TestableMutant', () => {
describe(TestableMutant.name, () => {
let innerMutant: Mutant;

beforeEach(() => {
Expand All @@ -29,10 +29,19 @@ describe('TestableMutant', () => {
TestSelectionResult.FailedButAlreadyReported
);
expect(sut.timeSpentScopedTests).eq(54);
expect(sut.selectedTests).deep.eq([{ id: 0, name: 'spec1' }, { id: 1, name: 'spec2' }]);
expect(sut.runAllTests).true;
expect(sut.testSelectionResult).eq(TestSelectionResult.FailedButAlreadyReported);
});

it('should scope tests when selecting individual tests', () => {
const sut = new TestableMutant('3', innerMutant, new SourceFile(new File('foobar.js', 'alert("foobar")')));
sut.selectTest(testResult({ name: 'spec1', timeSpentMs: 12 }), 0);
sut.selectTest(testResult({ name: 'spec3', timeSpentMs: 32 }), 2);
expect(sut.timeSpentScopedTests).eq(44);
expect(sut.selectedTests).deep.eq([{ id: 0, name: 'spec1' }, { id: 2, name: 'spec3' }]);
expect(sut.testSelectionResult).eq(TestSelectionResult.Success);
});

it('should calculate position using sourceFile', () => {
const sut = new TestableMutant('3', innerMutant, new SourceFile(new File('foobar.js', 'some content')));
innerMutant.range = [1, 2];
Expand Down
36 changes: 15 additions & 21 deletions packages/core/test/unit/mutants/MutantTestMatcher.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,11 @@ describe(MutantTestMatcher.name, () => {
});

describe('without code coverage info', () => {
it('should add both tests to the mutants and report failure', async () => {
const expectedTestSelection = [{ id: 0, name: 'test one' }, { id: 1, name: 'test two' }];

it('should run all tests for the mutants and report failure', async () => {
const result = await sut.matchWithMutants(mutants);

expect(result[0].selectedTests).deep.eq(expectedTestSelection);
expect(result[1].selectedTests).deep.eq(expectedTestSelection);
expect(result[0].runAllTests).true;
expect(result[1].runAllTests).true;
expect(TestSelectionResult[result[0].testSelectionResult]).eq(TestSelectionResult[TestSelectionResult.FailedButAlreadyReported]);
expect(TestSelectionResult[result[1].testSelectionResult]).eq(TestSelectionResult[TestSelectionResult.FailedButAlreadyReported]);
expect(testInjector.logger.warn).calledWith(
Expand All @@ -113,15 +111,17 @@ describe(MutantTestMatcher.name, () => {
id: '0',
mutatorName: result[0].mutatorName,
replacement: result[0].replacement,
scopedTestIds: result[0].selectedTests.map(test => test.id),
runAllTests: true,
scopedTestIds: [],
timeSpentScopedTests: result[0].timeSpentScopedTests
},
{
fileName: result[1].fileName,
id: '1',
mutatorName: result[1].mutatorName,
replacement: result[1].replacement,
scopedTestIds: result[1].selectedTests.map(test => test.id),
runAllTests: true,
scopedTestIds: [],
timeSpentScopedTests: result[1].timeSpentScopedTests
}
];
Expand Down Expand Up @@ -254,12 +254,10 @@ describe(MutantTestMatcher.name, () => {
});

it('should select all test in the test run but not report the error yet', async () => {
const expectedTestSelection: TestSelection[] = [{ name: 'test one', id: 0 }, { name: 'test two', id: 1 }];

const result = await sut.matchWithMutants(mutants);

expect(result[0].selectedTests).deep.eq(expectedTestSelection);
expect(result[1].selectedTests).deep.eq(expectedTestSelection);
expect(result[0].runAllTests).true;
expect(result[1].runAllTests).true;
expect(result[0].testSelectionResult).eq(TestSelectionResult.Failed);
expect(result[1].testSelectionResult).eq(TestSelectionResult.Failed);
expect(testInjector.logger.warn).not.called;
Expand Down Expand Up @@ -315,12 +313,10 @@ describe(MutantTestMatcher.name, () => {
});

it('should add all test results to the mutant that is covered by the baseline', async () => {
const expectedTestSelection = [{ id: 0, name: 'test one' }, { id: 1, name: 'test two' }];

const result = await sut.matchWithMutants(mutants);

expect(result[0].selectedTests).deep.eq(expectedTestSelection);
expect(result[1].selectedTests).deep.eq(expectedTestSelection);
expect(result[0].runAllTests).true;
expect(result[1].runAllTests).true;
});
});
});
Expand Down Expand Up @@ -458,12 +454,11 @@ describe(MutantTestMatcher.name, () => {
it('should match all mutants to all tests and log a warning when there is no coverage data', async () => {
mutants.push(mutant({ fileName: 'fileWithMutantOne' }), mutant({ fileName: 'fileWithMutantTwo' }));
initialRunResult.runResult.tests.push(testResult(), testResult());
const expectedTestSelection: TestSelection[] = [{ id: 0, name: 'name' }, { id: 1, name: 'name' }];

const result = await sut.matchWithMutants(mutants);

expect(result[0].selectedTests).deep.eq(expectedTestSelection);
expect(result[1].selectedTests).deep.eq(expectedTestSelection);
expect(result[0].runAllTests).true;
expect(result[1].runAllTests).true;
expect(result[0].testSelectionResult).deep.eq(TestSelectionResult.FailedButAlreadyReported);
expect(result[1].testSelectionResult).deep.eq(TestSelectionResult.FailedButAlreadyReported);
expect(testInjector.logger.warn).to.have.been.calledWith(
Expand Down Expand Up @@ -537,12 +532,11 @@ describe(MutantTestMatcher.name, () => {
it('should match all mutants to all tests', async () => {
mutants.push(mutant({ fileName: 'fileWithMutantOne' }), mutant({ fileName: 'fileWithMutantTwo' }));
initialRunResult.runResult.tests.push(testResult(), testResult());
const expectedTestSelection = [{ id: 0, name: 'name' }, { id: 1, name: 'name' }];

const result = await sut.matchWithMutants(mutants);

expect(result[0].selectedTests).deep.eq(expectedTestSelection);
expect(result[1].selectedTests).deep.eq(expectedTestSelection);
expect(result[0].runAllTests).true;
expect(result[1].runAllTests).true;
});
});

Expand Down
11 changes: 11 additions & 0 deletions packages/core/test/unit/reporters/ProgressReporter.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,17 @@ describe('ProgressReporter', () => {
sut.onAllMutantsMatchedWithTests(matchedMutants);
});

it('the total of MatchedMutants in the progress bar should be 2', () => {
expect(progressBarModule.default).to.have.been.calledWithMatch(progressBarContent, { total: 2 });
});
});
describe('when mutants match to all tests', () => {
beforeEach(() => {
matchedMutants = [matchedMutant(0, '0', true), matchedMutant(0, '1', true)];

sut.onAllMutantsMatchedWithTests(matchedMutants);
});

it('the total of MatchedMutants in the progress bar should be 2', () => {
expect(progressBarModule.default).to.have.been.calledWithMatch(progressBarContent, { total: 2 });
});
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/unit/utils/TemporaryDirectory.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ describe(TemporaryDirectory.name, () => {
describe('dispose', () => {
describe('when temp directory is initialized', () => {
beforeEach(() => sut.initialize());
it('should call deleteDir fileApi', () => {
it('should call deleteDir fileApi', async () => {
const expectedPath = path.resolve(tempDirName);
deleteDirStub.resolves('delResolveStub');

Expand Down
Loading

0 comments on commit 16294e5

Please sign in to comment.