Merge pull request #426 from gemini-testing/feat/early.browser.free

Feat: release browser before firing any test finish event

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) {

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')) {

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;
             }
         });
 

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) {

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;

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);
             });

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', () => {