From c91cb81e496c2e8c758304d77e7c3b7a7c29f073 Mon Sep 17 00:00:00 2001 From: jvalkeejarvi Date: Wed, 5 Sep 2018 19:27:14 +0300 Subject: [PATCH] fix(runner): Do not persist grep option across runs (#3121) --- lib/helper.js | 12 + lib/middleware/runner.js | 2 + test/unit/helper.spec.js | 28 ++ test/unit/middleware/runner.spec.js | 480 ++++++++++++++-------------- 4 files changed, 287 insertions(+), 235 deletions(-) diff --git a/lib/helper.js b/lib/helper.js index cdf051f01..aaa00b6f5 100644 --- a/lib/helper.js +++ b/lib/helper.js @@ -169,3 +169,15 @@ exports.defer = () => { promise: promise } } + +exports.saveOriginalArgs = (config) => { + if (config && config.client && config.client.args) { + config.client.originalArgs = _.cloneDeep(config.client.args) + } +} + +exports.restoreOriginalArgs = (config) => { + if (config && config.client && config.client.originalArgs) { + config.client.args = _.cloneDeep(config.client.originalArgs) + } +} diff --git a/lib/middleware/runner.js b/lib/middleware/runner.js index a90cf5a73..a78134dbf 100644 --- a/lib/middleware/runner.js +++ b/lib/middleware/runner.js @@ -15,6 +15,7 @@ var json = require('body-parser').json() function createRunnerMiddleware (emitter, fileList, capturedBrowsers, reporter, executor, /* config.protocol */ protocol, /* config.hostname */ hostname, /* config.port */ port, /* config.urlRoot */ urlRoot, config) { + helper.saveOriginalArgs(config) return function (request, response, next) { if (request.url !== '/__run__' && request.url !== urlRoot + 'run') { return next() @@ -48,6 +49,7 @@ function createRunnerMiddleware (emitter, fileList, capturedBrowsers, reporter, }) }) + helper.restoreOriginalArgs(config) if (_.isEmpty(data.args)) { log.debug('Ignoring empty client.args from run command') } else if ((_.isArray(data.args) && _.isArray(config.client.args)) || diff --git a/test/unit/helper.spec.js b/test/unit/helper.spec.js index 1cf286c07..3cabf4a1c 100644 --- a/test/unit/helper.spec.js +++ b/test/unit/helper.spec.js @@ -309,4 +309,32 @@ describe('helper', () => { helper.mmPatternWeight('{**,*}').should.be.deep.equal([2, 1, 0, 0, 0, 0]) }) }) + + describe('saveOriginalArgs', () => { + it('should clone config.client.args', () => { + var config = { + client: { + args: ['--somearg'] + } + } + expect(config.client.originalArgs).to.not.exist + helper.saveOriginalArgs(config) + config.client.args.should.be.deep.equal(['--somearg']) + config.client.originalArgs.should.be.deep.equal(['--somearg']) + }) + }) + + describe('restoreOriginalArgs', () => { + it('should restore config.client.originalArgs', () => { + var config = { + client: { + args: ['--somearg'], + originalArgs: [] + } + } + helper.restoreOriginalArgs(config) + config.client.args.should.be.deep.equal([]) + config.client.originalArgs.should.be.deep.equal([]) + }) + }) }) diff --git a/test/unit/middleware/runner.spec.js b/test/unit/middleware/runner.spec.js index 9a7985fb3..c2dd60c7c 100644 --- a/test/unit/middleware/runner.spec.js +++ b/test/unit/middleware/runner.spec.js @@ -23,6 +23,21 @@ describe('middleware.runner', () => { var handler var fileListMock + function createHandler () { + handler = createRunnerMiddleware( + emitter, + fileListMock, + capturedBrowsers, + new MultReporter([mockReporter]), + executor, + 'http:', + 'localhost', + 8877, + '/', + config + ) + } + before(() => { Promise.setScheduler((fn) => fn()) }) @@ -55,302 +70,297 @@ describe('middleware.runner', () => { nextSpy = sinon.spy() response = new HttpResponseMock() config = {client: {}, basePath: '/'} - - handler = createRunnerMiddleware( - emitter, - fileListMock, - capturedBrowsers, - new MultReporter([mockReporter]), - executor, - 'http:', - 'localhost', - 8877, - '/', - config - ) }) - it('should trigger test run and stream the reporter', (done) => { - capturedBrowsers.add(new Browser()) - sinon.stub(capturedBrowsers, 'areAllReady').callsFake(() => true) - - response.once('end', () => { - expect(nextSpy).to.not.have.been.called - expect(response).to.beServedAs(200, 'result\x1FEXIT10') - done() + describe('', () => { + beforeEach(() => { + createHandler() }) - handler(new HttpRequestMock('/__run__'), response, nextSpy) + it('should trigger test run and stream the reporter', (done) => { + capturedBrowsers.add(new Browser()) + sinon.stub(capturedBrowsers, 'areAllReady').callsFake(() => true) - mockReporter.write('result') - emitter.emit('run_complete', capturedBrowsers, {exitCode: 0}) - }) + response.once('end', () => { + expect(nextSpy).to.not.have.been.called + expect(response).to.beServedAs(200, 'result\x1FEXIT10') + done() + }) - it('should set the empty to 0 if empty results', (done) => { - capturedBrowsers.add(new Browser()) - sinon.stub(capturedBrowsers, 'areAllReady').callsFake(() => true) + handler(new HttpRequestMock('/__run__'), response, nextSpy) - response.once('end', () => { - expect(nextSpy).to.not.have.been.called - expect(response).to.beServedAs(200, 'result\x1FEXIT00') - done() + mockReporter.write('result') + emitter.emit('run_complete', capturedBrowsers, {exitCode: 0}) }) - handler(new HttpRequestMock('/__run__'), response, nextSpy) + it('should set the empty to 0 if empty results', (done) => { + capturedBrowsers.add(new Browser()) + sinon.stub(capturedBrowsers, 'areAllReady').callsFake(() => true) - mockReporter.write('result') - emitter.emit('run_complete', capturedBrowsers, {exitCode: 0, success: 0, failed: 0}) - }) + response.once('end', () => { + expect(nextSpy).to.not.have.been.called + expect(response).to.beServedAs(200, 'result\x1FEXIT00') + done() + }) - it('should set the empty to 1 if successful tests', (done) => { - capturedBrowsers.add(new Browser()) - sinon.stub(capturedBrowsers, 'areAllReady').callsFake(() => true) + handler(new HttpRequestMock('/__run__'), response, nextSpy) - response.once('end', () => { - expect(nextSpy).to.not.have.been.called - expect(response).to.beServedAs(200, 'result\x1FEXIT10') - done() + mockReporter.write('result') + emitter.emit('run_complete', capturedBrowsers, {exitCode: 0, success: 0, failed: 0}) }) - handler(new HttpRequestMock('/__run__'), response, nextSpy) + it('should set the empty to 1 if successful tests', (done) => { + capturedBrowsers.add(new Browser()) + sinon.stub(capturedBrowsers, 'areAllReady').callsFake(() => true) - mockReporter.write('result') - emitter.emit('run_complete', capturedBrowsers, {exitCode: 0, success: 3, failed: 0}) - }) + response.once('end', () => { + expect(nextSpy).to.not.have.been.called + expect(response).to.beServedAs(200, 'result\x1FEXIT10') + done() + }) - it('should set the empty to 1 if failed tests', (done) => { - capturedBrowsers.add(new Browser()) - sinon.stub(capturedBrowsers, 'areAllReady').callsFake(() => true) + handler(new HttpRequestMock('/__run__'), response, nextSpy) - response.once('end', () => { - expect(nextSpy).to.not.have.been.called - expect(response).to.beServedAs(200, 'result\x1FEXIT10') - done() + mockReporter.write('result') + emitter.emit('run_complete', capturedBrowsers, {exitCode: 0, success: 3, failed: 0}) }) - handler(new HttpRequestMock('/__run__'), response, nextSpy) + it('should set the empty to 1 if failed tests', (done) => { + capturedBrowsers.add(new Browser()) + sinon.stub(capturedBrowsers, 'areAllReady').callsFake(() => true) - mockReporter.write('result') - emitter.emit('run_complete', capturedBrowsers, {exitCode: 0, success: 0, failed: 6}) - }) + response.once('end', () => { + expect(nextSpy).to.not.have.been.called + expect(response).to.beServedAs(200, 'result\x1FEXIT10') + done() + }) - it('should not run if there is no browser captured', (done) => { - sinon.stub(fileListMock, 'refresh') + handler(new HttpRequestMock('/__run__'), response, nextSpy) - response.once('end', () => { - expect(nextSpy).to.not.have.been.called - expect(response).to.beServedAs(200, 'No captured browser, open http://localhost:8877/\n') - expect(fileListMock.refresh).not.to.have.been.called - done() + mockReporter.write('result') + emitter.emit('run_complete', capturedBrowsers, {exitCode: 0, success: 0, failed: 6}) }) - handler(new HttpRequestMock('/__run__'), response, nextSpy) - }) + it('should not run if there is no browser captured', (done) => { + sinon.stub(fileListMock, 'refresh') - var clientArgsRuns = [ - { - desc: 'should parse body and set client.args', - expected: ['arg1', 'arg2'], - rawMessage: '{"args": ["arg1", "arg2"]}' - }, - { - desc: 'should set array client args passed by run when there are no existing client.args', - expected: ['my_args'], - rawMessage: '{"args": ["my_args"]}' - }, - { - desc: 'should set object client args passed by run when there are no existing client.args', - expected: {arg2: 'fig', arg3: 'chocolate'}, - rawMessage: '{"args": {"arg2": "fig", "arg3": "chocolate"}}' - }, - { - desc: 'should overwrite empty array client.args when run passes an array for client.args', - expected: ['user_arg1'], - rawMessage: '{"args": ["user_arg1"]}', - existingConfig: [] - }, - { - desc: 'should overwrite empty array client.args when run passes an object for client.args', - expected: {arg2: 'figs', arg3: 'chocolates'}, - rawMessage: '{"args": {"arg2": "figs", "arg3": "chocolates"}}', - existingConfig: [] - }, - { - desc: 'should overwrite empty object client.args when run passes an array for client.args', - expected: ['user_arg'], - rawMessage: '{"args": ["user_arg"]}', - existingConfig: {} - }, - { - desc: 'should not overwrite existing array client.args when run passes an empty array for client.args', - expected: ['user_arg'], - rawMessage: '{"args": []}', - existingConfig: ['user_arg'] - }, - { - desc: 'should not overwrite existing array client.args when run passes an empty object for client.args', - expected: ['user_arg'], - rawMessage: '{"args": {}}', - existingConfig: ['user_arg'] - }, - { - desc: 'should not overwrite existing array client.args when run passes no client.args', - expected: ['user_arg'], - rawMessage: '{}', - existingConfig: ['user_arg'] - }, - { - desc: 'should merge existing client.args with client.args passed by run', - expected: {arg1: 'cherry', arg2: 'fig', arg3: 'chocolate'}, - rawMessage: '{"args": {"arg2": "fig", "arg3": "chocolate"}}', - existingConfig: {arg1: 'cherry', arg2: 'mango'} - }, - { - desc: 'should merge empty client.args with client.args passed by run', - expected: {arg2: 'fig', arg3: 'chocolate'}, - rawMessage: '{"args": {"arg2": "fig", "arg3": "chocolate"}}', - existingConfig: {} - } - ] + response.once('end', () => { + expect(nextSpy).to.not.have.been.called + expect(response).to.beServedAs(200, 'No captured browser, open http://localhost:8877/\n') + expect(fileListMock.refresh).not.to.have.been.called + done() + }) - describe('', function () { - clientArgsRuns.forEach(function (run) { - it(run.desc, (done) => { - capturedBrowsers.add(new Browser()) - sinon.stub(capturedBrowsers, 'areAllReady').callsFake(() => true) - if (run.existingConfig) { - config = _.merge(config, {client: {args: run.existingConfig}}) - } + handler(new HttpRequestMock('/__run__'), response, nextSpy) + }) - emitter.once('run_start', () => { - expect(config.client.args).to.deep.equal(run.expected) - done() - }) + it('should refresh explicit files if specified', (done) => { + capturedBrowsers.add(new Browser()) + sinon.stub(capturedBrowsers, 'areAllReady').callsFake(() => true) + sinon.stub(fileListMock, 'refresh') + sinon.stub(fileListMock, 'addFile') + sinon.stub(fileListMock, 'changeFile') + sinon.stub(fileListMock, 'removeFile') + + var RAW_MESSAGE = JSON.stringify({ + addedFiles: ['/new.js'], + removedFiles: ['/foo.js', '/bar.js'], + changedFiles: ['/changed.js'] + }) - var RAW_MESSAGE = run.rawMessage + var request = new HttpRequestMock('/__run__', { + 'content-type': 'application/json', + 'content-length': RAW_MESSAGE.length + }) - var request = new HttpRequestMock('/__run__', { - 'content-type': 'application/json', - 'content-length': RAW_MESSAGE.length - }) + handler(request, response, nextSpy) - handler(request, response, nextSpy) + request.emit('data', RAW_MESSAGE) + request.emit('end') - request.emit('data', RAW_MESSAGE) - request.emit('end') + process.nextTick(() => { + expect(fileListMock.refresh).not.to.have.been.called + expect(fileListMock.addFile).to.have.been.calledWith(path.resolve('/new.js')) + expect(fileListMock.removeFile).to.have.been.calledWith(path.resolve('/foo.js')) + expect(fileListMock.removeFile).to.have.been.calledWith(path.resolve('/bar.js')) + expect(fileListMock.changeFile).to.have.been.calledWith(path.resolve('/changed.js')) + done() }) }) - }) - it('should refresh explicit files if specified', (done) => { - capturedBrowsers.add(new Browser()) - sinon.stub(capturedBrowsers, 'areAllReady').callsFake(() => true) - sinon.stub(fileListMock, 'refresh') - sinon.stub(fileListMock, 'addFile') - sinon.stub(fileListMock, 'changeFile') - sinon.stub(fileListMock, 'removeFile') - - var RAW_MESSAGE = JSON.stringify({ - addedFiles: ['/new.js'], - removedFiles: ['/foo.js', '/bar.js'], - changedFiles: ['/changed.js'] - }) + it('should wait for refresh to finish if applicable before scheduling execution', (done) => { + capturedBrowsers.add(new Browser()) + sinon.stub(capturedBrowsers, 'areAllReady').callsFake(() => true) - var request = new HttpRequestMock('/__run__', { - 'content-type': 'application/json', - 'content-length': RAW_MESSAGE.length - }) + var res = null + var fileListPromise = new Promise((resolve, reject) => { + res = resolve + }) + sinon.stub(fileListMock, 'refresh').returns(fileListPromise) + sinon.stub(executor, 'schedule') - handler(request, response, nextSpy) + var request = new HttpRequestMock('/__run__') + handler(request, response, nextSpy) - request.emit('data', RAW_MESSAGE) - request.emit('end') + process.nextTick(() => { + expect(fileListMock.refresh).to.have.been.called + expect(executor.schedule).to.not.have.been.called - process.nextTick(() => { - expect(fileListMock.refresh).not.to.have.been.called - expect(fileListMock.addFile).to.have.been.calledWith(path.resolve('/new.js')) - expect(fileListMock.removeFile).to.have.been.calledWith(path.resolve('/foo.js')) - expect(fileListMock.removeFile).to.have.been.calledWith(path.resolve('/bar.js')) - expect(fileListMock.changeFile).to.have.been.calledWith(path.resolve('/changed.js')) - done() + // Now try resolving the promise + res() + setTimeout(() => { + expect(executor.schedule).to.have.been.called + done() + }, 2) + }) }) - }) - it('should wait for refresh to finish if applicable before scheduling execution', (done) => { - capturedBrowsers.add(new Browser()) - sinon.stub(capturedBrowsers, 'areAllReady').callsFake(() => true) + it('should schedule execution if no refresh', (done) => { + capturedBrowsers.add(new Browser()) + sinon.stub(capturedBrowsers, 'areAllReady').callsFake(() => true) - var res = null - var fileListPromise = new Promise((resolve, reject) => { - res = resolve - }) - sinon.stub(fileListMock, 'refresh').returns(fileListPromise) - sinon.stub(executor, 'schedule') + sinon.spy(fileListMock, 'refresh') + sinon.stub(executor, 'schedule') + + var RAW_MESSAGE = JSON.stringify({refresh: false}) + + var request = new HttpRequestMock('/__run__', { + 'content-type': 'application/json', + 'content-length': RAW_MESSAGE.length + }) - var request = new HttpRequestMock('/__run__') - handler(request, response, nextSpy) + handler(request, response, nextSpy) - process.nextTick(() => { - expect(fileListMock.refresh).to.have.been.called - expect(executor.schedule).to.not.have.been.called + request.emit('data', RAW_MESSAGE) + request.emit('end') - // Now try resolving the promise - res() - setTimeout(() => { + process.nextTick(() => { + expect(fileListMock.refresh).not.to.have.been.called expect(executor.schedule).to.have.been.called done() - }, 2) + }) }) - }) - it('should schedule execution if no refresh', (done) => { - capturedBrowsers.add(new Browser()) - sinon.stub(capturedBrowsers, 'areAllReady').callsFake(() => true) + it('should not schedule execution if refreshing and autoWatch', (done) => { + config.autoWatch = true - sinon.spy(fileListMock, 'refresh') - sinon.stub(executor, 'schedule') + capturedBrowsers.add(new Browser()) + sinon.stub(capturedBrowsers, 'areAllReady').callsFake(() => true) - var RAW_MESSAGE = JSON.stringify({refresh: false}) + sinon.spy(fileListMock, 'refresh') + sinon.stub(executor, 'schedule') - var request = new HttpRequestMock('/__run__', { - 'content-type': 'application/json', - 'content-length': RAW_MESSAGE.length - }) + handler(new HttpRequestMock('/__run__'), response, nextSpy) - handler(request, response, nextSpy) - - request.emit('data', RAW_MESSAGE) - request.emit('end') + process.nextTick(() => { + expect(fileListMock.refresh).to.have.been.called + expect(executor.schedule).not.to.have.been.called + done() + }) + }) - process.nextTick(() => { - expect(fileListMock.refresh).not.to.have.been.called - expect(executor.schedule).to.have.been.called - done() + it('should ignore other urls', (done) => { + handler(new HttpRequestMock('/something'), response, () => { + expect(response).to.beNotServed() + done() + }) }) }) - it('should not schedule execution if refreshing and autoWatch', (done) => { - config.autoWatch = true + describe('', () => { + var clientArgsRuns = [ + { + desc: 'should parse body and set client.args', + expected: ['arg1', 'arg2'], + rawMessage: '{"args": ["arg1", "arg2"]}' + }, + { + desc: 'should set array client args passed by run when there are no existing client.args', + expected: ['my_args'], + rawMessage: '{"args": ["my_args"]}' + }, + { + desc: 'should set object client args passed by run when there are no existing client.args', + expected: {arg2: 'fig', arg3: 'chocolate'}, + rawMessage: '{"args": {"arg2": "fig", "arg3": "chocolate"}}' + }, + { + desc: 'should overwrite empty array client.args when run passes an array for client.args', + expected: ['user_arg1'], + rawMessage: '{"args": ["user_arg1"]}', + existingConfig: [] + }, + { + desc: 'should overwrite empty array client.args when run passes an object for client.args', + expected: {arg2: 'figs', arg3: 'chocolates'}, + rawMessage: '{"args": {"arg2": "figs", "arg3": "chocolates"}}', + existingConfig: [] + }, + { + desc: 'should overwrite empty object client.args when run passes an array for client.args', + expected: ['user_arg'], + rawMessage: '{"args": ["user_arg"]}', + existingConfig: {} + }, + { + desc: 'should not overwrite existing array client.args when run passes an empty array for client.args', + expected: ['user_arg'], + rawMessage: '{"args": []}', + existingConfig: ['user_arg'] + }, + { + desc: 'should not overwrite existing array client.args when run passes an empty object for client.args', + expected: ['user_arg'], + rawMessage: '{"args": {}}', + existingConfig: ['user_arg'] + }, + { + desc: 'should not overwrite existing array client.args when run passes no client.args', + expected: ['user_arg'], + rawMessage: '{}', + existingConfig: ['user_arg'] + }, + { + desc: 'should merge existing client.args with client.args passed by run', + expected: {arg1: 'cherry', arg2: 'fig', arg3: 'chocolate'}, + rawMessage: '{"args": {"arg2": "fig", "arg3": "chocolate"}}', + existingConfig: {arg1: 'cherry', arg2: 'mango'} + }, + { + desc: 'should merge empty client.args with client.args passed by run', + expected: {arg2: 'fig', arg3: 'chocolate'}, + rawMessage: '{"args": {"arg2": "fig", "arg3": "chocolate"}}', + existingConfig: {} + } + ] - capturedBrowsers.add(new Browser()) - sinon.stub(capturedBrowsers, 'areAllReady').callsFake(() => true) + clientArgsRuns.forEach(function (run) { + it(run.desc, (done) => { + capturedBrowsers.add(new Browser()) + sinon.stub(capturedBrowsers, 'areAllReady').callsFake(() => true) + if (run.existingConfig) { + config = _.merge(config, {client: {args: run.existingConfig}}) + } + createHandler() - sinon.spy(fileListMock, 'refresh') - sinon.stub(executor, 'schedule') + emitter.once('run_start', () => { + expect(config.client.args).to.deep.equal(run.expected) + expect(config.client.originalArgs).to.deep.equal(run.existingConfig) + done() + }) - handler(new HttpRequestMock('/__run__'), response, nextSpy) + var RAW_MESSAGE = run.rawMessage - process.nextTick(() => { - expect(fileListMock.refresh).to.have.been.called - expect(executor.schedule).not.to.have.been.called - done() - }) - }) + var request = new HttpRequestMock('/__run__', { + 'content-type': 'application/json', + 'content-length': RAW_MESSAGE.length + }) + + handler(request, response, nextSpy) - it('should ignore other urls', (done) => { - handler(new HttpRequestMock('/something'), response, () => { - expect(response).to.beNotServed() - done() + request.emit('data', RAW_MESSAGE) + request.emit('end') + }) }) }) })