From 926b907bfbffe948afe673f84cd21a2d15b1076f Mon Sep 17 00:00:00 2001 From: Anton Usmansky Date: Wed, 24 Jul 2019 13:18:01 +0300 Subject: [PATCH 1/2] chore: ability to send custom events from workers --- lib/utils/workers-registry.js | 15 ++++++++++++--- test/lib/utils/workers-registry.js | 26 ++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/lib/utils/workers-registry.js b/lib/utils/workers-registry.js index b2004f537..dfc037726 100644 --- a/lib/utils/workers-registry.js +++ b/lib/utils/workers-registry.js @@ -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'); @@ -17,6 +18,7 @@ module.exports = class WorkersRegistry extends EventEmitter { this._config = config; this._ended = false; this._workerFarm = null; + this._eventBridge = new EventEmitter(); } init() { @@ -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) { @@ -46,6 +49,7 @@ module.exports = class WorkersRegistry extends EventEmitter { return Promise.promisify(this._workerFarm)(workerFilepath, methodName, args); }; } + return workers; } @@ -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', @@ -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; } }); diff --git a/test/lib/utils/workers-registry.js b/test/lib/utils/workers-registry.js index 17d5f4909..e32545ebe 100644 --- a/test/lib/utils/workers-registry.js +++ b/test/lib/utils/workers-registry.js @@ -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', () => { From 876510a2c046360f02eb5ea831ae64cca1e337db Mon Sep 17 00:00:00 2001 From: Anton Usmansky Date: Wed, 24 Jul 2019 15:50:27 +0300 Subject: [PATCH 2/2] feat: release browser before firing any test finish event --- lib/runner/test-runner/regular-test-runner.js | 18 +++-- lib/utils/ipc.js | 2 +- lib/worker/runner/test-runner/index.js | 4 +- package-lock.json | 12 +-- test/lib/_mocha/test.js | 1 + .../runner/test-runner/regular-test-runner.js | 75 ++++++++++++------- test/lib/worker/runner/test-runner/index.js | 41 ++++++++-- 7 files changed, 103 insertions(+), 50 deletions(-) diff --git a/lib/runner/test-runner/regular-test-runner.js b/lib/runner/test-runner/regular-test-runner.js index 4a4be8252..dc82a5874 100644 --- a/lib/runner/test-runner/regular-test-runner.js +++ b/lib/runner/test-runner/regular-test-runner.js @@ -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(); @@ -35,7 +43,7 @@ module.exports = class RegularTestRunner extends Runner { this._emit(Events.TEST_END); - await this._freeBrowser(); + await freeBrowserPromise; } _emit(event) { @@ -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; } @@ -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) { diff --git a/lib/utils/ipc.js b/lib/utils/ipc.js index de39e3b1f..0e0437f6c 100644 --- a/lib/utils/ipc.js +++ b/lib/utils/ipc.js @@ -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')) { diff --git a/lib/worker/runner/test-runner/index.js b/lib/worker/runner/test-runner/index.js index e61df0519..90ec84550 100644 --- a/lib/worker/runner/test-runner/index.js +++ b/lib/worker/runner/test-runner/index.js @@ -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) { @@ -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) { diff --git a/package-lock.json b/package-lock.json index 6305ef9dc..06a60d62c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2104,7 +2104,7 @@ }, "conventional-commits-parser": { "version": "2.1.7", - "resolved": "http://registry.npmjs.org/conventional-commits-parser/-/conventional-commits-parser-2.1.7.tgz", + "resolved": "https://registry.npmjs.org/conventional-commits-parser/-/conventional-commits-parser-2.1.7.tgz", "integrity": "sha512-BoMaddIEJ6B4QVMSDu9IkVImlGOSGA1I2BQyOZHeLQ6qVOJLcLKn97+fL6dGbzWEiqDzfH4OkcveULmeq2MHFQ==", "dev": true, "requires": { @@ -2147,7 +2147,7 @@ }, "git-raw-commits": { "version": "1.3.6", - "resolved": "http://registry.npmjs.org/git-raw-commits/-/git-raw-commits-1.3.6.tgz", + "resolved": "https://registry.npmjs.org/git-raw-commits/-/git-raw-commits-1.3.6.tgz", "integrity": "sha512-svsK26tQ8vEKnMshTDatSIQSMDdz8CxIIqKsvPqbtV23Etmw6VNaFAitu8zwZ0VrOne7FztwPyRLxK7/DIUTQg==", "dev": true, "requires": { @@ -2203,7 +2203,7 @@ }, "minimist": { "version": "1.2.0", - "resolved": "http://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", + "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", "integrity": "sha1-o1AIsg9BOD7sH7kU9M1d95omQoQ=", "dev": true }, @@ -3639,7 +3639,7 @@ }, "get-stream": { "version": "3.0.0", - "resolved": "http://registry.npmjs.org/get-stream/-/get-stream-3.0.0.tgz", + "resolved": "https://registry.npmjs.org/get-stream/-/get-stream-3.0.0.tgz", "integrity": "sha1-jpQ9E1jcN1VQVOy+LtsFqhdO3hQ=", "dev": true }, @@ -7697,7 +7697,7 @@ }, "load-json-file": { "version": "2.0.0", - "resolved": "http://registry.npmjs.org/load-json-file/-/load-json-file-2.0.0.tgz", + "resolved": "https://registry.npmjs.org/load-json-file/-/load-json-file-2.0.0.tgz", "integrity": "sha1-eUfkIUmvgNaWy/eXvKq8/h/inKg=", "dev": true, "requires": { @@ -7875,7 +7875,7 @@ }, "strip-eof": { "version": "1.0.0", - "resolved": "http://registry.npmjs.org/strip-eof/-/strip-eof-1.0.0.tgz", + "resolved": "https://registry.npmjs.org/strip-eof/-/strip-eof-1.0.0.tgz", "integrity": "sha1-u0P/VZim6wXYm1n80SnJgzE2Br8=", "dev": true }, diff --git a/test/lib/_mocha/test.js b/test/lib/_mocha/test.js index 165dde59e..2b601d068 100644 --- a/test/lib/_mocha/test.js +++ b/test/lib/_mocha/test.js @@ -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; diff --git a/test/lib/runner/test-runner/regular-test-runner.js b/test/lib/runner/test-runner/regular-test-runner.js index 5631939b3..80d99f989 100644 --- a/test/lib/runner/test-runner/regular-test-runner.js +++ b/test/lib/runner/test-runner/regular-test-runner.js @@ -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'); @@ -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 = {}) => { @@ -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 () => { @@ -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); @@ -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 () => { @@ -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); }); diff --git a/test/lib/worker/runner/test-runner/index.js b/test/lib/worker/runner/test-runner/index.js index 0f7cada16..4185be683 100644 --- a/test/lib/worker/runner/test-runner/index.js +++ b/test/lib/worker/runner/test-runner/index.js @@ -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'); @@ -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()); @@ -477,11 +480,10 @@ 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); }); @@ -489,7 +491,6 @@ describe('worker/runner/test-runner', () => { const result = await run_(); assert.match(result.meta, {foo: 'bar'}); - assert.match(result.browserState, {baz: 'qux'}); }); it('should release browser', async () => { @@ -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); }); @@ -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`); }); }); @@ -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()); }); @@ -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 () => { @@ -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'}); + }); }); }); });