Skip to content

Commit

Permalink
fix(Test runner): Don't crash on first failure (#1037)
Browse files Browse the repository at this point in the history
Make the `RetryDecorator` (a decorator for test runners) attempt a retry for all rejections
  • Loading branch information
simondel authored and nicojs committed Jul 23, 2018
1 parent 9385b94 commit 94790c3
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 36 deletions.
28 changes: 11 additions & 17 deletions packages/stryker/src/isolated-runner/RetryDecorator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,24 +33,18 @@ export default class RetryDecorator extends TestRunnerDecorator {
return isErrnoException(error) && error.code === BROKEN_PIPE_ERROR_CODE;
}

private tryRun(options: RunOptions, retriesLeft = 2, lastError?: any) {
if (retriesLeft > 0) {
this.innerRunner.run(options).then(result =>
this.currentRunTask.resolve(result),
rejectReason => {
if (this.innerProcessIsCrashed(rejectReason)) {
this.recover().then(
() => this.tryRun(options, retriesLeft - 1, rejectReason),
reason => this.currentRunTask.reject(reason));
} else {
// Oops... not intended to catch this one
this.currentRunTask.reject(rejectReason);
}
});
private async tryRun(options: RunOptions, attemptsLeft = 2, lastError?: any) {
if (attemptsLeft > 0) {
try {
let result = await this.innerRunner.run(options);
this.currentRunTask.resolve(result);
} catch (error) {
await this.recover();
await this.tryRun(options, attemptsLeft - 1, error);
}
} else {
this.recover().then(
() => this.currentRunTask.resolve({ status: RunStatus.Error, errorMessages: [ERROR_MESSAGE + errorToString(lastError)], tests: [] }),
(reason) => this.currentRunTask.reject(reason));
await this.recover();
this.currentRunTask.resolve({ status: RunStatus.Error, errorMessages: [ERROR_MESSAGE + errorToString(lastError)], tests: [] });
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,25 +63,25 @@ describe('ResilientTestRunnerFactory integration', function () {
}

it('should be able to receive a regex', async () => {
sut = arrangeSut('discover-regex');
arrangeSut('discover-regex');
const result = await actRun();
expect(result.status).eq(RunStatus.Complete);
});

it('should pass along the coverage result from the test runner behind', async () => {
sut = arrangeSut('coverage-reporting');
arrangeSut('coverage-reporting');
const result = await actRun();
expect(result.coverage).eq('realCoverage');
});

it('should pass along the run result', async () => {
sut = arrangeSut('direct-resolved');
arrangeSut('direct-resolved');
const result = await actRun();
expect(result.status).eq(RunStatus.Complete);
});

it('should try to report coverage from the global scope, even when the test runner behind does not', async () => {
sut = arrangeSut('direct-resolved');
arrangeSut('direct-resolved');
const result = await actRun();
expect(result.coverage).eq('coverageObject');
});
Expand Down Expand Up @@ -118,7 +118,7 @@ describe('ResilientTestRunnerFactory integration', function () {
});

it('should be able to run twice in quick succession', async () => {
sut = arrangeSut('direct-resolved');
arrangeSut('direct-resolved');
await actRun();
const result = await actRun();
expect(RunStatus[result.status]).eq(RunStatus[RunStatus.Complete]);
Expand All @@ -130,14 +130,14 @@ describe('ResilientTestRunnerFactory integration', function () {
});

it('should change the current working directory to the sandbox directory', async () => {
sut = arrangeSut('verify-working-folder');
arrangeSut('verify-working-folder');
const result = await actRun();
expect(result.errorMessages).undefined;
});

it('should be able to recover from any crash', async () => {
// time-bomb will crash after 100 ms
sut = arrangeSut('time-bomb');
arrangeSut('time-bomb');
await sleep(101);
const result = await actRun();
expect(RunStatus[result.status]).eq(RunStatus[RunStatus.Complete]);
Expand Down
20 changes: 8 additions & 12 deletions packages/stryker/test/unit/isolated-runner/RetryDecoratorSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,26 +60,22 @@ describe('RetryDecorator', () => {
return expect(result).to.eventually.eq(expectedResult);
});

it('should pass through non-crash related rejects', () => {
it('should retry on a new test runner if a reject appears', () => {
testRunner1.run.rejects(new Error('Error'));
return expect(sut.run(options)).to.be.rejectedWith('Error');
});

it('should retry on a new test runner if a crash related reject appears', () => {
testRunner1.run.rejects(brokenPipeError);
testRunner2.run.resolves(expectedResult);
return expect(sut.run(options)).to.eventually.eq(expectedResult);
});

it('should retry at most 3 times before rejecting', () => {
testRunner1.run.rejects(brokenPipeError);
testRunner2.run.rejects(brokenPipeError);
testRunner3.run.rejects(brokenPipeError);
testRunner4.run.rejects(brokenPipeError);
it('should retry at most 1 times before rejecting', () => {
const finalError = new Error('Error');

testRunner1.run.rejects(new Error('Error'));
testRunner2.run.rejects(finalError);

return sut.run(options).then((runResult: RunResult) => {
expect(runResult.status).to.be.eq(RunStatus.Error);
expect(runResult.errorMessages).to.be.deep.eq(['Test runner crashed. Tried twice to restart it without any luck. Last time the error message was: '
+ errorToString(brokenPipeError)]);
+ errorToString(finalError)]);
expect(availableTestRunners).to.have.lengthOf(0);
});
});
Expand Down

0 comments on commit 94790c3

Please sign in to comment.