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.

Sign up for GitHub

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

Jump to bottom

Feat: release browser before firing any test finish event #426

Merged
merged 2 commits into from
Jul 24, 2019
Merged

Feat: release browser before firing any test finish event #426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
926b907
chore: ability to send custom events from workers
j0tunn Jul 24, 2019
876510a
feat: release browser before firing any test finish event
j0tunn Jul 24, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions lib/runner/test-runner/regular-test-runner.js
View file
Open in desktop
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
Open in desktop
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
15 changes: 12 additions & 3 deletions lib/utils/workers-registry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const {EventEmitter} = require('events');
const workerFarm = require('worker-farm');
const Promise = require('bluebird');
const _ = require('lodash');
const Events = require('../constants/runner-events');
const RuntimeConfig = require('../config/runtime-config');
const WorkerProcess = require('./worker-process');
Expand All @@ -17,6 +18,7 @@ module.exports = class WorkersRegistry extends EventEmitter {
this._config = config;
this._ended = false;
this._workerFarm = null;
this._eventBridge = new EventEmitter();
}

init() {
Expand All @@ -37,7 +39,8 @@ module.exports = class WorkersRegistry extends EventEmitter {
}

register(workerFilepath, exportedMethods) {
const workers = {};
const workers = Object.create(this._eventBridge);

for (const methodName of exportedMethods) {
workers[methodName] = (...args) => {
if (this._ended) {
Expand All @@ -46,6 +49,7 @@ module.exports = class WorkersRegistry extends EventEmitter {
return Promise.promisify(this._workerFarm)(workerFilepath, methodName, args);
};
}

return workers;
}

Expand Down Expand Up @@ -87,8 +91,8 @@ module.exports = class WorkersRegistry extends EventEmitter {
}

_initChild(child) {
child.on('message', ({event} = {}) => {
switch (event) {
child.on('message', (data = {}) => {
switch (data.event) {
case 'worker.init':
child.send({
event: 'master.init',
Expand All @@ -102,6 +106,11 @@ module.exports = class WorkersRegistry extends EventEmitter {
config: this._config.serialize()
});
break;
default:
if (data.event) {
this._eventBridge.emit(data.event, _.omit(data, 'event'));
}
break;
}
});

Expand Down
4 changes: 3 additions & 1 deletion lib/worker/runner/test-runner/index.js
View file
Open in desktop
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
Open in desktop

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
Open in desktop
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
Open in desktop
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
26 changes: 26 additions & 0 deletions test/lib/utils/workers-registry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,32 @@ describe('WorkersRegistry', () => {
config: {foo: 'bar'}
});
});

it('should emit other events through workers object', () => {
const workersRegistry = mkWorkersRegistry_();
const workers = workersRegistry.register(null, []);

const onEvent = sandbox.stub().named('onEvent');
workers.on('foo', onEvent);

const child = initChild_();
child.emit('message', {event: 'foo', bar: 'baz'});

assert.calledOnceWith(onEvent, {bar: 'baz'});
});

it('should not emit unknown events (without event field) through workers object', () => {
const workersRegistry = mkWorkersRegistry_();
const workers = workersRegistry.register(null, []);

const onEvent = sandbox.stub().named('onEvent');
workers.on('foo', onEvent);

const child = initChild_();
child.emit('message', {foo: 'bar'});

assert.notCalled(onEvent);
});
});

describe('execute worker\'s method', () => {
Expand Down
Loading