Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(956) human readable estimates (#956) #1176

Merged
merged 12 commits into from
Oct 9, 2018
Merged
16 changes: 2 additions & 14 deletions packages/stryker/src/reporters/ProgressAppendOnlyReporter.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,13 @@
import { MatchedMutant } from 'stryker-api/report';
import * as os from 'os';
import ProgressKeeper from './ProgressKeeper';
import Timer from '../utils/Timer';

export default class ProgressAppendOnlyReporter extends ProgressKeeper {
private intervalReference: NodeJS.Timer;
private timer: Timer;

public onAllMutantsMatchedWithTests(matchedMutants: ReadonlyArray<MatchedMutant>): void {
super.onAllMutantsMatchedWithTests(matchedMutants);
if (matchedMutants.length) {
this.timer = new Timer();
this.intervalReference = setInterval(() => this.render(), 10000);
}
}
Expand All @@ -20,21 +17,12 @@ export default class ProgressAppendOnlyReporter extends ProgressKeeper {
}

private render() {
process.stdout.write(`Mutation testing ${this.percent()} (ETC ${this.etc()}) ` +
process.stdout.write(`Mutation testing ${this.getPercentDone()} (ETC ${this.getEtc()}) ` +
`${this.progress.tested}/${this.progress.total} tested (${this.progress.survived} survived)` +
os.EOL);
}

private percent() {
private getPercentDone() {
return Math.floor(this.progress.tested / this.progress.total * 100) + '%';
}

private etc() {
const etcSeconds = Math.floor(this.timer.elapsedSeconds() / this.progress.tested * (this.progress.total - this.progress.tested));
if (isFinite(etcSeconds)) {
return etcSeconds + 's';
} else {
return 'n/a';
}
}
}
24 changes: 22 additions & 2 deletions packages/stryker/src/reporters/ProgressKeeper.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
import { MatchedMutant, Reporter, MutantResult } from 'stryker-api/report';
import { MutantStatus } from 'stryker-api/report';
import Timer from '../utils/Timer';

abstract class ProgressKeeper implements Reporter {

private timer: Timer;
protected progress = {
survived: 0,
tested: 0,
total: 0
total: 0,
};

private mutantIdsWithoutCoverage: string[];

public onAllMutantsMatchedWithTests(matchedMutants: ReadonlyArray<MatchedMutant>): void {
this.timer = new Timer();
this.mutantIdsWithoutCoverage = matchedMutants.filter(m => m.scopedTestIds.length === 0).map(m => m.id);
this.progress.total = matchedMutants.length - this.mutantIdsWithoutCoverage.length;
}
Expand All @@ -24,5 +26,23 @@ abstract class ProgressKeeper implements Reporter {
this.progress.survived++;
}
}

protected getEtc() {
const timeLeft = Math.floor(this.timer.elapsedSeconds() / this.progress.tested * (this.progress.total - this.progress.tested));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should now be renamed to "totalSecondsLeft", as everything here has to do with "time left"


if (isFinite(timeLeft)) {
const hours = Math.floor(timeLeft / 3600);
const minutes = Math.floor((timeLeft / 60) - (hours * 60));
const seconds = Math.floor(timeLeft - (hours * 3600) - (minutes * 60));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplification suggestion:

const hours = Math.floor(totalSecondsLeft / 3600); 
const minutes = Math.floor(totalSecondsLeft / 60 % 60);
const seconds = Math.floor(totalSecondsLeft % 60);


let output = (hours > 0) ? `${hours}h, ` : '';
output += (minutes > 0) ? `${minutes}m, ` : '';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When hours is greater than 0, but minutes is 0 it means that minutes is left out. Though technically correct, i think it would be clearer if minutes is displayed. For example: 1h, 0m, 20s instead of 1h, 20s.

Suggestion:

if (hours > 0) {
  return `${hours}h, ${minutes}m, ${seconds}s`;
} else if(minutes > 0) {
  return `${minutes}m, ${seconds}s`;
} else {
  return `${seconds}s`
}

output += (seconds > 0) ? `${seconds}s` : 0;

return output;
} else {
return 'n/a';
}
}
}
export default ProgressKeeper;
17 changes: 10 additions & 7 deletions packages/stryker/src/reporters/ProgressReporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export default class ProgressBarReporter extends ProgressKeeper {
public onAllMutantsMatchedWithTests(matchedMutants: ReadonlyArray<MatchedMutant>): void {
super.onAllMutantsMatchedWithTests(matchedMutants);
const progressBarContent =
`Mutation testing [:bar] :percent (ETC :etas) :tested/:total tested (:survived survived)`;
sanderkoenders marked this conversation as resolved.
Show resolved Hide resolved
`Mutation testing [:bar] :percent (ETC :etc) :tested/:total tested (:survived survived)`;

this.progressBar = new ProgressBar(progressBarContent, {
complete: '=',
Expand All @@ -22,18 +22,21 @@ export default class ProgressBarReporter extends ProgressKeeper {
public onMutantTested(result: MutantResult): void {
const ticksBefore = this.progress.tested;
super.onMutantTested(result);

const progressBarContent = { ...this.progress, etc: this.getEtc() };

if (ticksBefore < this.progress.tested) {
this.tick();
this.tick(progressBarContent);
} else {
this.render();
this.render(progressBarContent);
}
}

private tick(): void {
this.progressBar.tick(this.progress);
private tick(tickObj: object): void {
this.progressBar.tick(tickObj);
}

private render(): void {
this.progressBar.render(this.progress);
private render(renderObj: object): void {
this.progressBar.render(renderObj);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ import { MutantStatus } from 'stryker-api/report';
import ProgressAppendOnlyReporter from '../../../src/reporters/ProgressAppendOnlyReporter';
import { matchedMutant, mutantResult } from '../../helpers/producers';

const SECOND = 1000;
const TEN_SECONDS = SECOND * 10;
const HUNDRED_SECONDS = SECOND * 100;
const TEN_THOUSAND_SECONDS = SECOND * 10000;

describe('ProgressAppendOnlyReporter', () => {
let sut: ProgressAppendOnlyReporter;
let sandbox: sinon.SinonSandbox;
Expand All @@ -13,7 +18,7 @@ describe('ProgressAppendOnlyReporter', () => {
sut = new ProgressAppendOnlyReporter();
sandbox = sinon.createSandbox();
sandbox.useFakeTimers();
sandbox.stub(process.stdout, 'write');
sandbox.spy(process.stdout, 'write');
});

afterEach(() => sandbox.restore());
Expand All @@ -28,17 +33,31 @@ describe('ProgressAppendOnlyReporter', () => {
expect(process.stdout.write).to.not.have.been.called;
});

it('should log zero progress after 10s without completed tests', () => {
sandbox.clock.tick(10000);
it('should log zero progress after ten seconds without completed tests', () => {
sandbox.clock.tick(TEN_SECONDS);
expect(process.stdout.write).to.have.been.calledWith(`Mutation testing 0% (ETC n/a) ` +
`0/2 tested (0 survived)${os.EOL}`);
});

it('should should log 50% with 10s ETC after 10s with 1 completed test', () => {
it('should log 50% with 10s ETC after ten seconds with 1 completed test', () => {
sut.onMutantTested(mutantResult({ status: MutantStatus.Killed }));
expect(process.stdout.write).to.not.have.been.called;
sandbox.clock.tick(10000);
sandbox.clock.tick(TEN_SECONDS);
expect(process.stdout.write).to.have.been.calledWith(`Mutation testing 50% (ETC 10s) 1/2 tested (0 survived)${os.EOL}`);
});

it('should log 50% with "1m, 40s" ETC after hundred seconds with 1 completed test', () => {
sut.onMutantTested(mutantResult({ status: MutantStatus.Killed }));
expect(process.stdout.write).to.not.have.been.called;
sandbox.clock.tick(HUNDRED_SECONDS);
expect(process.stdout.write).to.have.been.calledWith(`Mutation testing 50% (ETC 1m, 40s) 1/2 tested (0 survived)${os.EOL}`);
});

it('should log 50% with "2h, 46m, 40s" ETC after ten tousand seconds with 1 completed test', () => {
sut.onMutantTested(mutantResult({ status: MutantStatus.Killed }));
expect(process.stdout.write).to.not.have.been.called;
sandbox.clock.tick(TEN_THOUSAND_SECONDS);
expect(process.stdout.write).to.have.been.calledWith(`Mutation testing 50% (ETC 2h, 46m, 40s) 1/2 tested (0 survived)${os.EOL}`);
});
});
});
43 changes: 40 additions & 3 deletions packages/stryker/test/unit/reporters/ProgressReporterSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,25 @@ import * as progressBarModule from '../../../src/reporters/ProgressBar';
import { matchedMutant, mutantResult, Mock, mock } from '../../helpers/producers';
import ProgressBar = require('progress');

const SECOND = 1000;
const TEN_SECONDS = SECOND * 10;
const HUNDRED_SECONDS = SECOND * 100;
const TEN_THOUSAND_SECONDS = SECOND * 10000;

describe('ProgressReporter', () => {

let sut: ProgressReporter;
let sandbox: sinon.SinonSandbox;
let matchedMutants: MatchedMutant[];
let progressBar: Mock<ProgressBar>;
const progressBarContent: string =
`Mutation testing [:bar] :percent (ETC :etas) :tested/:total tested (:survived survived)`;
const progressBarContent: string = `Mutation testing [:bar] :percent (ETC :etc) :tested/:total tested (:survived survived)`;

beforeEach(() => {
sut = new ProgressReporter();
sandbox = sinon.createSandbox();
sandbox.useFakeTimers();

sut = new ProgressReporter();

progressBar = mock(ProgressBar);
sandbox.stub(progressBarModule, 'default').returns(progressBar);
});
Expand Down Expand Up @@ -78,4 +85,34 @@ describe('ProgressReporter', () => {
expect(progressBar.tick).to.have.been.calledWithMatch(progressBarTickTokens);
});
});

describe('ProgressBar estimate time', () => {
beforeEach(() => {
sut.onAllMutantsMatchedWithTests([matchedMutant(1), matchedMutant(1)]);
});

it('should show to an estimate of "10s" in the progressBar after ten seconds and 1 mutants tested', () => {
sandbox.clock.tick(TEN_SECONDS);

sut.onMutantTested(mutantResult({ status: MutantStatus.Killed }));

expect(progressBar.tick).to.have.been.calledWithMatch({ etc: '10s' });
});

it('should show to an estimate of "1m, 40s" in the progressBar after hundred seconds and 1 mutants tested', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See previous remark about leaving out minutes.

sandbox.clock.tick(HUNDRED_SECONDS);

sut.onMutantTested(mutantResult({ status: MutantStatus.Killed }));

expect(progressBar.tick).to.have.been.calledWithMatch({ etc: '1m, 40s' });
});

it('should show to an estimate of "2h, 46m, 40s" in the progressBar after ten thousand seconds and 1 mutants tested', () => {
sandbox.clock.tick(TEN_THOUSAND_SECONDS);

sut.onMutantTested(mutantResult({ status: MutantStatus.Killed }));

expect(progressBar.tick).to.have.been.calledWithMatch({ etc: '2h, 46m, 40s' });
});
});
});