Skip to content

Commit

Permalink
feat: release browser before firing any test finish event
Browse files Browse the repository at this point in the history
  • Loading branch information
@j0tunn
j0tunn committed Jul 24, 2019
1 parent 926b907 commit 876510a
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 50 deletions.
18 changes: 13 additions & 5 deletions lib/runner/test-runner/regular-test-runner.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,17 @@ module.exports = class RegularTestRunner extends Runner {

this._test = Object.create(test);
this._browserAgent = browserAgent;

this._browser = null;
}

async run(workers) {
let freeBrowserPromise;

workers.once(`worker.${this._test.id}.freeBrowser`, (browserState) => {
freeBrowserPromise = this._freeBrowser(browserState);
});

try {
await this._getBrowser();

Expand All @@ -35,7 +43,7 @@ module.exports = class RegularTestRunner extends Runner {

this._emit(Events.TEST_END);

await this._freeBrowser();
await freeBrowserPromise;
}

_emit(event) {
Expand All @@ -57,15 +65,13 @@ module.exports = class RegularTestRunner extends Runner {
);
}

_applyTestResults({meta, hermioneCtx = {}, browserState}) {
_applyTestResults({meta, hermioneCtx = {}}) {
hermioneCtx.assertViewResults = AssertViewResults.fromRawObject(hermioneCtx.assertViewResults || []);
this._test.assertViewResults = hermioneCtx.assertViewResults.get();

this._test.meta = _.extend(this._test.meta, meta);
this._test.hermioneCtx = hermioneCtx;

this._browser && this._browser.applyState(browserState);

this._test.duration = Date.now() - this._startTime;
}

Expand All @@ -78,11 +84,13 @@ module.exports = class RegularTestRunner extends Runner {
}
}

async _freeBrowser() {
async _freeBrowser(browserState) {
if (!this._browser) {
return;
}

this._browser.applyState(browserState);

try {
await this._browserAgent.freeBrowser(this._browser);
} catch (error) {
Expand Down
2 changes: 1 addition & 1 deletion lib/utils/ipc.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
const _ = require('lodash');

module.exports = {
emit: (event) => process.send({event}),
emit: (event, data = {}) => process.send({event, ...data}),
on: (event, baseHandler) => {
process.on('message', (...args) => {
if (event !== _.get(args[0], 'event')) {
Expand Down
4 changes: 3 additions & 1 deletion lib/worker/runner/test-runner/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const HookRunner = require('./hook-runner');
const ExecutionThread = require('./execution-thread');
const OneTimeScreenshooter = require('./one-time-screenshooter');
const AssertViewError = require('../../../browser/commands/assert-view/errors/assert-view-error');
const ipc = require('../../../utils/ipc');

module.exports = class TestRunner {
static create(...args) {
Expand Down Expand Up @@ -66,8 +67,9 @@ module.exports = class TestRunner {

hermioneCtx.assertViewResults = assertViewResults ? assertViewResults.toRawObject() : [];
const {meta, state: browserState} = browser;
const results = {hermioneCtx, meta, browserState};
const results = {hermioneCtx, meta};

ipc.emit(`worker.${this._test.id}.freeBrowser`, browserState);
this._browserAgent.freeBrowser(browser);

if (error) {
Expand Down
12 changes: 6 additions & 6 deletions package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions test/lib/_mocha/test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ module.exports = class Test extends Runnable {

this.type = 'test';
this.title = options.title || 'some-test';
this.id = options.id || 'some-id';
this.fn = options.fn || _.noop;
this.file = options.file || '';
this.pending = options.pending || false;
Expand Down
75 changes: 46 additions & 29 deletions test/lib/runner/test-runner/regular-test-runner.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const logger = require('lib/utils/logger');
const Events = require('lib/constants/runner-events');
const AssertViewResults = require('lib/browser/commands/assert-view/assert-view-results');
const Promise = require('bluebird');
const {EventEmitter} = require('events');

const {makeTest} = require('../../../utils');

Expand All @@ -24,9 +25,9 @@ describe('runner/test-runner/regular-test-runner', () => {
};

const mkWorkers_ = () => {
return {
return _.extend(new EventEmitter(), {
runTest: sandbox.stub().resolves(stubTestResult_())
};
});
};

const mkRunner_ = (opts = {}) => {
Expand Down Expand Up @@ -426,51 +427,49 @@ describe('runner/test-runner/regular-test-runner', () => {
});
});

describe('browser state', () => {
it('should apply browser state after test finished', async () => {
describe('freeBrowser', () => {
it('should release browser on related event from worker', async () => {
const browser = stubBrowser_();
BrowserAgent.prototype.getBrowser.resolves(browser);

const workers = mkWorkers_();
workers.runTest.resolves(stubTestResult_({
browserState: {isBroken: true}
}));
workers.runTest.callsFake((test) => {
workers.emit(`worker.${test.id}.freeBrowser`);
return stubTestResult_();
});

await run_({workers});

assert.calledOnceWith(browser.applyState, {isBroken: true});
assert.calledOnceWith(BrowserAgent.prototype.freeBrowser, browser);
});

it('should apply browser state on test fail', async () => {
const browser = stubBrowser_();
BrowserAgent.prototype.getBrowser.resolves(browser);

it('should release browser only once', async () => {
const workers = mkWorkers_();
workers.runTest.rejects({browserState: {isBroken: true}});
workers.runTest.callsFake((test) => {
workers.emit(`worker.${test.id}.freeBrowser`);
workers.emit(`worker.${test.id}.freeBrowser`);
return stubTestResult_();
});

await run_({workers});

assert.calledOnceWith(browser.applyState, {isBroken: true});
assert.calledOnce(BrowserAgent.prototype.freeBrowser);
});
});

describe('freeBrowser', () => {
it('should release browser after test finish', async () => {
const browser = stubBrowser_({id: 'bro'});
it('should apply browser state passed with free event before releasing browser', async () => {
const browser = stubBrowser_();
BrowserAgent.prototype.getBrowser.resolves(browser);

await run_();

assert.calledOnceWith(BrowserAgent.prototype.freeBrowser, browser);
});

it('should release browser even if test fails', async () => {
const workers = mkWorkers_();
workers.runTest.rejects();
workers.runTest.callsFake((test) => {
workers.emit(`worker.${test.id}.freeBrowser`, {foo: 'bar'});
return stubTestResult_();
});

await run_({workers});

assert.calledOnce(BrowserAgent.prototype.freeBrowser);
assert.calledOnceWith(browser.applyState, {foo: 'bar'});
assert.callOrder(browser.applyState, BrowserAgent.prototype.freeBrowser);
});

it('should wait until browser is released', async () => {
Expand All @@ -479,7 +478,13 @@ describe('runner/test-runner/regular-test-runner', () => {

BrowserAgent.prototype.freeBrowser.callsFake(() => Promise.delay(10).then(afterBrowserFree));

await run_();
const workers = mkWorkers_();
workers.runTest.callsFake((test) => {
workers.emit(`worker.${test.id}.freeBrowser`);
return stubTestResult_();
});

await run_({workers});
afterRun();

assert.callOrder(afterBrowserFree, afterRun);
Expand All @@ -488,7 +493,13 @@ describe('runner/test-runner/regular-test-runner', () => {
it('should not reject on browser release fail', async () => {
BrowserAgent.prototype.freeBrowser.rejects();

await assert.isFulfilled(run_());
const workers = mkWorkers_();
workers.runTest.callsFake((test) => {
workers.emit(`worker.${test.id}.freeBrowser`);
return stubTestResult_();
});

await assert.isFulfilled(run_({workers}));
});

it('should not fail test on browser release fail', async () => {
Expand All @@ -499,7 +510,13 @@ describe('runner/test-runner/regular-test-runner', () => {

BrowserAgent.prototype.freeBrowser.rejects();

await run_({runner});
const workers = mkWorkers_();
workers.runTest.callsFake((test) => {
workers.emit(`worker.${test.id}.freeBrowser`);
return stubTestResult_();
});

await run_({runner, workers}).catch((e) => e);

assert.notCalled(onTestFail);
});
Expand Down
41 changes: 33 additions & 8 deletions test/lib/worker/runner/test-runner/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const OneTimeScreenshooter = require('lib/worker/runner/test-runner/one-time-scr
const BrowserAgent = require('lib/worker/runner/browser-agent');
const AssertViewError = require('lib/browser/commands/assert-view/errors/assert-view-error');
const AssertViewResults = require('lib/browser/commands/assert-view/assert-view-results');
const ipc = require('lib/utils/ipc');
const {makeConfigStub} = require('../../../../utils');
const {Suite, Test} = require('../../../_mocha');

Expand Down Expand Up @@ -57,6 +58,8 @@ describe('worker/runner/test-runner', () => {
sandbox.stub(HookRunner.prototype, 'runAfterEachHooks').resolves();

sandbox.stub(OneTimeScreenshooter, 'create').returns(Object.create(OneTimeScreenshooter.prototype));

sandbox.stub(ipc, 'emit');
});

afterEach(() => sandbox.restore());
Expand Down Expand Up @@ -477,19 +480,17 @@ describe('worker/runner/test-runner', () => {
await assert.isRejected(run_(), AssertViewError);
});

it('should resolve with browser meta and state', async () => {
it('should resolve with browser meta', async () => {
ExecutionThread.create.callsFake(({browser}) => {
ExecutionThread.prototype.run.callsFake(() => {
browser.meta.foo = 'bar';
browser.state.baz = 'qux';
});
return Object.create(ExecutionThread.prototype);
});

const result = await run_();

assert.match(result.meta, {foo: 'bar'});
assert.match(result.browserState, {baz: 'qux'});
});

it('should release browser', async () => {
Expand All @@ -505,7 +506,6 @@ describe('worker/runner/test-runner', () => {
ExecutionThread.create.callsFake(({browser}) => {
ExecutionThread.prototype.run.callsFake(() => {
browser.meta.foo = 'bar';
browser.state.baz = 'qux';
});
return Object.create(ExecutionThread.prototype);
});
Expand All @@ -518,7 +518,15 @@ describe('worker/runner/test-runner', () => {
const result = await run_();

assert.match(result.meta, {foo: 'bar'});
assert.match(result.browserState, {baz: 'qux'});
});

it('should send test related freeBrowser event on browser release', async () => {
const test = mkTest_({id: 'foo'});
const runner = mkRunner_({test});

await run_({runner});

assert.calledOnceWith(ipc.emit, `worker.foo.freeBrowser`);
});
});

Expand Down Expand Up @@ -577,11 +585,10 @@ describe('worker/runner/test-runner', () => {
await assert.isRejected(run_(), 'runtime error');
});

it('should extend error with browser meta and state', async () => {
it('should extend error with browser meta', async () => {
ExecutionThread.create.callsFake(({browser}) => {
ExecutionThread.prototype.run.callsFake(() => {
browser.meta.foo = 'bar';
browser.state.baz = 'qux';

return Promise.reject(new Error());
});
Expand All @@ -592,7 +599,6 @@ describe('worker/runner/test-runner', () => {
const error = await run_().catch((e) => e);

assert.match(error.meta, {foo: 'bar'});
assert.match(error.browserState, {baz: 'qux'});
});

it('should release browser', async () => {
Expand All @@ -605,6 +611,25 @@ describe('worker/runner/test-runner', () => {

assert.calledOnceWith(BrowserAgent.prototype.freeBrowser, browser);
});

it('should send test related freeBrowser event on browser release', async () => {
const test = mkTest_({id: 'foo'});
const runner = mkRunner_({test});

ExecutionThread.create.callsFake(({browser}) => {
ExecutionThread.prototype.run.callsFake(() => {
browser.state.bar = 'baz';

return Promise.reject(new Error());
});

return Object.create(ExecutionThread.prototype);
});

await run_({runner}).catch((e) => e);

assert.calledOnceWith(ipc.emit, `worker.foo.freeBrowser`, {bar: 'baz'});
});
});
});
});

0 comments on commit 876510a

Please sign in to comment.