Improve reporting of failed test runs (#13038)

* build(test): allow specifying a junit prefix

This is specifically aimed at differentiating between Android main and Android 5.0 tests

* ci(danger): report missing test reports

This is to try and notify when the build fails or app crashes

* ci(danger): improve dependabot checks

* ci(danger): do not fail build if tests fail

As we have some unstable tests due to network conditions

Jenkinsfile

@@ -94,7 +94,7 @@ def androidUnitTests(testName, nodeVersion, npmVersion, deviceId) {
 							timeout(30) {
 								// Forcibly remove value for specific build tools version to use (set by module builds)
 								sh returnStatus: true, script: 'ti config android.buildTools.selectedVersion --remove'
-								sh label: 'Run Test Suite on emulator', script: "npm run test:integration -- android -T emulator -D test -C ${deviceId}"
+								sh label: 'Run Test Suite on emulator', script: "npm run test:integration -- android -T emulator -D test -C ${deviceId} -J ${testName}"
 							} // timeout
 						}
 					} catch (e) {

build/lib/test/index.js

@@ -25,7 +25,7 @@ async function runTests(platforms, program) {
 		fs.emptyDir(path.join(snapshotDir, '..', 'generated')),
 		fs.emptyDir(path.join(snapshotDir, '..', 'diffs'))
 	]);
-	return test(platforms, program.target, program.deviceId, program.deployType, program.deviceFamily, snapshotDir);
+	return test(platforms, program.target, program.deviceId, program.deployType, program.deviceFamily, program.junitPrefix, snapshotDir);
 }
 
 /**

build/lib/test/test.js

@@ -50,12 +50,13 @@ const isCI = !!(process.env.BUILD_NUMBER || process.env.CI || false);
  * @param {String} [deviceId] Titanium device id target to run the tests on
  * @param {string} [deployType] 'development' || 'test'
  * @param {string} [deviceFamily] 'ipad' || 'iphone'
+ * @param {string} [junitPrefix] A prefix for the junit filename
  * @param {string} [snapshotDir='../../../tests/Resources'] directory to place generated snapshot images
  * @returns {Promise<object>}
  */
-async function test(platforms, target, deviceId, deployType, deviceFamily, snapshotDir = path.join(__dirname, '../../../tests/Resources')) {
+async function test(platforms, target, deviceId, deployType, deviceFamily, junitPrefix, snapshotDir = path.join(__dirname, '../../../tests/Resources')) {
 	const snapshotPromises = []; // place to stick commands we've fired off to pull snapshot images
-
+	console.log(platforms);
 	// delete old test app (if does not exist, this will no-op)
 	await fs.remove(PROJECT_DIR);
 
@@ -70,7 +71,7 @@ async function test(platforms, target, deviceId, deployType, deviceFamily, snaps
 		const results = {};
 		for (const platform of platforms) {
 			const result = await runBuild(platform, target, deviceId, deployType, deviceFamily, snapshotDir, snapshotPromises);
-			const prefix = generateJUnitPrefix(platform, target, deviceFamily);
+			const prefix = generateJUnitPrefix(platform, target, junitPrefix || deviceFamily);
 			results[prefix] = result;
 			await outputJUnitXML(result, prefix);
 		}
@@ -902,16 +903,16 @@ function massageJSONString(testResults) {
 /**
  * @param {string} platform 'ios' || 'android'
  * @param {string} [target] 'emulator' || 'simulator' || 'device'
- * @param {string} [deviceFamily] 'iphone' || 'ipad'
+ * @param {string} [customPrefix] A custom prefix to help differentiate junit results
  * @returns {string}
  */
-function generateJUnitPrefix(platform, target, deviceFamily) {
+function generateJUnitPrefix(platform, target, customPrefix) {
 	let prefix = platform;
 	if (target) {
 		prefix += '.' + target;
 	}
-	if (deviceFamily) {
-		prefix += '.' + deviceFamily;
+	if (customPrefix) {
+		prefix += '.' + customPrefix;
 	}
 	return prefix;
 }

build/scons-test.js

@@ -9,6 +9,7 @@ program
 	.option('-v, --sdk-version [version]', 'Override the SDK version we report', process.env.PRODUCT_VERSION || version)
 	.option('-D, --deploy-type <type>', 'Override the deploy type used to build the project', /^(development|test)$/)
 	.option('-F, --device-family <value>', 'Override the device family used to build the project', /^(iphone|ipad)$/)
+	.option('-J --junit-prefix <value>', 'A prefix to add to junit tests to help distinguish them. Useful if targeting same platform and target', '')
 	.parse(process.argv);
 
 async function main(program) {

dangerfile.js

@@ -97,7 +97,7 @@ async function checkCommitMessages() {
 // Check that we have a JIRA Link in the body
 async function checkJIRA() {
 	// Don't require dependabot dependency updates require a JIRA ticket
-	if (github.pr.user.login === 'dependabot-preview[bot]') {
+	if (github.pr.user.type === 'Bot') {
 		return;
 	}
 
@@ -169,7 +169,7 @@ async function checkMergeable() {
 // Check PR author to see if it's community, etc
 async function checkCommunity() {
 	// Don't give special thanks to bot accounts
-	if (github.pr.user.login === 'dependabot-preview[bot]') {
+	if (github.pr.user.type === 'Bot') {
 		return;
 	}
 
@@ -307,6 +307,26 @@ async function linkToSDK() {
 	}
 }
 
+// Checks for the expected test reports and reports to the PR if one or more is missing
+async function checkForTestRunFailures () {
+	const expectedJunitFiles = {
+		'android.emulator.5.0': 'Android 5.0',
+		'android.emulator.main': 'Android Main',
+		'cli.report': 'CLI',
+		'ios.ipad': 'iPad',
+		'ios.iphone': 'iPhone',
+		'ios.macos': 'MacOS'
+	};
+	const files = await fs.readdir(__dirname);
+	const junitFiles = files.filter(p => p.startsWith('junit') && p.endsWith('.xml'));
+
+	if (junitFiles.length < Object.keys(expectedJunitFiles).length) {
+		const missing = Object.keys(expectedJunitFiles).filter(name => !junitFiles.includes(`junit.${name}.xml`)).map(name => expectedJunitFiles[name]);
+		fail(`Test reports missing for ${missing.join(', ')}. This indicates that a build failed or the test app crashed`);
+	}
+
+}
+
 async function main() {
 	// do a bunch of things in parallel
 	// Specifically, anything that collects what labels to add or remove has to be done first before...
@@ -317,14 +337,15 @@ async function main() {
 		checkJIRA(),
 		linkToSDK(),
 		checkForIOSCrash(),
-		junit({ pathToReport: './junit.*.xml' }),
+		junit({ pathToReport: './junit.*.xml', onlyWarn: true }),
 		checkChangedFileLocations(),
 		checkCommunity(),
 		checkMergeable(),
 		checkPRisApproved(),
 		updateMilestone(),
 		eslint(),
 		dependencies({ type: 'npm' }),
+		checkForTestRunFailures()
 	]);
 
 	// ...once we've gathered what labels to add/remove, do that last

package.json

@@ -123,7 +123,7 @@
     "@seadub/clang-format-lint": "0.0.2",
     "@seadub/danger-plugin-dependencies": "1.0.0",
     "@seadub/danger-plugin-eslint": "^2.0.0",
-    "@seadub/danger-plugin-junit": "0.2.0",
+    "@seadub/danger-plugin-junit": "^0.3.0",
     "babel-plugin-transform-titanium": "^0.1.1",
     "chai": "^4.3.4",
     "clang-format": "1.5.0",