Skip to content

Commit

Permalink
fix: do not store all mocha instances in workers
Browse files Browse the repository at this point in the history
  • Loading branch information
j0tunn committed Feb 8, 2018
1 parent f389bf1 commit ed7aab4
Show file tree
Hide file tree
Showing 21 changed files with 78 additions and 291 deletions.
2 changes: 1 addition & 1 deletion lib/runner/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ module.exports = class MainRunner extends AsyncEmitter {
}

run(testFiles) {
const workers = Workers.create(testFiles, this._config);
const workers = Workers.create(this._config);

return this.emitAndWait(RunnerEvents.RUNNER_START, this)
.then(() => {
Expand Down
2 changes: 1 addition & 1 deletion lib/runner/mocha-runner/mocha-adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ module.exports = class MochaAdapter extends EventEmitter {
const start = Date.now();
let isBrokenSession = false;

return workers.runTest(fullTitle, {browserId: browser.id, sessionId: browser.sessionId})
return workers.runTest(fullTitle, {browserId: browser.id, sessionId: browser.sessionId, file: test.file})
.then((data) => {
this._extendTestInfo(test, data);
browser.updateChanges(data.changes);
Expand Down
4 changes: 1 addition & 3 deletions lib/runner/workers.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ module.exports = class Workers {
return new Workers(...args);
}

constructor(testFiles, config) {
this._testFiles = testFiles;
constructor(config) {
this._config = config;

this._workers = this._createWorkerFarm();
Expand Down Expand Up @@ -45,7 +44,6 @@ module.exports = class Workers {
case 'worker.init':
child.send({
event: 'master.init',
testFiles: this._testFiles,
configPath: this._config.configPath,
runtimeConfig: RuntimeConfig.getInstance()
});
Expand Down
5 changes: 2 additions & 3 deletions lib/worker/hermione-facade.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,14 @@ module.exports = class HermioneFacade {
debug('init worker');
process.send({event: 'worker.init'});

process.on('message', ({event, testFiles, configPath, runtimeConfig} = {}) => {
process.on('message', ({event, configPath, runtimeConfig} = {}) => {
if (event !== 'master.init') {
return;
}

try {
const hermione = Hermione.create(configPath);
hermione.init(testFiles);
RuntimeConfig.getInstance().extend(runtimeConfig);
const hermione = Hermione.create(configPath);

debug('worker initialized');
resolve(hermione);
Expand Down
6 changes: 3 additions & 3 deletions lib/worker/hermione.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ const Runner = require('./runner');
const BaseHermione = require('../base-hermione');

module.exports = class Hermione extends BaseHermione {
init(testFiles) {
constructor(configPath) {
super(configPath);

this._runner = Runner.create(this._config);

eventsUtils.passthroughEvent(this._runner, this, [
Expand All @@ -16,8 +18,6 @@ module.exports = class Hermione extends BaseHermione {

RunnerEvents.NEW_BROWSER
]);

this._runner.init(testFiles);
}

runTest(fullTitle, options) {
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

const {Calibrator} = require('gemini-core');
const _ = require('lodash');
const Browser = require('../browser/existing-browser');
const RunnerEvents = require('./constants/runner-events');
const Browser = require('../../browser/existing-browser');
const RunnerEvents = require('../constants/runner-events');

module.exports = class BrowserPool {
static create(config, emitter) {
Expand Down
37 changes: 17 additions & 20 deletions lib/worker/runner/index.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
'use strict';

const _ = require('lodash');
const AsyncEmitter = require('gemini-core').events.AsyncEmitter;
const eventsUtils = require('gemini-core').events.utils;
const BrowserPool = require('../browser-pool');
const RunnerEvents = require('../constants/runner-events');
const MochaRunner = require('./mocha-runner');
const BrowserPool = require('./browser-pool');
const BrowserAgent = require('./browser-agent');
const MochaAdapter = require('./mocha-adapter');

module.exports = class MainRunner extends AsyncEmitter {
module.exports = class Runner extends AsyncEmitter {
static create(config) {
return new MainRunner(config);
return new Runner(config);
}

constructor(config) {
Expand All @@ -18,29 +18,26 @@ module.exports = class MainRunner extends AsyncEmitter {
this._config = config;
this._browserPool = BrowserPool.create(this._config, this);

MochaRunner.prepare();
MochaAdapter.prepare();
}

init(testPaths) {
this._mochaRunners = _.mapValues(testPaths, (files, browserId) => {
return this._createMochaRunner(browserId).init(files);
});
runTest(fullTitle, options) {
return this._createMocha(fullTitle, options.file, options.browserId)
.runInSession(options.sessionId);
}

_createMochaRunner(browserId) {
const mochaRunner = MochaRunner.create(browserId, this._config, this._browserPool);
_createMocha(fullTitle, file, browserId) {
const browserAgent = BrowserAgent.create(browserId, this._browserPool);
const mocha = MochaAdapter.create(browserAgent, this._config.system);

eventsUtils.passthroughEvent(mochaRunner, this, [
eventsUtils.passthroughEvent(mocha, this, [
RunnerEvents.BEFORE_FILE_READ,
RunnerEvents.AFTER_FILE_READ,

RunnerEvents.NEW_BROWSER
RunnerEvents.AFTER_FILE_READ
]);

return mochaRunner;
}
mocha.attachTestFilter((test) => test.fullTitle() === fullTitle);
mocha.loadFiles(file);

runTest(fullTitle, options) {
return this._mochaRunners[options.browserId].runTest(fullTitle, options.sessionId);
return mocha;
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ const clearRequire = require('clear-require');
const _ = require('lodash');
const Mocha = require('mocha');
const q = require('q');
const RunnerEvents = require('../../../constants/runner-events');
const Skip = require('../../../runner/mocha-runner/skip');
const SkipBuilder = require('../../../runner/mocha-runner/skip/skip-builder');
const OnlyBuilder = require('../../../runner/mocha-runner/skip/only-builder');
const ProxyReporter = require('../../../runner/mocha-runner/proxy-reporter');
const {getShortMD5} = require('../../../../lib/utils/crypto');
const RunnerEvents = require('../constants/runner-events');
const Skip = require('../../runner/mocha-runner/skip');
const SkipBuilder = require('../../runner/mocha-runner/skip/skip-builder');
const OnlyBuilder = require('../../runner/mocha-runner/skip/only-builder');
const ProxyReporter = require('../../runner/mocha-runner/proxy-reporter');
const {getShortMD5} = require('../../../lib/utils/crypto');

// Avoid mochajs warning about possible EventEmitter memory leak
// https://nodejs.org/docs/latest/api/events.html#events_emitter_setmaxlisteners_n
Expand Down Expand Up @@ -173,14 +173,6 @@ module.exports = class MochaAdapter extends EventEmitter {
};
}

reinit() {
this._initMocha();
}

isFailed() {
return Boolean(this._fail);
}

attachTestFilter(shouldRunTest) {
this._addEventHandler('test', (test) => {
if (shouldRunTest(test)) {
Expand Down
51 changes: 0 additions & 51 deletions lib/worker/runner/mocha-runner/index.js

This file was deleted.

54 changes: 0 additions & 54 deletions lib/worker/runner/mocha-runner/mocha-builder.js

This file was deleted.

47 changes: 0 additions & 47 deletions lib/worker/runner/mocha-runner/single-test-mocha-adapter.js

This file was deleted.

4 changes: 2 additions & 2 deletions test/lib/runner/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,9 @@ describe('Runner', () => {
const config = makeConfigStub();
const runner = new Runner(config);

return runner.run({foo: 'bar'})
return runner.run()
.then(() => {
assert.calledOnceWith(Workers.create, {foo: 'bar'}, config);
assert.calledOnceWith(Workers.create, config);
});
});

Expand Down
4 changes: 2 additions & 2 deletions test/lib/runner/mocha-runner/mocha-adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -636,10 +636,10 @@ describe('mocha-runner/mocha-adapter', () => {

browserAgent.getBrowser.returns(q({id: 'bro-id', sessionId: '100-500'}));

MochaStub.lastInstance.updateSuiteTree((suite) => suite.addTest({title: 'test-title'}));
MochaStub.lastInstance.updateSuiteTree((suite) => suite.addTest({title: 'test-title', file: 'some/file'}));

return mochaAdapter.run(workers)
.then(() => assert.calledOnceWith(workers.runTest, 'test-title', {browserId: 'bro-id', sessionId: '100-500'}));
.then(() => assert.calledOnceWith(workers.runTest, 'test-title', {browserId: 'bro-id', sessionId: '100-500', file: 'some/file'}));
});

it('should extend test with browser data', () => {
Expand Down
Loading

0 comments on commit ed7aab4

Please sign in to comment.