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
This commit is contained in:
Timmy Willison 2024-03-11 10:39:38 -04:00 committed by GitHub
parent ae67ace649
commit 822362e6ef
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 94 additions and 38 deletions

View File

@ -62,4 +62,4 @@ jobs:
run: npm run pretest run: npm run pretest
- name: Run tests - 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

View File

@ -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 fullBrowser = getBrowserString( worker.browser );
const verbose = worker.options.verbose; const verbose = worker.options.verbose;
try { try {
@ -74,13 +92,7 @@ async function ensureAcknowledged( worker, restarts ) {
return worker; return worker;
} catch ( error ) { } catch ( error ) {
console.error( error.message ); console.error( error.message );
await cleanupWorker( worker, { verbose } ); await restartWorker( worker.browser );
await createBrowserWorker(
worker.url,
worker.browser,
worker.options,
restarts + 1
);
} }
} }
@ -132,7 +144,7 @@ export async function createBrowserWorker( url, browser, options, restarts = 0 )
// Wait for the worker to show up in the list // Wait for the worker to show up in the list
// before returning it. // before returning it.
return ensureAcknowledged( worker, restarts ); return ensureAcknowledged( worker );
} }
export async function setBrowserWorkerUrl( browser, url ) { export async function setBrowserWorkerUrl( browser, url ) {
@ -159,13 +171,7 @@ export async function checkLastTouches() {
}min.` }min.`
); );
} }
await cleanupWorker( worker, options ); await restartWorker( worker );
await createBrowserWorker(
worker.url,
worker.browser,
options,
worker.restarts
);
} }
} }
} }

View File

@ -1,6 +1,11 @@
import chalk from "chalk"; import chalk from "chalk";
import { getBrowserString } from "../lib/getBrowserString.js"; 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; const TEST_POLL_TIMEOUT = 1000;
@ -32,6 +37,9 @@ export function getNextBrowserTest( reportId ) {
} }
export function retryTest( reportId, maxRetries ) { export function retryTest( reportId, maxRetries ) {
if ( !maxRetries ) {
return;
}
const test = queue.find( ( test ) => test.id === reportId ); const test = queue.find( ( test ) => test.id === reportId );
if ( test ) { if ( test ) {
test.retries++; 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 ) { export function addBrowserStackRun( url, browser, options ) {
queue.push( { queue.push( {
browser, browser,
fullBrowser: getBrowserString( browser ), fullBrowser: getBrowserString( browser ),
hardRetries: 0,
id: options.reportId, id: options.reportId,
url, url,
options, options,
@ -59,7 +88,7 @@ export function addBrowserStackRun( url, browser, options ) {
} }
export async function runAllBrowserStack() { export async function runAllBrowserStack() {
return new Promise( async( resolve, reject )=> { return new Promise( async( resolve, reject ) => {
while ( queue.length ) { while ( queue.length ) {
try { try {
await checkLastTouches(); await checkLastTouches();

View File

@ -64,12 +64,6 @@ const argv = yargs( process.argv.slice( 2 ) )
type: "boolean", type: "boolean",
description: "Log additional information." 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", { .option( "run-id", {
type: "string", type: "string",
description: "A unique identifier for this run." 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, " + "Otherwise, the --browser option will be used, " +
"with the latest version/device for that browser, on a matching OS." "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", { .option( "list-browsers", {
type: "string", type: "string",
description: description:

View File

@ -26,23 +26,29 @@ export async function createTestServer( report ) {
// Add a script tag to the index.html to load the QUnit listeners // Add a script tag to the index.html to load the QUnit listeners
app.use( /\/test(?:\/index.html)?\//, ( _req, res ) => { app.use( /\/test(?:\/index.html)?\//, ( _req, res ) => {
res.send( indexHTML.replace( res.send(
indexHTML.replace(
"</head>", "</head>",
"<script src=\"/test/runner/listeners.js\"></script></head>" "<script src=\"/test/runner/listeners.js\"></script></head>"
) ); )
);
} ); } );
// Bind the reporter // Bind the reporter
app.post( "/api/report", bodyParser.json( { limit: "50mb" } ), ( req, res ) => { app.post(
"/api/report",
bodyParser.json( { limit: "50mb" } ),
async( req, res ) => {
if ( report ) { if ( report ) {
const response = report( req.body ); const response = await report( req.body );
if ( response ) { if ( response ) {
res.json( response ); res.json( response );
return; return;
} }
} }
res.sendStatus( 204 ); res.sendStatus( 204 );
} ); }
);
// Handle errors from the body parser // Handle errors from the body parser
app.use( bodyParserErrorHandler() ); app.use( bodyParserErrorHandler() );

View File

@ -14,6 +14,7 @@ import { cleanupAllBrowsers, touchBrowser } from "./browserstack/browsers.js";
import { import {
addBrowserStackRun, addBrowserStackRun,
getNextBrowserTest, getNextBrowserTest,
hardRetryTest,
retryTest, retryTest,
runAllBrowserStack runAllBrowserStack
} from "./browserstack/queue.js"; } from "./browserstack/queue.js";
@ -30,6 +31,7 @@ export async function run( {
concurrency, concurrency,
debug, debug,
esm, esm,
hardRetries,
headless, headless,
isolate, isolate,
modules = [], modules = [],
@ -72,7 +74,7 @@ export async function run( {
// Create the test app and // Create the test app and
// hook it up to the reporter // hook it up to the reporter
const reports = Object.create( null ); const reports = Object.create( null );
const app = await createTestServer( ( message ) => { const app = await createTestServer( async( message ) => {
switch ( message.type ) { switch ( message.type ) {
case "testEnd": { case "testEnd": {
const reportId = message.id; const reportId = message.id;
@ -120,6 +122,11 @@ export async function run( {
if ( retry ) { if ( retry ) {
return retry; return retry;
} }
// Return early if hardRetryTest returns true
if ( await hardRetryTest( reportId, hardRetries ) ) {
return;
}
errorMessages.push( ...Object.values( pendingErrors[ reportId ] ) ); errorMessages.push( ...Object.values( pendingErrors[ reportId ] ) );
} }