Merge pull request #172 from Financial-Times/lk/check-status

Improve output for troubleshooting failed smoke tests

README.md

@@ -54,7 +54,8 @@ module.exports = [
 		urls: {
 			'/': 200,
 			'/redirect': '/'
-		}
+		},
+		description: 'Test suite descriptions are optional',
 	}
 ];
 ```
@@ -110,7 +111,8 @@ urls: {
 		  contentSelector: '.headline, .image, .standfirst'
 		  threshold: 30 // % of viewport that should be visible content
 		},
-		performance: true //checks firstPaint/firstContentfulPaint against baseline. default = 2000, or can specify.
+		performance: true, //checks firstPaint/firstContentfulPaint against baseline. default = 2000, or can specify.
+		description: 'Each test may have an optional description. It will display when the test result is reported',
 	}
 }
 ...

lib/checks/index.js

@@ -1,4 +1,5 @@
 module.exports = {
+	status: require('./status'),
 	content: require('./content'),
 	cssCoverage: require('./css-coverage'),
 	elements: require('./elements'),
@@ -8,6 +9,5 @@ module.exports = {
 	performance: require('./performance'),
 	responseHeaders: require('./response-headers'),
 	screenshot: require('./screenshot'),
-	status: require('./status'),
 	visibleContent: require('./visible-content')
 };

lib/checks/status.js

@@ -1,4 +1,16 @@
-module.exports = (testPage) => {
+const outputErrorInfo = async (testPage) => {
+	const headers = testPage.response.headers();
+	const isHTML = headers['content-Type'] && headers['content-Type'].includes('text/html');
+	const content = isHTML ? await testPage.page.content() : await testPage.response.text();
+	return `Status = ${testPage.status}. Response body content :
+
+	${content}
+
+	~ENDS~
+	`;
+};
+
+module.exports = async (testPage) => {
 	if (testPage.check.status === 204) {
 		//eslint-disable-next-line no-console
 		console.info('204 status checks are not supported yet!');
@@ -10,6 +22,11 @@ module.exports = (testPage) => {
 			result: testPage.redirect && testPage.check.status === testPage.redirect.to
 		};
 	} else {
-		return { expected: testPage.check.status, actual: testPage.status, result: testPage.status === testPage.check.status || testPage.status === 304 && testPage.check.status === 200 };
+		const isUnexpectedHttpError = testPage.check.status !== 500 && testPage.status === 500;
+		return {
+			expected: `Status = ${testPage.check.status}`,
+			actual: isUnexpectedHttpError ? await outputErrorInfo(testPage) : `Status = ${testPage.status}`,
+			result: testPage.status === testPage.check.status || testPage.status === 304 && testPage.check.status === 200
+		};
 	}
 };

lib/smoke/puppeteer-page.js

@@ -15,6 +15,7 @@ class PuppeteerPage {
 		this.browser = null;
 		this.type = 'Chrome';
 		this.name = options.name;
+		this.description = options.description || '';
 		this.url = options.url;
 		this.url.hash = '';
 		this.host = options.url.host;

lib/smoke/setup-page.js

@@ -10,6 +10,10 @@ module.exports = (path, urlOpts, suiteOpts, globalOpts) => {
 
 	options.name = suiteOpts.name;
 
+	// description=false prevents the test suite description
+	// cascading down to the individual test.
+	options.description = urlOpts.description !== false && [suiteOpts.description, urlOpts.description].filter(Boolean).join('. ');
+
 	options.requestHeaders = Object.assign({}, globalOpts.headers || {}, suiteOpts.headers || {}, urlOpts.headers || {});
 	options.requestHeaderPaths = urlOpts.requestHeaderPaths || suiteOpts.requestHeaderPaths || [];
 

lib/smoke/verify-url.js

@@ -35,7 +35,8 @@ const verifyUrl = (testPage, browser, tests) => async () => {
 				${testPage.url.pathname || ''}
 			`);
 		}
-		console.log(chalk`{bold Testing URL }({blue ${testPage.type}}): {underline.yellow ${realUrl.href || testPage.url.toString()}}`);
+		const description = testPage.description? chalk`\n\t{bold Description:} ${testPage.description}` : '';
+		console.log(chalk`{bold Testing URL }({blue ${testPage.type}}): {underline.yellow ${realUrl.href || testPage.url.toString()}}${description}`);
 		checkResults
 			.filter(result => !!result)
 			.forEach((check) => {

lib/smoke/webdriver-page.js

@@ -8,6 +8,7 @@ class WebdriverPage {
 	constructor (options, browserName, isAutomatedTest) {
 		this.type = browserName;
 		this.url = options.url;
+		this.description = options.description || '';
 
 		this.isAutomatedTest = isAutomatedTest;
 

test/fixtures/smoke-fail.js

@@ -3,9 +3,14 @@ const assert = require('assert');
 module.exports = [{
 	urls: {
 		'/status/503': 200,
+		'/status/200': 500,
 		'/status/404': {
 			status: 404
 		},
+		'/status/html/500': {
+			description: 'Output should show HTML response body when unexpected status code',
+			status: 200,
+		},
 		'/coverage/bad': {
 			cssCoverage: {
 				'coverage/bad': 50
@@ -25,7 +30,7 @@ module.exports = [{
 		},
 		'/json': {
 			content: (body) => {
-				assert.equal(body.key, 'wrong-value');
+				assert.strictEqual(body.key, 'wrong-value');
 			}
 		}
 	}

test/server/app.js

@@ -6,6 +6,24 @@ app.get('/status/:status', (req, res) => {
 	res.status(req.params.status).send(req.params.status);
 });
 
+app.get('/status/html/:status', (req, res) => {
+	res
+		.status(req.params.status)
+		.set('Content-Type', 'text/html')
+		.send(`
+			<!DOCTYPE html>
+			<html lang="en">
+				<head>
+				<meta charset="utf-8">
+				<title>Status ${req.params.status}</title>
+				</head>
+				<body>
+					<h1>Status is ${req.params.status}</h1>
+				</body>
+			</html>
+		`);
+});
+
 app.get('/coverage/good', (req, res) => {
 	res.send(`
 		<body>

test/tasks/smoke.test.js

@@ -37,7 +37,7 @@ describe('Smoke Tests of the Smoke', () => {
 			return smoke.run()
 				.catch((results) => {
 					expect(results.passed.length).toEqual(1);
-					expect(results.failed.length).toEqual(5);
+					expect(results.failed.length).toEqual(7);
 					expect(results.errors.length).toEqual(0);
 					done();
 				});