Skip to content

Commit

Permalink
fix(progress reporter): improve ETC prediction (#4024)
Browse files Browse the repository at this point in the history
Improve Estimated Time to Completion (ETC) prediction by considering test runner capabilities. When a test runner can reload the test environment, it will no longer assume the test runner overhead cost is needed for static mutants.

Fixes #4018
  • Loading branch information
nicojs authored Mar 16, 2023
1 parent 006972d commit 956bbe9
Show file tree
Hide file tree
Showing 8 changed files with 129 additions and 71 deletions.
3 changes: 2 additions & 1 deletion packages/api/src/report/dry-run-completed-event.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { CompleteDryRunResult } from '../test-runner/index.js';
import { CompleteDryRunResult, TestRunnerCapabilities } from '../test-runner/index.js';

export interface DryRunCompletedEvent {
result: CompleteDryRunResult;
timing: RunTiming;
capabilities: TestRunnerCapabilities;
}

export interface RunTiming {
Expand Down
10 changes: 5 additions & 5 deletions packages/core/src/mutants/mutant-test-planner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { TestResult } from '@stryker-mutator/api/test-runner';
import { MutantRunPlan, MutantTestPlan, PlanKind, Mutant, StrykerOptions, MutantStatus, MutantEarlyResultPlan } from '@stryker-mutator/api/core';
import { commonTokens, tokens } from '@stryker-mutator/api/plugin';
import { Logger } from '@stryker-mutator/api/logging';
import { I, notEmpty } from '@stryker-mutator/util';
import { I, notEmpty, split } from '@stryker-mutator/util';

import { coreTokens } from '../di/index.js';
import { StrictReporter } from '../reporters/strict-reporter.js';
Expand Down Expand Up @@ -174,11 +174,11 @@ export class MutantTestPlanner {
const ABSOLUTE_CUT_OFF_PERUNAGE = 0.4;
const RELATIVE_CUT_OFF_FACTOR = 2;
const zeroIfNaN = (n: number) => (isNaN(n) ? 0 : n);
const totalNetTime = (runPlans: MutantRunPlan[]) => runPlans.reduce((acc, { netTime }) => acc + netTime, 0);
const runPlans = mutantPlans.filter(isRunPlan);
const staticRunPlans = runPlans.filter(({ mutant }) => mutant.static);
const runTimeRunPlans = runPlans.filter(({ mutant }) => !mutant.static);
const estimatedTimeForStaticMutants = staticRunPlans.reduce((acc, { netTime }) => acc + netTime, 0);
const estimatedTimeForRunTimeMutants = runTimeRunPlans.reduce((acc, { netTime }) => acc + netTime, 0);
const [staticRunPlans, runTimeRunPlans] = split(runPlans, ({ mutant }) => Boolean(mutant.static));
const estimatedTimeForStaticMutants = totalNetTime(staticRunPlans);
const estimatedTimeForRunTimeMutants = totalNetTime(runTimeRunPlans);
const estimatedTotalTime = estimatedTimeForRunTimeMutants + estimatedTimeForStaticMutants;
const avgTimeForAStaticMutant = zeroIfNaN(estimatedTimeForStaticMutants / staticRunPlans.length);
const avgTimeForARunTimeMutant = zeroIfNaN(estimatedTimeForRunTimeMutants / runTimeRunPlans.length);
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/process/3-dry-run-executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,12 @@ export class DryRunExecutor {
files: dryRunFiles,
});
const grossTimeMS = this.timer.elapsedMs(INITIAL_TEST_RUN_MARKER);
const capabilities = await testRunner.capabilities();
this.validateResultCompleted(result);

this.remapSandboxFilesToOriginalFiles(result);
const timing = this.calculateTiming(grossTimeMS, result.tests);
const dryRunCompleted = { result, timing };
const dryRunCompleted = { result, timing, capabilities };
this.reporter.onDryRunCompleted(dryRunCompleted);
return dryRunCompleted;
}
Expand Down
7 changes: 5 additions & 2 deletions packages/core/src/reporters/progress-keeper.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { MutantResult, MutantStatus, MutantRunPlan, MutantTestPlan, PlanKind } from '@stryker-mutator/api/core';
import { DryRunCompletedEvent, MutationTestingPlanReadyEvent, Reporter, RunTiming } from '@stryker-mutator/api/report';
import { TestRunnerCapabilities } from '@stryker-mutator/api/src/test-runner/test-runner-capabilities.js';

import { Timer } from '../utils/timer.js';

export abstract class ProgressKeeper implements Reporter {
private timer!: Timer;
private timing!: RunTiming;
private capabilities!: TestRunnerCapabilities;
private ticksByMutantId!: Map<string, number>;
protected progress = {
survived: 0,
Expand All @@ -16,8 +18,9 @@ export abstract class ProgressKeeper implements Reporter {
ticks: 0,
};

public onDryRunCompleted({ timing }: DryRunCompletedEvent): void {
public onDryRunCompleted({ timing, capabilities }: DryRunCompletedEvent): void {
this.timing = timing;
this.capabilities = capabilities;
}

/**
Expand All @@ -29,7 +32,7 @@ export abstract class ProgressKeeper implements Reporter {
this.ticksByMutantId = new Map(
mutantPlans.filter(isRunPlan).map(({ netTime, mutant, runOptions }) => {
let ticks = netTime;
if (runOptions.reloadEnvironment) {
if (!this.capabilities.reloadEnvironment && runOptions.reloadEnvironment) {
ticks += this.timing.overhead;
}
return [mutant.id, ticks];
Expand Down
13 changes: 12 additions & 1 deletion packages/core/test/unit/process/3-dry-run-executor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,14 @@ import { EOL } from 'os';
import { Injector } from 'typed-inject';
import { assertions, factory, testInjector } from '@stryker-mutator/test-helpers';
import sinon from 'sinon';
import { TestRunner, CompleteDryRunResult, ErrorDryRunResult, TimeoutDryRunResult, DryRunResult } from '@stryker-mutator/api/test-runner';
import {
TestRunner,
CompleteDryRunResult,
ErrorDryRunResult,
TimeoutDryRunResult,
DryRunResult,
TestRunnerCapabilities,
} from '@stryker-mutator/api/test-runner';
import { expect } from 'chai';
import { Observable, mergeMap } from 'rxjs';
import { I } from '@stryker-mutator/util';
Expand All @@ -22,6 +29,7 @@ import { FileSystemTestDouble } from '../../helpers/file-system-test-double.js';
describe(DryRunExecutor.name, () => {
let injectorMock: sinon.SinonStubbedInstance<Injector<MutationTestContext>>;
let testRunnerPoolMock: sinon.SinonStubbedInstance<I<Pool<TestRunner>>>;
let testRunnerCapabilities: TestRunnerCapabilities;
let sut: DryRunExecutor;
let timerMock: sinon.SinonStubbedInstance<Timer>;
let testRunnerMock: sinon.SinonStubbedInstance<Required<TestRunner>>;
Expand All @@ -33,7 +41,9 @@ describe(DryRunExecutor.name, () => {
beforeEach(() => {
reporterStub = factory.reporter();
timerMock = sinon.createStubInstance(Timer);
testRunnerCapabilities = factory.testRunnerCapabilities();
testRunnerMock = factory.testRunner();
testRunnerMock.capabilities.resolves(testRunnerCapabilities);
testRunnerPoolMock = createTestRunnerPoolMock();
(
testRunnerPoolMock.schedule as sinon.SinonStub<
Expand Down Expand Up @@ -170,6 +180,7 @@ describe(DryRunExecutor.name, () => {
overhead: expectedOverHeadTimeMs,
net: expectedNetTime,
},
capabilities: testRunnerCapabilities,
});
});

Expand Down
116 changes: 99 additions & 17 deletions packages/core/test/unit/reporters/progress-keeper.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { expect } from 'chai';
import { factory } from '@stryker-mutator/test-helpers';
import { MutantStatus } from 'mutation-testing-report-schema';

import { DryRunCompletedEvent, MutationTestingPlanReadyEvent } from '@stryker-mutator/api/src/report/index.js';

import { ProgressKeeper } from '../../../src/reporters/progress-keeper.js';

class TestProgressKeeper extends ProgressKeeper {
Expand All @@ -17,23 +19,103 @@ describe(ProgressKeeper.name, () => {
sut = new TestProgressKeeper();
});

it("should not count mutants that didn't run", () => {
// Arrange
sut.onDryRunCompleted(factory.dryRunCompletedEvent());
sut.onMutationTestingPlanReady(
factory.mutationTestingPlanReadyEvent({
mutantPlans: [
factory.mutantEarlyResultPlan({ mutant: factory.ignoredMutantTestCoverage({ id: '1' }) }),
factory.mutantRunPlan({ mutant: factory.mutantTestCoverage({ id: '2' }) }),
],
})
);

// Act
sut.onMutantTested(factory.mutantResult({ id: '1', status: MutantStatus.Survived }));

// Assert
expect(sut.progressForTesting.survived).eq(0);
describe('onMutationTestingPlanReady()', () => {
it("should not count mutants that didn't run", () => {
// Arrange
sut.onDryRunCompleted(factory.dryRunCompletedEvent());
sut.onMutationTestingPlanReady(
factory.mutationTestingPlanReadyEvent({
mutantPlans: [
factory.mutantEarlyResultPlan({ mutant: factory.ignoredMutantTestCoverage({ id: '1' }) }),
factory.mutantRunPlan({ mutant: factory.mutantTestCoverage({ id: '2' }) }),
],
})
);

// Act
sut.onMutantTested(factory.mutantResult({ id: '1', status: MutantStatus.Survived }));

// Assert
expect(sut.progressForTesting.survived).eq(0);
});

function arrangeEvents({
netTimeMutant1,
netTimeMutant2,
netTimeMutant3,
overhead,
reloadEnvironment,
}: {
netTimeMutant1: number;
netTimeMutant2: number;
netTimeMutant3: number;
overhead: number;
reloadEnvironment: boolean;
}): { dryRunCompleted: DryRunCompletedEvent; mutationTestingPlanReady: MutationTestingPlanReadyEvent } {
const test1 = '1';
const test2 = '2';
return {
dryRunCompleted: factory.dryRunCompletedEvent({
result: factory.completeDryRunResult({
tests: [factory.testResult({ id: test1, timeSpentMs: 10 }), factory.testResult({ id: test2, timeSpentMs: 5 })],
}),
timing: factory.runTiming({ net: 15, overhead }),
capabilities: factory.testRunnerCapabilities({ reloadEnvironment }),
}),
mutationTestingPlanReady: factory.mutationTestingPlanReadyEvent({
mutantPlans: [
// Ignored mutant
factory.mutantEarlyResultPlan({ mutant: factory.ignoredMutantTestCoverage({ id: '1' }) }),
// Run test 1, takes 10ms
factory.mutantRunPlan({
mutant: factory.mutantTestCoverage({ id: '2' }),
runOptions: factory.mutantRunOptions({ testFilter: [test1] }),
netTime: netTimeMutant1,
}),
// Run test 2, takes 5ms
factory.mutantRunPlan({
mutant: factory.mutantTestCoverage({ id: '3' }),
runOptions: factory.mutantRunOptions({ testFilter: [test2] }),
netTime: netTimeMutant2,
}),
// Run all tests, takes 115ms
factory.mutantRunPlan({
mutant: factory.mutantTestCoverage({ id: '4' }),
runOptions: factory.mutantRunOptions({ testFilter: undefined, reloadEnvironment: true }),
netTime: netTimeMutant3,
}),
],
}),
};
}

it('should calculate the correct total', () => {
const { dryRunCompleted, mutationTestingPlanReady } = arrangeEvents({
netTimeMutant1: 10,
netTimeMutant2: 5,
netTimeMutant3: 15,
overhead: 100,
reloadEnvironment: false,
});
sut.onDryRunCompleted(dryRunCompleted);
sut.onMutationTestingPlanReady(mutationTestingPlanReady);

expect(sut.progressForTesting.total).eq(130 /*115 + 5 + 10*/);
});

it('should not take overhead into account when the test runner is capable to reload the environment', () => {
const { dryRunCompleted, mutationTestingPlanReady } = arrangeEvents({
netTimeMutant1: 10,
netTimeMutant2: 5,
netTimeMutant3: 15,
overhead: 100,
reloadEnvironment: true,
});
sut.onDryRunCompleted(dryRunCompleted);
sut.onMutationTestingPlanReady(mutationTestingPlanReady);

expect(sut.progressForTesting.total).eq(30 /*15 + 5 + 10*/);
});
});

// Other tests still need to be moved from progress-reporter.spec.ts some day
Expand Down
44 changes: 0 additions & 44 deletions packages/core/test/unit/reporters/progress-reporter.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { expect } from 'chai';
import sinon from 'sinon';
import ProgressBar from 'progress';
import { factory } from '@stryker-mutator/test-helpers';
Expand All @@ -17,8 +16,6 @@ describe(ProgressBarReporter.name, () => {
let sut: ProgressBarReporter;
let progressBar: Mock<ProgressBar>;
let progressBarConstructorStub: sinon.SinonStub;
const progressBarContent =
'Mutation testing [:bar] :percent (elapsed: :et, remaining: :etc) :tested/:mutants Mutants tested (:survived survived, :timedOut timed out)';

beforeEach(() => {
sinon.useFakeTimers();
Expand All @@ -28,47 +25,6 @@ describe(ProgressBarReporter.name, () => {
progressBarConstructorStub.returns(progressBar);
});

describe('onMutationTestingPlanReady()', () => {
it('should calculate the correct total', () => {
sut.onDryRunCompleted(
factory.dryRunCompletedEvent({
result: factory.completeDryRunResult({
tests: [factory.testResult({ id: '1', timeSpentMs: 10 }), factory.testResult({ id: '2', timeSpentMs: 5 })],
}),
timing: factory.runTiming({ net: 15, overhead: 100 }),
})
);
sut.onMutationTestingPlanReady(
factory.mutationTestingPlanReadyEvent({
mutantPlans: [
// Ignored mutant
factory.mutantEarlyResultPlan({ mutant: factory.ignoredMutantTestCoverage({ id: '1' }) }),
// Run test 1, takes 10ms
factory.mutantRunPlan({
mutant: factory.mutantTestCoverage({ id: '2' }),
runOptions: factory.mutantRunOptions({ testFilter: ['1'] }),
netTime: 10,
}),
// Run test 2, takes 5ms
factory.mutantRunPlan({
mutant: factory.mutantTestCoverage({ id: '3' }),
runOptions: factory.mutantRunOptions({ testFilter: ['2'] }),
netTime: 5,
}),
// Run all tests, takes 115ms
factory.mutantRunPlan({
mutant: factory.mutantTestCoverage({ id: '4' }),
runOptions: factory.mutantRunOptions({ testFilter: undefined, reloadEnvironment: true }),
netTime: 15,
}),
],
})
);

expect(progressBarConstructorStub).calledWithMatch(progressBarContent, { total: 115 + 5 + 10 });
});
});

describe('onMutantTested()', () => {
let progressBarTickTokens: any;

Expand Down
4 changes: 4 additions & 0 deletions packages/test-helpers/src/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
ErrorMutantRunResult,
TestStatus,
TestResult,
TestRunnerCapabilities,
} from '@stryker-mutator/api/test-runner';
import { Checker, CheckResult, CheckStatus, FailedCheckResult } from '@stryker-mutator/api/check';

Expand Down Expand Up @@ -376,9 +377,12 @@ export const runTiming = factoryMethod<RunTiming>(() => ({
overhead: 765,
}));

export const testRunnerCapabilities = factoryMethod<TestRunnerCapabilities>(() => ({ reloadEnvironment: false }));

export const dryRunCompletedEvent = factoryMethod<DryRunCompletedEvent>(() => ({
result: completeDryRunResult(),
timing: runTiming(),
capabilities: testRunnerCapabilities(),
}));

export function injector<T = unknown>(): sinon.SinonStubbedInstance<Injector<T>> {
Expand Down

0 comments on commit 956bbe9

Please sign in to comment.