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

- 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
index cb637661ffa4f1b63598afe3debda42c3d63f84d..07313a11e8fcfe6cdf5a84980d29b6350b2efe05 100644 (file)
--- 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
index 3a7da4fc9f0da459adf15788f435ced7b5b58b7f..218d13fe72edd18ae98fbf1b336f6bca7188a869 100644 (file)
--- a/test/runner/browserstack/browsers.js
+++ b/test/runner/browserstack/browsers.js
@@ -66,7 +66,25 @@ 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 {
@@ -74,13 +92,7 @@ async function ensureAcknowledged( worker, restarts ) {
                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
index c948f29bf95542c29a278ac4ba5c88ca8e5dc03c..6d1c8d51fe6bee45068105744901c8aa3d2bf5de 100644 (file)
--- 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
index 83c90066a8287ba4f0f8814729a2dbde2e1e5c1d..9cc73fefacc284e41931ccd7109b84d49d5e3743 100644 (file)
--- 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
index d9ea9e28097ef0276bb40dee62a3bdd98bc9a52c..c591917e5e42d6078dfc0fed385edf7a13140b5c 100644 (file)
--- 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
index 4874fb6f96b8385dcf216545d35e682594bc5f31..0e13e015e206a84971302b98973c2a798922b86a 100644 (file)
--- 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 ] ) );
                                }