Skip to content

Commit

Permalink
Merge pull request #141 from gemini-testing/handling-of-before-all-ho…
Browse files Browse the repository at this point in the history 
…ok-failure

fix: handle 'before*' hooks failure correctly
  • Loading branch information
@eGavr
eGavr authored May 2, 2017
2 parents 2034fe2 + a0934ba commit ed5940c
Show file tree
Hide file tree
Showing 3 changed files with 151 additions and 28 deletions.
48 changes: 32 additions & 16 deletions lib/runner/mocha-runner/mocha-adapter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@ const EventEmitter = require('events').EventEmitter;
// Reason: each mocha runner sets 'uncaughtException' listener
process.setMaxListeners(0);

const isSkipped = (suite) => {
return _.every(suite.suites, (s) => isSkipped(s))
&& _.every(suite.tests, 'pending');
};

module.exports = class MochaAdapter extends EventEmitter {
static init() {
global.hermione = {};
Expand Down Expand Up @@ -51,9 +46,9 @@ module.exports = class MochaAdapter extends EventEmitter {

this._currentRunnable = null;

this._injectBrowser();
this._injectBeforeHookErrorHandling();
this._injectBeforeEachHookErrorHandling();
this._injectBrowser();
this._injectPretestFailVerification();
this._injectRunnableSpy();
this._injectSkip();
Expand Down Expand Up @@ -181,23 +176,22 @@ module.exports = class MochaAdapter extends EventEmitter {
}

_injectBeforeHookErrorHandling() {
this._injectHookErrorHandling('beforeAll', (error, hook) => {
hook.parent.fail = error;
hook.parent.eachTest((test) => test.fail = error);
});
this._injectHookErrorHandling('beforeAll', (error, hook) => markSuiteAsFailed(hook.parent, error));
}

_injectBeforeEachHookErrorHandling() {
this._injectHookErrorHandling('beforeEach', (error, hook) => {
hook.ctx.currentTest.fail = error;
});
this._injectHookErrorHandling('beforeEach', (error, hook) => markTestAsFailed(hook.ctx.currentTest, error));
}

_injectHookErrorHandling(event, onError) {
this._addEventHandler(event, this._overrideRunnableFn((hook, baseFn) => {
return function() {
return hook.parent.fail
? q.reject(hook.parent.fail)
const previousBeforeAllHookFail = hook.parent.fail;
const previousBeforeEachHookFail = _.get(hook, 'ctx.currentTest.fail');
const previousFail = previousBeforeAllHookFail || previousBeforeEachHookFail;

return previousFail
? onError(previousFail, hook)
: q(baseFn).apply(this, arguments).catch((error) => onError(error, hook));
};
}));
Expand Down Expand Up @@ -247,7 +241,7 @@ module.exports = class MochaAdapter extends EventEmitter {

_requestBrowser() {
if (isSkipped(this.suite)) {
return;
return q();
}

return this._browserAgent.getBrowser()
Expand All @@ -272,3 +266,25 @@ module.exports = class MochaAdapter extends EventEmitter {
return this._browser || {id: this._browserAgent.browserId};
}
};

function isSkipped(suite) {
return _.every(suite.suites, (s) => isSkipped(s))
&& _.every(suite.tests, 'pending');
}

function markSuiteAsFailed(suite, error) {
suite.fail = error;
eachSuiteAndTest(suite, (runnable) => runnable.fail = error);
}

function eachSuiteAndTest(runnable, cb) {
runnable.tests.forEach((test) => cb(test));
runnable.suites.forEach((suite) => {
cb(suite);
eachSuiteAndTest(suite, cb);
});
}

function markTestAsFailed(test, error) {
test.fail = error;
}
6 changes: 3 additions & 3 deletions test/lib/_mocha/suite.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,11 @@ module.exports = class Suite extends EventEmitter {
}

get beforeEachHooks() {
return this._beforeEach;
return this.parent ? this.parent.beforeEachHooks.concat(this._beforeEach) : this._beforeEach;
}

get afterEachHooks() {
return this._afterEach;
return this.parent ? this.parent.afterEachHooks.concat(this._afterEach) : this._afterEach;
}

get afterAllHooks() {
Expand Down Expand Up @@ -160,7 +160,7 @@ module.exports = class Suite extends EventEmitter {
.catch((error) => this.emit(EVENTS.FAIL, {error, test}))
.then(this._execRunnables(this.afterEachHooks));
}, q()))
.then(this._execRunnables(this.suites, []))
.then(this._execRunnables(this.suites))
.then(this._execRunnables(this.afterAllHooks));
}

Expand Down
125 changes: 116 additions & 9 deletions test/lib/runner/mocha-runner/mocha-adapter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,28 @@ describe('mocha-runner/mocha-adapter', () => {
.then(() => assert.calledOnce(browserAgent.getBrowser));
});

it('should fail all suite tests if requesting of a browser fails', () => {
const mochaAdapter = mkMochaAdapter_();
const testFailSpy = sinon.spy();
const error = new Error();

browserAgent.getBrowser.returns(q.reject(error));

MochaStub.lastInstance.updateSuiteTree((rootSuite) => {
return rootSuite
.addTest({title: 'first-test'})
.addSuite(MochaStub.Suite.create(rootSuite).addTest({title: 'second-test'}).onFail(testFailSpy))
.onFail(testFailSpy);
});

return mochaAdapter.run()
.then(() => {
assert.calledTwice(testFailSpy);
assert.calledWithMatch(testFailSpy, {error, test: {title: 'first-test'}});
assert.calledWithMatch(testFailSpy, {error, test: {title: 'second-test'}});
});
});

it('should not request browsers for suite with one skipped test', () => {
const mochaAdapter = mkMochaAdapter_();

Expand Down Expand Up @@ -585,19 +607,24 @@ describe('mocha-runner/mocha-adapter', () => {
.then(() => assert.notCalled(testCb));
});

it('should fail suite tests with "before" hook error', () => {
it('should fail all suite tests with "before" hook error', () => {
const error = new Error();
const testFailSpy = sinon.spy();

MochaStub.lastInstance.updateSuiteTree((suite) => {
return suite
MochaStub.lastInstance.updateSuiteTree((rootSuite) => {
return rootSuite
.beforeAll(() => q.reject(error))
.addTest({title: 'some-test'})
.addTest({title: 'first-test'})
.addSuite(MochaStub.Suite.create(rootSuite).addTest({title: 'second-test'}).onFail(testFailSpy))
.onFail(testFailSpy);
});

return mochaAdapter.run()
.then(() => assert.calledWithMatch(testFailSpy, {error, test: {title: 'some-test'}}));
.then(() => {
assert.calledTwice(testFailSpy);
assert.calledWithMatch(testFailSpy, {error, test: {title: 'first-test'}});
assert.calledWithMatch(testFailSpy, {error, test: {title: 'second-test'}});
});
});

it('should handle sync "before hook" errors', () => {
Expand All @@ -616,7 +643,7 @@ describe('mocha-runner/mocha-adapter', () => {
.then(() => assert.calledOnce(testFailSpy));
});

it('should not execute "before each" hook if "before" hook failed', () => {
it('should not execute "before each" hook if "before" hook failed at the same level', () => {
const beforeEachHookFn = sinon.spy();

MochaStub.lastInstance.updateSuiteTree((suite) => {
Expand All @@ -630,7 +657,64 @@ describe('mocha-runner/mocha-adapter', () => {
.then(() => assert.notCalled(beforeEachHookFn));
});

it('should fail test with error from "before" hook if before each hook was executed successfully', () => {
it('should not execute "before each" hook if "before" hook has already failed on a higher level', () => {
const beforeEachHookFn = sinon.spy();

MochaStub.lastInstance.updateSuiteTree((rootSuite) => {
const suite = MochaStub.Suite.create();

rootSuite
.beforeAll(() => q.reject(new Error()))
.addSuite(suite);

suite.beforeEach(beforeEachHookFn).addTest();

return rootSuite;
});

return mochaAdapter.run()
.then(() => assert.notCalled(beforeEachHookFn));
});

it('should not execute "before" hook if another one has already failed on a higher level', () => {
const beforeAllHookFn = sandbox.spy();

MochaStub.lastInstance.updateSuiteTree((rootSuite) => {
const suite = MochaStub.Suite.create();

rootSuite
.beforeAll(() => q.reject(new Error()))
.addSuite(suite);

suite.beforeAll(beforeAllHookFn).addTest();

return rootSuite;
});

return mochaAdapter.run()
.then(() => assert.notCalled(beforeAllHookFn));
});

it('should not execute "before each" hook if "before" hook has already failed on a lower level', () => {
const beforeEachHookFn = sandbox.spy();

MochaStub.lastInstance.updateSuiteTree((rootSuite) => {
const suite = MochaStub.Suite.create();

rootSuite
.beforeEach(beforeEachHookFn)
.addSuite(suite);

suite.beforeAll(() => q.reject(new Error())).addTest();

return rootSuite;
});

return mochaAdapter.run()
.then(() => assert.notCalled(beforeEachHookFn));
});

it('should fail suite tests with error from "before" hook if "before each" hook is present at the same level', () => {
const error = new Error();
const hookFailSpy = sinon.spy();

Expand All @@ -639,11 +723,15 @@ describe('mocha-runner/mocha-adapter', () => {
.beforeAll(() => q.reject(error))
.beforeEach(() => true)
.addTest()
.addTest()
.onFail(hookFailSpy);
});

return mochaAdapter.run()
.then(() => assert.calledWithMatch(hookFailSpy, {error}));
.then(() => {
assert.calledTwice(hookFailSpy);
assert.calledWithMatch(hookFailSpy, {error});
});
});
});

Expand Down Expand Up @@ -689,7 +777,7 @@ describe('mocha-runner/mocha-adapter', () => {

MochaStub.lastInstance.updateSuiteTree((suite) => {
return suite
.beforeEach(sandbox.stub().returns(q.reject(error)))
.beforeEach(() => q.reject(error))
.addTest({title: 'some-test'})
.onFail(testFailSpy);
});
Expand All @@ -713,5 +801,24 @@ describe('mocha-runner/mocha-adapter', () => {
return mochaAdapter.run()
.then(() => assert.calledOnce(testFailSpy));
});

it('should not execute "before each" hook if another one has already failed on a higher level', () => {
const beforeEachHookFn = sandbox.spy();

MochaStub.lastInstance.updateSuiteTree((rootSuite) => {
const suite = MochaStub.Suite.create(rootSuite);

rootSuite
.beforeEach(() => q.reject(new Error()))
.addSuite(suite);

suite.beforeEach(beforeEachHookFn).addTest();

return rootSuite;
});

return mochaAdapter.run()
.then(() => assert.notCalled(beforeEachHookFn));
});
});
});

0 comments on commit ed5940c

Please sign in to comment.