Tests: add --hard-retries option to test runner

timmywil · web-flow · commit 822362e6efae · 2024-03-11T10:39:38.000-04:00
- Add the ability to retry by restarting the worker and getting a different browser instance, after all normal retries have been exhausted. This can sometimes be successful when a refresh is not. Close gh-5438
diff --git a/.github/workflows/browserstack.yml b/.github/workflows/browserstack.yml
@@ -62,4 +62,4 @@ jobs:
         run: npm run pretest
 
       - name: Run tests
-        run: npm run test:unit -- -v --browserstack "${{ matrix.BROWSER }}" --run-id ${{ github.run_id }} --isolate --retries 3
+        run: npm run test:unit -- -v --browserstack "${{ matrix.BROWSER }}" --run-id ${{ github.run_id }} --isolate --retries 3 --hard-retries 1
diff --git a/test/runner/browserstack/browsers.js b/test/runner/browserstack/browsers.js
@@ -66,21 +66,33 @@ async function waitForAck( worker, { fullBrowser, verbose } ) {
 	} );
 }
 
-async function ensureAcknowledged( worker, restarts ) {
+async function restartWorker( worker ) {
+	await cleanupWorker( worker, worker.options );
+	await createBrowserWorker(
+		worker.url,
+		worker.browser,
+		worker.options,
+		worker.restarts + 1
+	);
+}
+
+export async function restartBrowser( browser ) {
+	const fullBrowser = getBrowserString( browser );
+	const worker = workers[ fullBrowser ];
+	if ( worker ) {
+		await restartWorker( worker );
+	}
+}
+
+async function ensureAcknowledged( worker ) {
 	const fullBrowser = getBrowserString( worker.browser );
 	const verbose = worker.options.verbose;
 	try {
 		await waitForAck( worker, { fullBrowser, verbose } );
 		return worker;
 	} catch ( error ) {
 		console.error( error.message );
-		await cleanupWorker( worker, { verbose } );
-		await createBrowserWorker(
-			worker.url,
-			worker.browser,
-			worker.options,
-			restarts + 1
-		);
+		await restartWorker( worker.browser );
 	}
 }
 
@@ -132,7 +144,7 @@ export async function createBrowserWorker( url, browser, options, restarts = 0 )
 
 	// Wait for the worker to show up in the list
 	// before returning it.
-	return ensureAcknowledged( worker, restarts );
+	return ensureAcknowledged( worker );
 }
 
 export async function setBrowserWorkerUrl( browser, url ) {
@@ -159,13 +171,7 @@ export async function checkLastTouches() {
 					}min.`
 				);
 			}
-			await cleanupWorker( worker, options );
-			await createBrowserWorker(
-				worker.url,
-				worker.browser,
-				options,
-				worker.restarts
-			);
+			await restartWorker( worker );
 		}
 	}
 }
diff --git a/test/runner/browserstack/queue.js b/test/runner/browserstack/queue.js
@@ -1,6 +1,11 @@
 import chalk from "chalk";
 import { getBrowserString } from "../lib/getBrowserString.js";
-import { checkLastTouches, createBrowserWorker, setBrowserWorkerUrl } from "./browsers.js";
+import {
+	checkLastTouches,
+	createBrowserWorker,
+	restartBrowser,
+	setBrowserWorkerUrl
+} from "./browsers.js";
 
 const TEST_POLL_TIMEOUT = 1000;
 
@@ -32,6 +37,9 @@ export function getNextBrowserTest( reportId ) {
 }
 
 export function retryTest( reportId, maxRetries ) {
+	if ( !maxRetries ) {
+		return;
+	}
 	const test = queue.find( ( test ) => test.id === reportId );
 	if ( test ) {
 		test.retries++;
@@ -46,10 +54,31 @@ export function retryTest( reportId, maxRetries ) {
 	}
 }
 
+export async function hardRetryTest( reportId, maxHardRetries ) {
+	if ( !maxHardRetries ) {
+		return false;
+	}
+	const test = queue.find( ( test ) => test.id === reportId );
+	if ( test ) {
+		test.hardRetries++;
+		if ( test.hardRetries <= maxHardRetries ) {
+			console.log(
+				`Hard retrying test ${ reportId } for ${ chalk.yellow(
+					test.options.modules.join( ", " )
+				) }...${ test.hardRetries }`
+			);
+			await restartBrowser( test.browser );
+			return true;
+		}
+	}
+	return false;
+}
+
 export function addBrowserStackRun( url, browser, options ) {
 	queue.push( {
 		browser,
 		fullBrowser: getBrowserString( browser ),
+		hardRetries: 0,
 		id: options.reportId,
 		url,
 		options,
@@ -59,7 +88,7 @@ export function addBrowserStackRun( url, browser, options ) {
 }
 
 export async function runAllBrowserStack() {
-	return new Promise( async( resolve, reject )=> {
+	return new Promise( async( resolve, reject ) => {
 		while ( queue.length ) {
 			try {
 				await checkLastTouches();
diff --git a/test/runner/command.js b/test/runner/command.js
@@ -64,12 +64,6 @@ const argv = yargs( process.argv.slice( 2 ) )
 		type: "boolean",
 		description: "Log additional information."
 	} )
-	.option( "retries", {
-		alias: "r",
-		type: "number",
-		description: "Number of times to retry failed tests in BrowserStack.",
-		implies: [ "browserstack" ]
-	} )
 	.option( "run-id", {
 		type: "string",
 		description: "A unique identifier for this run."
@@ -90,6 +84,20 @@ const argv = yargs( process.argv.slice( 2 ) )
 			"Otherwise, the --browser option will be used, " +
 			"with the latest version/device for that browser, on a matching OS."
 	} )
+	.option( "retries", {
+		alias: "r",
+		type: "number",
+		description: "Number of times to retry failed tests in BrowserStack.",
+		implies: [ "browserstack" ]
+	} )
+	.option( "hard-retries", {
+		type: "number",
+		description:
+			"Number of times to retry failed tests in BrowserStack " +
+			"by restarting the worker. This is in addition to the normal retries " +
+			"and are only used when the normal retries are exhausted.",
+		implies: [ "browserstack" ]
+	} )
 	.option( "list-browsers", {
 		type: "string",
 		description:
diff --git a/test/runner/createTestServer.js b/test/runner/createTestServer.js
@@ -26,23 +26,29 @@ export async function createTestServer( report ) {
 
 	// Add a script tag to the index.html to load the QUnit listeners
 	app.use( /\/test(?:\/index.html)?\//, ( _req, res ) => {
-		res.send( indexHTML.replace(
-			"</head>",
-			"<script src=\"/test/runner/listeners.js\"></script></head>"
-		) );
+		res.send(
+			indexHTML.replace(
+				"</head>",
+				"<script src=\"/test/runner/listeners.js\"></script></head>"
+			)
+		);
 	} );
 
 	// Bind the reporter
-	app.post( "/api/report", bodyParser.json( { limit: "50mb" } ), ( req, res ) => {
-		if ( report ) {
-			const response = report( req.body );
-			if ( response ) {
-				res.json( response );
-				return;
+	app.post(
+		"/api/report",
+		bodyParser.json( { limit: "50mb" } ),
+		async( req, res ) => {
+			if ( report ) {
+				const response = await report( req.body );
+				if ( response ) {
+					res.json( response );
+					return;
+				}
 			}
+			res.sendStatus( 204 );
 		}
-		res.sendStatus( 204 );
-	} );
+	);
 
 	// Handle errors from the body parser
 	app.use( bodyParserErrorHandler() );
diff --git a/test/runner/run.js b/test/runner/run.js
@@ -14,6 +14,7 @@ import { cleanupAllBrowsers, touchBrowser } from "./browserstack/browsers.js";
 import {
 	addBrowserStackRun,
 	getNextBrowserTest,
+	hardRetryTest,
 	retryTest,
 	runAllBrowserStack
 } from "./browserstack/queue.js";
@@ -30,6 +31,7 @@ export async function run( {
 	concurrency,
 	debug,
 	esm,
+	hardRetries,
 	headless,
 	isolate,
 	modules = [],
@@ -72,7 +74,7 @@ export async function run( {
 	// Create the test app and
 	// hook it up to the reporter
 	const reports = Object.create( null );
-	const app = await createTestServer( ( message ) => {
+	const app = await createTestServer( async( message ) => {
 		switch ( message.type ) {
 			case "testEnd": {
 				const reportId = message.id;
@@ -120,6 +122,11 @@ export async function run( {
 					if ( retry ) {
 						return retry;
 					}
+
+					// Return early if hardRetryTest returns true
+					if ( await hardRetryTest( reportId, hardRetries ) ) {
+						return;
+					}
 					errorMessages.push( ...Object.values( pendingErrors[ reportId ] ) );
 				}
 
