[FIX] Improve error handling (#102)

- Fixes client-side error handling
- Adds error handling in case QUnit is not available
- Adds handling for Access Denied error in IE
- Adds error handling integration tests

lib/client/.eslintrc.js

@@ -3,7 +3,6 @@ module.exports = {
 		"browser": true
 	},
 	"globals": {
-		"sap": "readonly",
-		"karma": "readonly"
+		"sap": "readonly"
 	}
-}
+}

lib/client/browser.js

@@ -4,6 +4,18 @@ require("./discovery.js");
 (function(window) {
 	const karma = window.__karma__;
 
+	function reportSetupFailure(description, error) {
+		karma.info({total: 1});
+		karma.result({
+			description: description,
+			suite: [],
+			success: false,
+			log: error ? [error] : [],
+			time: 0
+		});
+		karma.complete({});
+	}
+
 	function setFullSize(element) {
 		element.style.height = "100%";
 		element.style.padding = "0";
@@ -62,6 +74,7 @@ require("./discovery.js");
 					+ "Please set a testpage in the config or via CLI.\n"
 					+ "For more details: https://github.com/SAP/karma-ui5#defining-testpage"]
 				);
+				// reportSetupFailure(); // TODO
 				return;
 			}
 
@@ -75,6 +88,7 @@ require("./discovery.js");
 					+ "\n\n"
 					+ "Please explicitly configure a \"testpage\" in your karma config or via CLI:\n"
 					+ "https://github.com/SAP/karma-ui5#defining-testpage"]);
+				// reportSetupFailure(); // TODO
 				return;
 			}
 
@@ -84,13 +98,15 @@ require("./discovery.js");
 		config.testpage = prependBase(config.testpage);
 
 		window.findTests(config.testpage).then(function(testpages) {
+			if (!testpages || testpages.length === 0) {
+				reportSetupFailure("Could not resolve any testpages!");
+				return;
+			}
 			runTests(testpages.map(function(testpage) {
 				return testpage["fullpage"];
 			}));
-		}, function(e) {
-			// console.error("fail");
-			karma.log("error", e.message);
-			// TODO: report error
+		}, function(err) {
+			reportSetupFailure("Error resolving testsuite: " + err.fullpage, err.error);
 		});
 
 		function getTestsuites(files) {
@@ -116,22 +132,25 @@ require("./discovery.js");
 			function runTestPage(i) {
 				const qunitHtmlFile = testpages[i];
 				windowUtil(qunitHtmlFile, function(testWindow) {
-					const QUnit = testWindow.contentWindow.QUnit;
 					let timer = null;
 					let testResult = {};
-
-					if (QUnit.begin) {
-						QUnit.begin(function(args) {
-							totalNumberOfTest += args.totalTests;
-							karma.info({total: totalNumberOfTest});
-						});
+					let QUnit;
+					let accessError = false;
+					try {
+						QUnit = testWindow.contentWindow.QUnit;
+					} catch (err) {
+						if (err.name === "TypeError" && err.message === "Permission denied") {
+							accessError = true;
+						} else {
+							throw err;
+						}
 					}
 
-					QUnit.done(function() {
-						// Test page done
-
+					function testFinished() {
 						// Merge test page coverage into global coverage object
-						mergeCoverage(testWindow.contentWindow.__coverage__);
+						if (!accessError) {
+							mergeCoverage(testWindow.contentWindow.__coverage__);
+						}
 
 						// Run next test or trigger completion
 						if (i < testpages.length - 1) {
@@ -143,7 +162,40 @@ require("./discovery.js");
 								coverage: coverageMap ? coverageMap.toJSON() : undefined
 							});
 						}
-					});
+					}
+
+					if (!QUnit) {
+						const log = [];
+						if (accessError || !testWindow.contentWindow.document.querySelector("script")) {
+							// Access Error (IE) or page doesn't have any script => probably 404
+							log.push("Error while loading testpage");
+						} else {
+							// QUnit couldn't be loaded
+							log.push("Missing QUnit framework");
+						}
+						// Report a test failure
+						totalNumberOfTest += 1;
+						karma.info({total: totalNumberOfTest});
+						karma.result( {
+							description: qunitHtmlFile,
+							suite: [],
+							success: false,
+							log: log,
+							time: 0
+						});
+
+						testFinished();
+						return;
+					}
+
+					if (QUnit.begin) {
+						QUnit.begin(function(args) {
+							totalNumberOfTest += args.totalTests;
+							karma.info({total: totalNumberOfTest});
+						});
+					}
+
+					QUnit.done(testFinished);
 
 					QUnit.testStart(function(test) {
 						timer = new Date().getTime();

lib/client/discovery.js

@@ -2,6 +2,8 @@ const Promise = require("es6-promise").Promise;
 const assign = require("lodash.assign");
 
 (function(window) {
+	const karma = window.__karma__;
+
 	/*
 	 * Simulate the same JSUnit Testsuite API as the TestRunner to collect the available test pages per suite
 	 */
@@ -80,11 +82,12 @@ const assign = require("lodash.assign");
 									simple: bPass
 								}, oTestPageConfig));
 							}, function(oError) {
-								karma.log("error", ["failed to load page '" + oTestPageConfig.fullpage + "'"]);
+								karma.log("error", ["Failed to load page " + oTestPageConfig.fullpage]);
+								karma.log("error", [oError]);
 								if (frame.parentNode) {
 									frame.parentNode.removeChild(frame);
 								}
-								resolve(assign({error: oError}, oTestPageConfig));
+								reject(assign({error: oError}, oTestPageConfig));
 							});
 						};
 
@@ -109,12 +112,7 @@ const assign = require("lodash.assign");
 					} else {
 						resolve(oTestPageConfig);
 					}
-				});
-			}).catch(function(e) {
-				const text = e.message;
-				karma.log("error", ["Failed to load page '" + oTestPageConfig.fullpage + "': " + text]);
-				// resolve(assign({error: text}, oTestPageConfig));
-				return;
+				}).catch(reject);
 			});
 		}
 
@@ -154,9 +152,6 @@ const assign = require("lodash.assign");
 				}).
 				then( function(aPages) {
 					return sequence(aPages);
-				}).
-				catch( function() {
-					return [];
 				});
 		}
 

test/integration.test.js

@@ -4,19 +4,38 @@ const execa = require("execa");
 
 const registerIntegrationTest = async (configPath) => {
 	it(configPath, async () => {
+		const fullConfigPath = path.join(__dirname, configPath);
+		const integrationTest = require(fullConfigPath);
 		const args = [
 			"start",
-			path.join(__dirname, configPath)
+			fullConfigPath
 		];
 		if (process.argv[process.argv.length - 1] === "--browsers=IE") {
 			// Allow switching to IE by passing a CLI arg
 			args.push("--browsers=IE");
 		}
-		await execa("karma", args, {
+		const karmaProcess = await execa("karma", args, {
 			cwd: __dirname,
-			stdio: "inherit",
-			preferLocal: true // allow executing local karma binary
+			preferLocal: true, // allow executing local karma binary
+			reject: false
 		});
+
+		console.log(configPath); // eslint-disable-line no-console
+		console.log(karmaProcess.all); // eslint-disable-line no-console
+
+		if (integrationTest.shouldFail && !karmaProcess.failed) {
+			throw new Error("Karma execution should have failed!");
+		}
+		if (!integrationTest.shouldFail && karmaProcess.failed) {
+			throw new Error("Karma execution should not have failed!");
+		}
+
+		if (integrationTest.assertions) {
+			integrationTest.assertions({
+				expect,
+				log: karmaProcess.all
+			});
+		}
 	});
 };
 

test/integration/application-ui5-tooling-error-handling/karma-empty-testsuite.conf.js

@@ -0,0 +1,19 @@
+module.exports = function(config) {
+	"use strict";
+
+	require("../karma-base.conf")(config);
+	config.set({
+
+		frameworks: ["ui5"],
+
+		ui5: {
+			testpage: "webapp/test/empty-testsuite/testsuite.qunit.html"
+		}
+
+	});
+};
+
+module.exports.shouldFail = true;
+module.exports.assertions = ({expect, log}) => {
+	expect(log).toMatch(/Could not resolve any testpages/);
+};

test/integration/application-ui5-tooling-error-handling/karma-initial-testsuite-not-found.conf.js

@@ -0,0 +1,19 @@
+module.exports = function(config) {
+	"use strict";
+
+	require("../karma-base.conf")(config);
+	config.set({
+
+		frameworks: ["ui5"],
+
+		ui5: {
+			testpage: "webapp/test/path-does-not-exist/testsuite.qunit.html"
+		}
+
+	});
+};
+
+module.exports.shouldFail = true;
+module.exports.assertions = ({expect, log}) => {
+	expect(log).toMatch(/Error resolving testsuite/);
+};

test/integration/application-ui5-tooling-error-handling/karma-testpage-QUnit-not-loaded.conf.js

@@ -0,0 +1,19 @@
+module.exports = function(config) {
+	"use strict";
+
+	require("../karma-base.conf")(config);
+	config.set({
+
+		frameworks: ["ui5"],
+
+		ui5: {
+			testpage: "webapp/test/testpage-QUnit-not-loaded/testsuite.qunit.html"
+		}
+
+	});
+};
+
+module.exports.shouldFail = true;
+module.exports.assertions = ({expect, log}) => {
+	expect(log).toMatch(/Missing QUnit framework/);
+};

test/integration/application-ui5-tooling-error-handling/karma-testpage-not-found.conf.js

@@ -0,0 +1,19 @@
+module.exports = function(config) {
+	"use strict";
+
+	require("../karma-base.conf")(config);
+	config.set({
+
+		frameworks: ["ui5"],
+
+		ui5: {
+			testpage: "webapp/test/testpage-not-found/testsuite.qunit.html"
+		}
+
+	});
+};
+
+module.exports.shouldFail = true;
+module.exports.assertions = ({expect, log}) => {
+	expect(log).toMatch(/Error while loading testpage/);
+};

test/integration/application-ui5-tooling-error-handling/karma-testsuite-promise-reject.conf.js

@@ -0,0 +1,19 @@
+module.exports = function(config) {
+	"use strict";
+
+	require("../karma-base.conf")(config);
+	config.set({
+
+		frameworks: ["ui5"],
+
+		ui5: {
+			testpage: "webapp/test/testsuite-promise-reject/testsuite.qunit.html"
+		}
+
+	});
+};
+
+module.exports.shouldFail = true;
+module.exports.assertions = ({expect, log}) => {
+	expect(log).toMatch(/Error from testsuite/);
+};

test/integration/application-ui5-tooling-error-handling/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "application-ui5-tooling-error-handling",
+  "version": "1.0.0",
+  "dependencies": {
+		"@openui5/sap.ui.core": "*"
+	}
+}

test/integration/application-ui5-tooling-error-handling/ui5.yaml

@@ -0,0 +1,5 @@
+---
+specVersion: "1.0"
+type: application
+metadata:
+  name: test.app

test/integration/application-ui5-tooling-error-handling/webapp/test/empty-testsuite/testsuite.qunit.html

@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<html>
+<head>
+	<title>Testsuite</title>
+	<script src="testsuite.qunit.js" data-sap-ui-testsuite></script>
+</head>
+<body>
+</body>
+</html>

test/integration/application-ui5-tooling-error-handling/webapp/test/empty-testsuite/testsuite.qunit.js

@@ -0,0 +1,10 @@
+/* global window, parent */
+
+window.suite = function() {
+	"use strict";
+
+	// eslint-disable-next-line
+	var oSuite = new parent.jsUnitTestSuite();
+
+	return oSuite;
+};

test/integration/application-ui5-tooling-error-handling/webapp/test/testpage-QUnit-not-loaded/test.qunit.html

@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html>
+<head>
+	<meta charset="utf-8">
+	<title>QUnit Test</title>
+
+	<script src="wrong/path/to/qunit.js"></script>
+
+</head>
+<body>
+	<div id="qunit"></div>
+</body>
+</html>

test/integration/application-ui5-tooling-error-handling/webapp/test/testpage-QUnit-not-loaded/testsuite.qunit.html

@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<html>
+<head>
+	<title>Testsuite</title>
+	<script src="testsuite.qunit.js" data-sap-ui-testsuite></script>
+</head>
+<body>
+</body>
+</html>