feat(956) human readable estimates (#956) #1176
Conversation
ghost
assigned 
| const minutes = Math.floor((timeLeft / 60) - (hours * 60)); | ||
| const seconds = Math.floor(timeLeft - (hours * 3600) - (minutes * 60)); | ||
|
|
||
| let output = (hours !== 0) ? `${hours}h, ` : ''; |
There was a problem hiding this comment.
I think it would be nicer to check for a positive number. If any of our inputs is negative, we will probably end up with a negative timeleft.
| @@ -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 = Object.assign(this.progress, { etc: this.getEtc() }); | |||
There was a problem hiding this comment.
You could also use the spread operator here: const progressBarContent = { ...this.progress, etc: this.getEtc() };
|
Cool PR. I would like to review before merging. 👍 |
|
I refactored a bit of the code to process Simons feedback. |
| 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)); |
There was a problem hiding this comment.
Simplification suggestion:
const hours = Math.floor(totalSecondsLeft / 3600);
const minutes = Math.floor(totalSecondsLeft / 60 % 60);
const seconds = Math.floor(totalSecondsLeft % 60);| @@ -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)); | |||
There was a problem hiding this comment.
I think this should now be renamed to "totalSecondsLeft", as everything here has to do with "time left"
| const seconds = Math.floor(timeLeft - (hours * 3600) - (minutes * 60)); | ||
|
|
||
| let output = (hours > 0) ? `${hours}h, ` : ''; | ||
| output += (minutes > 0) ? `${minutes}m, ` : ''; |
There was a problem hiding this comment.
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`
}| 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', () => { |
There was a problem hiding this comment.
See previous remark about leaving out minutes.
checks whether minutes and seconds is shown when there is exactly one hour left
|
@nicojs, should be good to go now. |
|
Awesome 👍 |
Adds human readable estimates to the progressbar during stryker testing.
resolves #956