From f6fa93fef0cee3de1e2365ef74bb8abf6262f87d Mon Sep 17 00:00:00 2001 From: Arjun Gadhia Date: Thu, 12 Apr 2018 17:06:45 +0100 Subject: [PATCH 1/2] =?UTF-8?q?Fix=20cleanup=20of=20smoke=20task=20and=20a?= =?UTF-8?q?dd=20test=20=20=F0=9F=90=BF=20v2.7.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/smoke/smoke-test.js | 68 +++++++++++++++++++++------------------- package.json | 4 +-- test/tasks/smoke.test.js | 19 +++++++++++ 3 files changed, 56 insertions(+), 35 deletions(-) diff --git a/lib/smoke/smoke-test.js b/lib/smoke/smoke-test.js index 21090de..34d3fc2 100644 --- a/lib/smoke/smoke-test.js +++ b/lib/smoke/smoke-test.js @@ -36,9 +36,11 @@ class SmokeTest { throw new Error(`Config file for smoke test does not exist at ${this.configFile}. Either create a config file at ./test/smoke.js, or pass in a path using --config.`); } - -// process.on('exit', async (code) => await this.cleanup(code)); -// process.on('SIGINT', async (code) => await this.cleanup(code)); + //On manual Ctrl-C quitting, cleanup leftover browserstack etc. + process.on('SIGINT', async (code) => { + await this.cleanup(); + process.exit(code); + }); } @@ -93,18 +95,16 @@ class SmokeTest { return Promise.all(allTests); } -// async cleanup (code) { -// this.browser && this.browser.close(); -// await getSeleniumGrid.cleanup(); -// process.exit(code); -// } + async cleanup () { + this.browser && this.browser.close(); + return await getSeleniumGrid.cleanup(); + } async run (sets) { const startTime = new Date().getTime(); const configsToRun = await filterConfigs(this.configFile, sets, this.isInteractive); this.browser = await puppeteer.launch(); - let results = []; try { @@ -112,32 +112,34 @@ class SmokeTest { const [puppetResults, crossBrowserResults] = await this.runSuite(suiteOpts); results = results.concat(puppetResults).concat(crossBrowserResults); } - } finally { - this.browser.close(); - } - - const totalResults = { - urlsTested: results.length, - passed: results.filter(url => (url.failed === 0 && url.errors === 0)), - failed: results.filter(url => url.failed > 0), - errors: results.filter(url => url.errors > 0) - }; - - console.log('--------------------------------'); - console.log(chalk`{bold.underline Smoke Test Results}`); - console.log(chalk`{bgBlue URLs tested:} ${totalResults.urlsTested}`); - console.log(chalk`{bgGreen Passed:} ${totalResults.passed.length}`); - console.log(chalk`{bgRed Failed:} ${totalResults.failed.length}`); - console.log(chalk`{bgBlack Errors:} ${totalResults.errors.length}`); - console.log(chalk`{bgWhite Time Taken:} ${new Date(new Date().getTime() - startTime).toISOString().substr(14, 5)}s`); - console.log('--------------------------------'); - - if(totalResults.failed.length || totalResults.errors.length > ERROR_THRESHOLD) { - return Promise.reject(totalResults); - } else { - return Promise.resolve(totalResults); + const totalResults = { + urlsTested: results.length, + passed: results.filter(url => (url.failed === 0 && url.errors === 0)), + failed: results.filter(url => url.failed > 0), + errors: results.filter(url => url.errors > 0) + }; + + console.log('--------------------------------'); + console.log(chalk`{bold.underline Smoke Test Results}`); + console.log(chalk`{bgBlue URLs tested:} ${totalResults.urlsTested}`); + console.log(chalk`{bgGreen Passed:} ${totalResults.passed.length}`); + console.log(chalk`{bgRed Failed:} ${totalResults.failed.length}`); + console.log(chalk`{bgBlack Errors:} ${totalResults.errors.length}`); + console.log(chalk`{bgWhite Time Taken:} ${new Date(new Date().getTime() - startTime).toISOString().substr(14, 5)}s`); + console.log('--------------------------------'); + + await this.cleanup(); + if(totalResults.failed.length || totalResults.errors.length > ERROR_THRESHOLD) { + return Promise.reject(totalResults); + } else { + return Promise.resolve(totalResults); + } + } catch(e) { + await this.cleanup(); + throw e; } + } }; diff --git a/package.json b/package.json index 33b6616..4302f92 100644 --- a/package.json +++ b/package.json @@ -36,7 +36,7 @@ "devDependencies": { "@financial-times/n-gage": "^1.19.2", "cookie-parser": "^1.4.3", - "jest": "^22.0.6", - "express": "^4.16.2" + "express": "^4.16.2", + "jest": "^22.0.6" } } diff --git a/test/tasks/smoke.test.js b/test/tasks/smoke.test.js index cdaefb8..e4d7c21 100644 --- a/test/tasks/smoke.test.js +++ b/test/tasks/smoke.test.js @@ -2,6 +2,7 @@ const server = require('../server/app'); const SmokeTest = require('../../lib/smoke/smoke-test'); +const { spawn } = require('child_process'); describe('Smoke Tests of the Smoke', () => { @@ -116,4 +117,22 @@ describe('Smoke Tests of the Smoke', () => { }); }); + + describe('CLI task', () => { + test('should exit with the correct code for a passing test', (done) => { + const proc = spawn('./bin/n-test.js', ['smoke', '--host', 'http://localhost:3004', '--config', 'test/fixtures/smoke-pass.js']); + proc.on('close', (code) => { + expect(code).toEqual(0); + done(); + }); + }); + + test('should exit with a bad code if the test fails', (done) => { + const proc = spawn('./bin/n-test.js', ['smoke', '--host', 'http://localhost:3004', '--config', 'test/fixtures/smoke-fail.js']); + proc.on('close', (code) => { + expect(code).toEqual(2); + done(); + }); + }); + }); }); From 1e559a10b47319445180c2283b4cfc840597828e Mon Sep 17 00:00:00 2001 From: Arjun Gadhia Date: Fri, 13 Apr 2018 09:56:15 +0100 Subject: [PATCH 2/2] =?UTF-8?q?Readability=20and=20correct=20exit=20code?= =?UTF-8?q?=20=20=F0=9F=90=BF=20v2.7.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/smoke/smoke-test.js | 4 +++- tasks/smoke.js | 2 +- test/tasks/smoke.test.js | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/smoke/smoke-test.js b/lib/smoke/smoke-test.js index 34d3fc2..70a57f8 100644 --- a/lib/smoke/smoke-test.js +++ b/lib/smoke/smoke-test.js @@ -120,13 +120,15 @@ class SmokeTest { errors: results.filter(url => url.errors > 0) }; + const timeTaken = new Date(new Date().getTime() - startTime).toISOString().substr(14, 5); + console.log('--------------------------------'); console.log(chalk`{bold.underline Smoke Test Results}`); console.log(chalk`{bgBlue URLs tested:} ${totalResults.urlsTested}`); console.log(chalk`{bgGreen Passed:} ${totalResults.passed.length}`); console.log(chalk`{bgRed Failed:} ${totalResults.failed.length}`); console.log(chalk`{bgBlack Errors:} ${totalResults.errors.length}`); - console.log(chalk`{bgWhite Time Taken:} ${new Date(new Date().getTime() - startTime).toISOString().substr(14, 5)}s`); + console.log(chalk`{bgWhite Time Taken:} ${timeTaken}s`); console.log('--------------------------------'); await this.cleanup(); diff --git a/tasks/smoke.js b/tasks/smoke.js index 2f13d00..113a3ae 100644 --- a/tasks/smoke.js +++ b/tasks/smoke.js @@ -44,7 +44,7 @@ module.exports = (program) => { // eslint-disable-next-line no-console console.error(err); } - process.exit(2); + process.exit(1); }); }); }; diff --git a/test/tasks/smoke.test.js b/test/tasks/smoke.test.js index e4d7c21..b8d1777 100644 --- a/test/tasks/smoke.test.js +++ b/test/tasks/smoke.test.js @@ -130,7 +130,7 @@ describe('Smoke Tests of the Smoke', () => { test('should exit with a bad code if the test fails', (done) => { const proc = spawn('./bin/n-test.js', ['smoke', '--host', 'http://localhost:3004', '--config', 'test/fixtures/smoke-fail.js']); proc.on('close', (code) => { - expect(code).toEqual(2); + expect(code).toEqual(1); done(); }); });