From 9d31db37a8e45755e9956925a18976597426ceed Mon Sep 17 00:00:00 2001 From: Timmy Willison Date: Sat, 2 Mar 2024 09:37:27 -0500 Subject: [PATCH 1/6] Tests: reuse browser workers in BrowserStack tests - add support for "latest" and "latest-1" in browser version filters - add support for specifying non-final browser versions, such as beta versions --- .github/workflows/browserstack.yml | 21 +- eslint.config.js | 33 ++- package.json | 14 +- test/runner/browserstack/api.js | 109 ++++--- test/runner/browserstack/browsers.js | 196 +++++++++++++ .../browserstack/buildBrowserFromString.js | 14 +- test/runner/browserstack/queue.js | 81 +++++ test/runner/browserstack/workers.js | 276 ------------------ test/runner/command.js | 29 +- test/runner/createTestServer.js | 6 +- test/runner/jsdom.js | 6 +- test/runner/lib/buildTestUrl.js | 6 +- test/runner/listeners.js | 13 +- test/runner/run.js | 144 +++++---- test/runner/selenium/createDriver.js | 3 +- test/runner/{ => selenium}/queue.js | 50 +--- test/runner/selenium/runSelenium.js | 1 - 17 files changed, 546 insertions(+), 456 deletions(-) create mode 100644 test/runner/browserstack/browsers.js create mode 100644 test/runner/browserstack/queue.js delete mode 100644 test/runner/browserstack/workers.js rename test/runner/{ => selenium}/queue.js (57%) diff --git a/.github/workflows/browserstack.yml b/.github/workflows/browserstack.yml index 7350ca36d4..b3f9a5e981 100644 --- a/.github/workflows/browserstack.yml +++ b/.github/workflows/browserstack.yml @@ -16,24 +16,25 @@ jobs: name: ${{ matrix.BROWSER }} concurrency: group: ${{ github.workflow }} ${{ matrix.BROWSER }} + timeout-minutes: 30 strategy: fail-fast: false matrix: BROWSER: - 'IE_11' - - 'Safari_17' - - 'Safari_16' - - 'Chrome_120' - - 'Chrome_119' - - 'Edge_120' - - 'Edge_119' - - 'Firefox_121' - - 'Firefox_120' + - 'Safari_latest' + - 'Safari_latest-1' + - 'Chrome_latest' + - 'Chrome_latest-1' + - 'Opera_latest' + - 'Edge_latest' + - 'Edge_latest-1' + - 'Firefox_latest' + - 'Firefox_latest-1' - 'Firefox_115' - '__iOS_17' - '__iOS_16' - '__iOS_15' - - 'Opera_106' steps: - name: Checkout uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 @@ -61,4 +62,4 @@ jobs: run: npm run pretest - name: Run tests - run: npm run test:unit -- -v --browserstack "${{ matrix.BROWSER }}" --retries 3 + run: npm run test:unit -- -v --browserstack "${{ matrix.BROWSER }}" --run-id ${{ github.run_id }} --isolate --retries 3 diff --git a/eslint.config.js b/eslint.config.js index dabff31574..952d39c71c 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -21,8 +21,7 @@ export default [ "test/node_smoke_tests/commonjs/**", "test/node_smoke_tests/module/**", "test/promises_aplus_adapters/**", - "test/middleware-mockserver.cjs", - "test/runner/**/*.js" + "test/middleware-mockserver.cjs" ], languageOptions: { globals: { @@ -35,13 +34,6 @@ export default [ } }, - { - files: [ "test/runner/listeners.js" ], - languageOptions: { - sourceType: "script" - } - }, - // Source { files: [ "src/**" ], @@ -222,6 +214,29 @@ export default [ } }, + { + files: [ + "test/runner/**/*.js" + ], + languageOptions: { + globals: { + ...globals.node + }, + sourceType: "module" + }, + rules: { + ...jqueryConfig.rules + } + }, + + { + files: [ "test/runner/listeners.js" ], + languageOptions: { + ecmaVersion: 5, + sourceType: "script" + } + }, + { files: [ "test/data/testrunner.js", diff --git a/package.json b/package.json index c976f42ca6..147df86624 100644 --- a/package.json +++ b/package.json @@ -54,18 +54,18 @@ "pretest": "npm run qunit-fixture && npm run babel:tests && npm run npmcopy", "qunit-fixture": "node build/tasks/qunit-fixture.js", "start": "node -e \"require('./build/tasks/build.js').buildDefaultFiles({ watch: true })\"", - "test:browser": "npm run pretest && npm run build:main && npm run test:unit -- -b chrome -b firefox --no-isolate -h", + "test:browser": "npm run pretest && npm run build:main && npm run test:unit -- -b chrome -b firefox -h", "test:browserless": "npm run pretest && npm run build:all && node build/tasks/node_smoke_tests.js && node build/tasks/promises_aplus_tests.js && npm run test:unit -- -b jsdom -m basic", "test:jsdom": "npm run pretest && npm run build:main && npm run test:unit -- -b jsdom -m basic", "test:node_smoke_tests": "npm run pretest && npm run build:all && node build/tasks/node_smoke_tests.js", "test:promises_aplus": "npm run build:main && node build/tasks/promises_aplus_tests.js", - "test:firefox": "npm run pretest && npm run build:main && npm run test:unit -- -v -b firefox --no-isolate -h", - "test:safari": "npm run pretest && npm run build:main && npm run test:unit -- -b safari --no-isolate", + "test:firefox": "npm run pretest && npm run build:main && npm run test:unit -- -v -b firefox -h", + "test:safari": "npm run pretest && npm run build:main && npm run test:unit -- -b safari", "test:server": "node test/runner/server.js", - "test:esm": "npm run pretest && npm run build:main && npm run test:unit -- --esm --no-isolate -h", - "test:no-deprecated": "npm run pretest && npm run build -- -e deprecated && npm run test:unit -- --no-isolate -h", - "test:selector-native": "npm run pretest && npm run build -- -e selector && npm run test:unit -- --no-isolate -h", - "test:slim": "npm run pretest && npm run build -- --slim && npm run test:unit -- --no-isolate -h", + "test:esm": "npm run pretest && npm run build:main && npm run test:unit -- --esm -h", + "test:no-deprecated": "npm run pretest && npm run build -- -e deprecated && npm run test:unit -- -h", + "test:selector-native": "npm run pretest && npm run build -- -e selector && npm run test:unit -- -h", + "test:slim": "npm run pretest && npm run build -- --slim && npm run test:unit -- -h", "test:unit": "node test/runner/command.js", "test": "npm run build:all && npm run lint && npm run test:browserless && npm run test:browser && npm run test:esmodules && npm run test:slim && npm run test:no-deprecated && npm run test:selector-native" }, diff --git a/test/runner/browserstack/api.js b/test/runner/browserstack/api.js index 40982c9b39..632f90c3b4 100644 --- a/test/runner/browserstack/api.js +++ b/test/runner/browserstack/api.js @@ -14,6 +14,7 @@ const accessKey = process.env.BROWSERSTACK_ACCESS_KEY; // iOS has null for version numbers, // and we do not need a similar check for OS versions. const rfinalVersion = /(?:^[0-9\.]+$)|(?:^null$)/; +const rlatest = /^latest-(\d+)$/; const rnonDigits = /(?:[^\d\.]+)|(?:20\d{2})/g; @@ -84,6 +85,15 @@ function compareVersionNumbers( a, b ) { return 1; } } + + if ( rnonDigits.test( a ) && !rnonDigits.test( b ) ) { + return -1; + } + if ( !rnonDigits.test( a ) && rnonDigits.test( b ) ) { + return 1; + } + + return 0; } function sortBrowsers( a, b ) { @@ -148,17 +158,62 @@ export async function filterBrowsers( filter ) { const filterOsVersion = ( filter.os_version ?? "" ).toLowerCase(); const filterDevice = ( filter.device ?? "" ).toLowerCase(); - return browsers.filter( ( browser ) => { + const filteredWithoutVersion = browsers.filter( ( browser ) => { return ( ( !filterBrowser || filterBrowser === browser.browser.toLowerCase() ) && - ( !filterVersion || - matchVersion( browser.browser_version, filterVersion ) ) && ( !filterOs || filterOs === browser.os.toLowerCase() ) && - ( !filterOsVersion || - filterOsVersion === browser.os_version.toLowerCase() ) && + ( !filterOsVersion || matchVersion( browser.os_version, filterOsVersion ) ) && ( !filterDevice || filterDevice === ( browser.device || "" ).toLowerCase() ) ); } ); + + if ( !filterVersion ) { + return filteredWithoutVersion; + } + + if ( filterVersion.startsWith( "latest" ) ) { + const groupedByName = filteredWithoutVersion + .filter( ( b ) => rfinalVersion.test( b.browser_version ) ) + .reduce( ( acc, browser ) => { + acc[ browser.browser ] = acc[ browser.browser ] ?? []; + acc[ browser.browser ].push( browser ); + return acc; + }, Object.create( null ) ); + + const filtered = []; + for ( const group of Object.values( groupedByName ) ) { + const latest = group[ group.length - 1 ]; + + // Mobile devices do not have browser version. + // Skip the version check for these, + // but include the latest in the list if it made it + // through filtering. + if ( !latest.browser_version ) { + + // Do not include in the list for latest-n. + if ( filterVersion === "latest" ) { + filtered.push( latest ); + } + continue; + } + + // Get the latest version and subtract the number from the filter, + // ignoring any patch versions, which may differ between major versions. + const num = rlatest.exec( filterVersion ); + const version = parseInt( latest.browser_version ) - ( num ? num[ 1 ] : 0 ); + const match = group.findLast( ( browser ) => { + return matchVersion( browser.browser_version, version.toString() ); + } ); + if ( match ) { + filtered.push( match ); + } + } + return filtered; + } + + return filteredWithoutVersion.filter( ( browser ) => { + return matchVersion( browser.browser_version, filterVersion ); + } ); } export async function listBrowsers( filter ) { @@ -177,13 +232,11 @@ export async function listBrowsers( filter ) { } export async function getLatestBrowser( filter ) { + if ( !filter.browser_version ) { + filter.browser_version = "latest"; + } const browsers = await filterBrowsers( filter ); - - // The list is sorted in ascending order, - // so the last item is the latest. - return browsers.findLast( ( browser ) => - rfinalVersion.test( browser.browser_version ) - ); + return browsers[ browsers.length - 1 ]; } /** @@ -229,31 +282,14 @@ export function getWorker( id ) { return fetchAPI( `/worker/${ id }` ); } -export async function deleteWorker( id, verbose ) { - await fetchAPI( `/worker/${ id }`, { method: "DELETE" } ); - if ( verbose ) { - console.log( `\nWorker ${ id } stopped.` ); - } +export async function deleteWorker( id ) { + return fetchAPI( `/worker/${ id }`, { method: "DELETE" } ); } export function getWorkers() { return fetchAPI( "/workers" ); } -/** - * Change the URL of a worker, - * or refresh if it's the same URL. - */ -export function changeUrl( id, url ) { - return fetchAPI( `/worker/${ id }/url.json`, { - method: "PUT", - body: JSON.stringify( { - timeout: 20, - url: encodeURI( url ) - } ) - } ); -} - /** * Stop all workers */ @@ -262,15 +298,17 @@ export async function stopWorkers() { // Run each request on its own // to avoid connect timeout errors. + console.log( `${ workers.length } workers running...` ); for ( const worker of workers ) { try { - await deleteWorker( worker.id, true ); + await deleteWorker( worker.id ); } catch ( error ) { // Log the error, but continue trying to remove workers. console.error( error ); } } + console.log( "All workers stopped." ); } /** @@ -284,6 +322,11 @@ export function getPlan() { } export async function getAvailableSessions() { - const [ plan, workers ] = await Promise.all( [ getPlan(), getWorkers() ] ); - return plan.parallel_sessions_max_allowed - workers.length; + try { + const [ plan, workers ] = await Promise.all( [ getPlan(), getWorkers() ] ); + return plan.parallel_sessions_max_allowed - workers.length; + } catch ( error ) { + console.error( error ); + return 0; + } } diff --git a/test/runner/browserstack/browsers.js b/test/runner/browserstack/browsers.js new file mode 100644 index 0000000000..01ce94c41e --- /dev/null +++ b/test/runner/browserstack/browsers.js @@ -0,0 +1,196 @@ +import chalk from "chalk"; +import { getBrowserString } from "../lib/getBrowserString.js"; +import { createWorker, deleteWorker, getAvailableSessions } from "./api.js"; + +const workers = Object.create( null ); + +/** + * Keys are browser strings + * Structure of a worker: + * { + * debug: boolean, // Stops the worker from being cleaned up when finished + * id: string, + * lastTouch: number, // The last time a request was received + * url: string, + * browser: object, // The browser object + * options: object // The options to create the worker + * } + */ + +// Acknowledge the worker within the time limit. +// BrowserStack can take much longer spinning up +// some browsers, such as iOS 15 Safari. +const ACKNOWLEDGE_INTERVAL = 1000; +const ACKNOWLEDGE_TIMEOUT = 60 * 1000 * 5; + +const MAX_WORKER_RESTARTS = 5; + +// No report after the time limit +// should refresh the worker +const RUN_WORKER_TIMEOUT = 60 * 1000 * 2; + +const WORKER_WAIT_TIME = 30000; + +export function touchBrowser( browser ) { + const fullBrowser = getBrowserString( browser ); + const worker = workers[ fullBrowser ]; + if ( worker ) { + worker.lastTouch = Date.now(); + } +} + +async function waitForAck( worker, { fullBrowser, verbose } ) { + delete worker.lastTouch; + return new Promise( ( resolve, reject ) => { + const interval = setInterval( () => { + if ( worker.lastTouch ) { + if ( verbose ) { + console.log( `\n${ fullBrowser } acknowledged.` ); + } + clearTimeout( timeout ); + clearInterval( interval ); + resolve(); + } + }, ACKNOWLEDGE_INTERVAL ); + + const timeout = setTimeout( () => { + clearInterval( interval ); + reject( + new Error( + `${ fullBrowser } not acknowledged after ${ + ACKNOWLEDGE_TIMEOUT / 1000 + }s.` + ) + ); + }, ACKNOWLEDGE_TIMEOUT ); + } ); +} + +async function ensureAcknowledged( worker, restarts ) { + const fullBrowser = getBrowserString( worker.browser ); + const verbose = worker.options.verbose; + try { + await waitForAck( worker, { fullBrowser, verbose } ); + return worker; + } catch ( error ) { + await cleanupWorker( worker, { verbose } ); + await createBrowserWorker( + worker.url, + worker.browser, + worker.options, + restarts + 1 + ); + } +} + +export async function createBrowserWorker( url, browser, options, restarts = 0 ) { + if ( restarts > MAX_WORKER_RESTARTS ) { + throw new Error( + `Reached the maximum number of restarts for ${ chalk.yellow( + getBrowserString( browser ) + ) }` + ); + } + const verbose = options.verbose; + while ( ( await getAvailableSessions() ) <= 0 ) { + if ( verbose ) { + console.log( "\nWaiting for available sessions..." ); + } + await new Promise( ( resolve ) => setTimeout( resolve, WORKER_WAIT_TIME ) ); + } + + const { debug, runId, tunnelId } = options; + const fullBrowser = getBrowserString( browser ); + + const worker = await createWorker( { + ...browser, + url: encodeURI( url ), + project: "jquery", + build: `Run ${ runId }`, + + // Set the max here, so that we can + // control the timeout + timeout: 1800, + + // Not documented in the API docs, + // but required to make local testing work. + // See https://www.browserstack.com/docs/automate/selenium/manage-multiple-connections#nodejs + "browserstack.local": true, + "browserstack.localIdentifier": tunnelId + } ); + + browser.debug = !!debug; + worker.url = url; + worker.browser = browser; + worker.restarts = restarts; + worker.options = options; + touchBrowser( browser ); + workers[ fullBrowser ] = worker; + + // Wait for the worker to show up in the list + // before returning it. + return ensureAcknowledged( worker, restarts ); +} + +export async function setBrowserWorkerUrl( browser, url ) { + const fullBrowser = getBrowserString( browser ); + const worker = workers[ fullBrowser ]; + if ( worker ) { + worker.url = url; + } +} + +/** + * Checks that all browsers have received + * a response in the given amount of time. + * If not, the worker is restarted. + */ +export async function checkLastTouches() { + for ( const [ fullBrowser, worker ] of Object.entries( workers ) ) { + if ( Date.now() - worker.lastTouch > RUN_WORKER_TIMEOUT ) { + const options = worker.options; + if ( options.verbose ) { + console.log( + `\nNo response from ${ chalk.yellow( fullBrowser ) } in ${ + RUN_WORKER_TIMEOUT / 1000 / 60 + }min.` + ); + } + await cleanupWorker( worker, options ); + await createBrowserWorker( + worker.url, + worker.browser, + options, + worker.restarts + ); + } + } +} + +export async function cleanupWorker( worker, { verbose } ) { + for ( const [ fullBrowser, w ] of Object.entries( workers ) ) { + if ( w === worker ) { + delete workers[ fullBrowser ]; + await deleteWorker( worker.id ); + if ( verbose ) { + console.log( `\nStopped ${ fullBrowser }.` ); + } + return; + } + } +} + +export async function cleanupAllBrowsers( { verbose } ) { + const workersRemaining = Object.values( workers ); + const numRemaining = workersRemaining.length; + if ( numRemaining ) { + await Promise.all( + workersRemaining.map( ( worker ) => deleteWorker( worker.id ) ) + ); + if ( verbose ) { + console.log( + `Stopped ${ numRemaining } browser${ numRemaining > 1 ? "s" : "" }...` + ); + } + } +} diff --git a/test/runner/browserstack/buildBrowserFromString.js b/test/runner/browserstack/buildBrowserFromString.js index 55aa38053b..e0d99a0392 100644 --- a/test/runner/browserstack/buildBrowserFromString.js +++ b/test/runner/browserstack/buildBrowserFromString.js @@ -3,8 +3,18 @@ export function buildBrowserFromString( str ) { // If the version starts with a colon, it's a device if ( versionOrDevice && versionOrDevice.startsWith( ":" ) ) { - return { browser, device: versionOrDevice.slice( 1 ), os, os_version: osVersion }; + return { + browser, + device: versionOrDevice.slice( 1 ), + os, + os_version: osVersion + }; } - return { browser, browser_version: versionOrDevice, os, os_version: osVersion }; + return { + browser, + browser_version: versionOrDevice, + os, + os_version: osVersion + }; } diff --git a/test/runner/browserstack/queue.js b/test/runner/browserstack/queue.js new file mode 100644 index 0000000000..6107e89d8b --- /dev/null +++ b/test/runner/browserstack/queue.js @@ -0,0 +1,81 @@ +import chalk from "chalk"; +import { getBrowserString } from "../lib/getBrowserString.js"; +import { checkLastTouches, createBrowserWorker, setBrowserWorkerUrl } from "./browsers.js"; + +const TEST_POLL_TIMEOUT = 1000; + +const queue = []; + +export function getNextBrowserTest( reportId ) { + const index = queue.findIndex( ( test ) => test.id === reportId ); + if ( index === -1 ) { + return; + } + const previousTest = queue[ index ]; + queue.splice( index, 1 ); + for ( const test of queue.slice( index ) ) { + if ( test.fullBrowser === previousTest.fullBrowser ) { + setBrowserWorkerUrl( test.browser, test.url ); + test.running = true; + return { url: test.url }; + } + } +} + +export function retryTest( reportId, maxRetries ) { + const test = queue.find( ( test ) => test.id === reportId ); + if ( test ) { + test.retries++; + if ( test.retries <= maxRetries ) { + console.log( + `Retrying test ${ reportId } for ${ chalk.yellow( + test.options.modules.join( ", " ) + ) }...` + ); + return test; + } + } +} + +export function addBrowserStackRun( url, browser, options ) { + queue.push( { + browser, + fullBrowser: getBrowserString( browser ), + id: options.reportId, + url, + options, + retries: 0, + running: false + } ); +} + +export async function runAllBrowserStack( options ) { + return new Promise( async( resolve, reject )=> { + while ( queue.length ) { + try { + await checkLastTouches( options ); + } catch ( error ) { + reject( error ); + } + + // Run one test per browser at a time + const browsersTaken = []; + for ( const test of queue ) { + if ( browsersTaken.indexOf( test.fullBrowser ) > -1 ) { + continue; + } + browsersTaken.push( test.fullBrowser ); + if ( !test.running ) { + test.running = true; + try { + await createBrowserWorker( test.url, test.browser, test.options ); + } catch ( error ) { + reject( error ); + } + } + } + await new Promise( ( resolve ) => setTimeout( resolve, TEST_POLL_TIMEOUT ) ); + } + resolve(); + } ); +} diff --git a/test/runner/browserstack/workers.js b/test/runner/browserstack/workers.js deleted file mode 100644 index 8f0ab68f04..0000000000 --- a/test/runner/browserstack/workers.js +++ /dev/null @@ -1,276 +0,0 @@ -import chalk from "chalk"; -import { getBrowserString } from "../lib/getBrowserString.js"; -import { changeUrl, createWorker, deleteWorker, getWorker } from "./api.js"; - -const workers = Object.create( null ); - -// Acknowledge the worker within the time limit. -// BrowserStack can take much longer spinning up -// some browsers, such as iOS 15 Safari. -const ACKNOWLEDGE_WORKER_TIMEOUT = 60 * 1000 * 8; -const ACKNOWLEDGE_WORKER_INTERVAL = 1000; - -// No report after the time limit -// should refresh the worker -const RUN_WORKER_TIMEOUT = 60 * 1000 * 2; -const MAX_WORKER_RESTARTS = 5; -const MAX_WORKER_REFRESHES = 1; -const POLL_WORKER_TIMEOUT = 1000; - -export async function cleanupWorker( reportId, verbose ) { - const worker = workers[ reportId ]; - if ( worker ) { - try { - delete workers[ reportId ]; - await deleteWorker( worker.id, verbose ); - } catch ( error ) { - console.error( error ); - } - } -} - -export function debugWorker( reportId ) { - const worker = workers[ reportId ]; - if ( worker ) { - worker.debug = true; - } -} - -/** - * Set the last time a request was - * received related to the worker. - */ -export function touchWorker( reportId ) { - const worker = workers[ reportId ]; - if ( worker ) { - worker.lastTouch = Date.now(); - } -} - -export function retryTest( reportId, retries ) { - const worker = workers[ reportId ]; - if ( worker ) { - worker.retries ||= 0; - worker.retries++; - if ( worker.retries <= retries ) { - worker.retry = true; - console.log( `\nRetrying test ${ reportId }...${ worker.retries }` ); - return true; - } - } - return false; -} - -export async function cleanupAllWorkers( verbose ) { - const workersRemaining = Object.keys( workers ).length; - if ( workersRemaining ) { - if ( verbose ) { - console.log( - `Stopping ${ workersRemaining } stray worker${ - workersRemaining > 1 ? "s" : "" - }...` - ); - } - await Promise.all( - Object.values( workers ).map( ( worker ) => deleteWorker( worker.id, verbose ) ) - ); - } -} - -async function waitForAck( id, verbose ) { - return new Promise( ( resolve, reject ) => { - const interval = setInterval( () => { - const worker = workers[ id ]; - if ( !worker ) { - clearTimeout( timeout ); - clearInterval( interval ); - return reject( new Error( `Worker ${ id } not found.` ) ); - } - if ( worker.lastTouch ) { - if ( verbose ) { - console.log( `\nWorker ${ id } acknowledged.` ); - } - clearTimeout( timeout ); - clearInterval( interval ); - resolve(); - } - }, ACKNOWLEDGE_WORKER_INTERVAL ); - const timeout = setTimeout( () => { - clearInterval( interval ); - const worker = workers[ id ]; - reject( - new Error( - `Worker ${ - worker ? worker.id : "" - } for test ${ id } not acknowledged after ${ - ACKNOWLEDGE_WORKER_TIMEOUT / 1000 - }s.` - ) - ); - }, ACKNOWLEDGE_WORKER_TIMEOUT ); - } ); -} - -export async function runWorker( - url, - browser, - options, - restarts = 0 -) { - const { modules, reportId, runId, verbose } = options; - const worker = await createWorker( { - ...browser, - url: encodeURI( url ), - project: "jquery", - build: `Run ${ runId }`, - name: `${ modules.join( "," ) } (${ reportId })`, - - // Set the max here, so that we can - // control the timeout - timeout: 1800, - - // Not documented in the API docs, - // but required to make local testing work. - // See https://www.browserstack.com/docs/automate/selenium/manage-multiple-connections#nodejs - "browserstack.local": true, - "browserstack.localIdentifier": runId - } ); - - workers[ reportId ] = worker; - - const timeMessage = `\nWorker ${ - worker.id - } created for test ${ reportId } (${ chalk.yellow( getBrowserString( browser ) ) })`; - - if ( verbose ) { - console.time( timeMessage ); - } - - async function retryWorker() { - await cleanupWorker( reportId, verbose ); - if ( verbose ) { - console.log( `Retrying worker for test ${ reportId }...${ restarts + 1 }` ); - } - return runWorker( url, browser, options, restarts + 1 ); - } - - // Wait for the worker to be acknowledged - try { - await waitForAck( reportId ); - } catch ( error ) { - if ( !workers[ reportId ] ) { - - // The worker has already been cleaned up - return; - } - - if ( restarts < MAX_WORKER_RESTARTS ) { - return retryWorker(); - } - - throw error; - } - - if ( verbose ) { - console.timeEnd( timeMessage ); - } - - let refreshes = 0; - let loggedStarted = false; - return new Promise( ( resolve, reject ) => { - async function refreshWorker() { - try { - await changeUrl( worker.id, url ); - touchWorker( reportId ); - return tick(); - } catch ( error ) { - if ( !workers[ reportId ] ) { - - // The worker has already been cleaned up - return resolve(); - } - console.error( error ); - return retryWorker().then( resolve, reject ); - } - } - - async function checkWorker() { - const worker = workers[ reportId ]; - - if ( !worker || worker.debug ) { - return resolve(); - } - - let fetchedWorker; - try { - fetchedWorker = await getWorker( worker.id ); - } catch ( error ) { - return reject( error ); - } - if ( - !fetchedWorker || - ( fetchedWorker.status !== "running" && fetchedWorker.status !== "queue" ) - ) { - return resolve(); - } - - if ( verbose && !loggedStarted ) { - loggedStarted = true; - console.log( - `\nTest ${ chalk.bold( reportId ) } is ${ - worker.status === "running" ? "running" : "in the queue" - }.` - ); - console.log( ` View at ${ fetchedWorker.browser_url }.` ); - } - - // Refresh the worker if a retry is triggered - if ( worker.retry ) { - worker.retry = false; - - // Reset recovery refreshes - refreshes = 0; - return refreshWorker(); - } - - if ( worker.lastTouch > Date.now() - RUN_WORKER_TIMEOUT ) { - return tick(); - } - - refreshes++; - - if ( refreshes >= MAX_WORKER_REFRESHES ) { - if ( restarts < MAX_WORKER_RESTARTS ) { - if ( verbose ) { - console.log( - `Worker ${ worker.id } not acknowledged after ${ - ACKNOWLEDGE_WORKER_TIMEOUT / 1000 - }s.` - ); - } - return retryWorker().then( resolve, reject ); - } - await cleanupWorker( reportId, verbose ); - return reject( - new Error( - `Worker ${ worker.id } for test ${ reportId } timed out after ${ MAX_WORKER_RESTARTS } restarts.` - ) - ); - } - - if ( verbose ) { - console.log( - `\nRefreshing worker ${ worker.id } for test ${ reportId }...${ refreshes }` - ); - } - - return refreshWorker(); - } - - function tick() { - setTimeout( checkWorker, POLL_WORKER_TIMEOUT ); - } - - checkWorker(); - } ); -} diff --git a/test/runner/command.js b/test/runner/command.js index 8419625a77..d5b67e8600 100644 --- a/test/runner/command.js +++ b/test/runner/command.js @@ -48,7 +48,8 @@ const argv = yargs( process.argv.slice( 2 ) ) type: "number", description: "Run tests in parallel in multiple browsers. " + - "Defaults to 8 in normal mode. In browserstack mode, defaults to the maximum available under your BrowserStack plan." + "Defaults to 8 in normal mode. In browserstack mode, " + + "defaults to the maximum available under your BrowserStack plan." } ) .option( "debug", { alias: "d", @@ -65,21 +66,28 @@ const argv = yargs( process.argv.slice( 2 ) ) .option( "retries", { alias: "r", type: "number", - description: "Number of times to retry failed tests.", - default: 0 + description: "Number of times to retry failed tests in BrowserStack.", + implies: [ "browserstack" ] } ) - .option( "no-isolate", { + .option( "run-id", { + type: "string", + description: "A unique identifier for this run." + } ) + .option( "isolate", { type: "boolean", - description: "Run all modules in the same browser instance." + description: "Run each module by itself in the test page. This can extend testing time." } ) .option( "browserstack", { type: "array", description: - "Run tests in BrowserStack.\nRequires BROWSERSTACK_USERNAME and BROWSERSTACK_ACCESS_KEY environment variables.\n" + + "Run tests in BrowserStack.\n" + + "Requires BROWSERSTACK_USERNAME and BROWSERSTACK_ACCESS_KEY environment variables.\n" + "The value can be empty for the default configuration, or a string in the format of\n" + "\"browser_[browserVersion | :device]_os_osVersion\" (see --list-browsers).\n" + - "Pass multiple browsers by repeating the option. The --browser option is ignored when --browserstack has a value.\n" + - "Otherwise, the --browser option will be used, with the latest version/device for that browser, on a matching OS." + "Pass multiple browsers by repeating the option.\n" + + "The --browser option is ignored when --browserstack has a value.\n" + + "Otherwise, the --browser option will be used, " + + "with the latest version/device for that browser, on a matching OS." } ) .option( "list-browsers", { type: "string", @@ -88,8 +96,11 @@ const argv = yargs( process.argv.slice( 2 ) ) "Leave blank to view all browsers or pass " + "\"browser_[browserVersion | :device]_os_osVersion\" with each parameter " + "separated by an underscore to filter the list (any can be omitted).\n" + + "\"latest\" can be used in place of \"browserVersion\" to find the latest version.\n" + + "\"latest-n\" can be used to find the nth latest browser version.\n" + "Use a colon to indicate a device.\n" + - "Examples: \"chrome__windows_10\", \"Mobile Safari\", \"Android Browser_:Google Pixel 8 Pro\".\n" + + "Examples: \"chrome__windows_10\", \"safari_latest\", " + + "\"Mobile Safari\", \"Android Browser_:Google Pixel 8 Pro\".\n" + "Use quotes if spaces are necessary." } ) .option( "stop-workers", { diff --git a/test/runner/createTestServer.js b/test/runner/createTestServer.js index ebc6bd4bbd..b78fed2780 100644 --- a/test/runner/createTestServer.js +++ b/test/runner/createTestServer.js @@ -35,7 +35,11 @@ export async function createTestServer( report ) { // Bind the reporter app.post( "/api/report", bodyParser.json( { limit: "50mb" } ), ( req, res ) => { if ( report ) { - report( req.body ); + const response = report( req.body ); + if ( response ) { + res.json( response ); + return; + } } res.sendStatus( 204 ); } ); diff --git a/test/runner/jsdom.js b/test/runner/jsdom.js index d370ac3482..d9ff9dda74 100644 --- a/test/runner/jsdom.js +++ b/test/runner/jsdom.js @@ -24,7 +24,7 @@ export async function runJSDOM( url, { reportId, verbose } ) { } ); } -export function cleanupJSDOM( reportId, verbose ) { +export function cleanupJSDOM( reportId, { verbose } ) { const window = windows[ reportId ]; if ( window ) { if ( window.finish ) { @@ -38,7 +38,7 @@ export function cleanupJSDOM( reportId, verbose ) { } } -export function cleanupAllJSDOM( verbose ) { +export function cleanupAllJSDOM( { verbose } ) { const windowsRemaining = Object.keys( windows ).length; if ( windowsRemaining ) { if ( verbose ) { @@ -49,7 +49,7 @@ export function cleanupAllJSDOM( verbose ) { ); } for ( const id in windows ) { - cleanupJSDOM( id, verbose ); + cleanupJSDOM( id, { verbose } ); } } } diff --git a/test/runner/lib/buildTestUrl.js b/test/runner/lib/buildTestUrl.js index 6e0f1a9b02..9d1a4b413c 100644 --- a/test/runner/lib/buildTestUrl.js +++ b/test/runner/lib/buildTestUrl.js @@ -1,6 +1,6 @@ import { generateModuleId } from "./generateHash.js"; -export function buildTestUrl( modules, { browserstack, esm, jsdom, port, reportId } ) { +export function buildTestUrl( modules, { amd, browserstack, jsdom, port, reportId } ) { if ( !port ) { throw new Error( "No port specified." ); } @@ -10,8 +10,8 @@ export function buildTestUrl( modules, { browserstack, esm, jsdom, port, reportI query.append( "moduleId", generateModuleId( module ) ); } - if ( esm ) { - query.append( "esmodules", "true" ); + if ( amd ) { + query.append( "amd", "true" ); } if ( jsdom ) { diff --git a/test/runner/listeners.js b/test/runner/listeners.js index a3c52c21e8..cca2bbd622 100644 --- a/test/runner/listeners.js +++ b/test/runner/listeners.js @@ -68,6 +68,7 @@ request.open( "POST", "/api/report", true ); request.setRequestHeader( "Content-Type", "application/json" ); request.send( json ); + return request; } // Send acknowledgement to the server. @@ -83,6 +84,16 @@ // childSuites is large and unused. data.childSuites = undefined; - send( "runEnd", data ); + var request = send( "runEnd", data ); + request.onload = function() { + if ( request.status === 200 && request.responseText ) { + try { + var json = JSON.parse( request.responseText ); + window.location = json.url; + } catch ( e ) { + console.error( e ); + } + } + }; } ); } )(); diff --git a/test/runner/run.js b/test/runner/run.js index 89b0c5eb7b..610d933f52 100644 --- a/test/runner/run.js +++ b/test/runner/run.js @@ -4,38 +4,40 @@ import { getLatestBrowser } from "./browserstack/api.js"; import { buildBrowserFromString } from "./browserstack/buildBrowserFromString.js"; import { localTunnel } from "./browserstack/local.js"; import { reportEnd, reportTest } from "./reporter.js"; -import { - cleanupAllWorkers, - cleanupWorker, - debugWorker, - retryTest, - touchWorker -} from "./browserstack/workers.js"; import { createTestServer } from "./createTestServer.js"; import { buildTestUrl } from "./lib/buildTestUrl.js"; import { generateHash, printModuleHashes } from "./lib/generateHash.js"; import { getBrowserString } from "./lib/getBrowserString.js"; -import { addRun, runFullQueue } from "./queue.js"; import { cleanupAllJSDOM, cleanupJSDOM } from "./jsdom.js"; import { modules as allModules } from "./modules.js"; +import { cleanupAllBrowsers, touchBrowser } from "./browserstack/browsers.js"; +import { + addBrowserStackRun, + getNextBrowserTest, + retryTest, + runAllBrowserStack +} from "./browserstack/queue.js"; +import { addSeleniumRun, runAllSelenium } from "./selenium/queue.js"; const EXIT_HOOK_WAIT_TIMEOUT = 60 * 1000; /** * Run modules in parallel in different browser instances. */ -export async function run( { - browsers: browserNames, - browserstack, - concurrency, - debug, - esm, - headless, - isolate = true, - modules = [], - retries = 3, - verbose -} = {} ) { +export async function run( options = {} ) { + let { + amd, + browsers: browserNames, + browserstack, + debug, + headless, + isolate, + modules = [], + retries = 0, + runId, + verbose + } = options; + if ( !browserNames || !browserNames.length ) { browserNames = [ "chrome" ]; } @@ -57,23 +59,27 @@ export async function run( { // Convert browser names to browser objects let browsers = browserNames.map( ( b ) => ( { browser: b } ) ); - - // A unique identifier for this run - const runId = generateHash( + const tunnelId = generateHash( `${ Date.now() }-${ modules.join( ":" ) }-${ ( browserstack || [] ) .concat( browserNames ) .join( ":" ) }` ); + // A unique identifier for this run + if ( !runId ) { + runId = tunnelId; + } + // Create the test app and // hook it up to the reporter const reports = Object.create( null ); - const app = await createTestServer( async( message ) => { + const app = await createTestServer( ( message ) => { switch ( message.type ) { case "testEnd": { const reportId = message.id; - touchWorker( reportId ); - const errors = reportTest( message.data, reportId, reports[ reportId ] ); + const report = reports[ reportId ]; + touchBrowser( report.browser ); + const errors = reportTest( message.data, reportId, report ); pendingErrors[ reportId ] ||= {}; if ( errors ) { pendingErrors[ reportId ][ message.data.name ] = errors; @@ -85,6 +91,7 @@ export async function run( { case "runEnd": { const reportId = message.id; const report = reports[ reportId ]; + touchBrowser( report.browser ); const { failed, total } = reportEnd( message.data, message.id, @@ -92,31 +99,34 @@ export async function run( { ); report.total = total; + cleanupJSDOM( reportId, { verbose } ); + + // Handle failure if ( failed ) { - if ( !retryTest( reportId, retries ) ) { - if ( debug ) { - debugWorker( reportId ); - } - errorMessages.push( ...Object.values( pendingErrors[ reportId ] ) ); + const retry = retryTest( reportId, retries ); + if ( retry ) { + return retry; } - } else { - if ( Object.keys( pendingErrors[ reportId ] ).length ) { - console.warn( "Detected flaky tests:" ); - for ( const [ , error ] in Object.entries( pendingErrors[ reportId ] ) ) { - console.warn( chalk.italic( chalk.gray( error ) ) ); - } - delete pendingErrors[ reportId ]; + errorMessages.push( ...Object.values( pendingErrors[ reportId ] ) ); + return getNextBrowserTest( reportId ); + } + + // Handle success + if ( + pendingErrors[ reportId ] && + Object.keys( pendingErrors[ reportId ] ).length + ) { + console.warn( "Detected flaky tests:" ); + for ( const [ , error ] in Object.entries( pendingErrors[ reportId ] ) ) { + console.warn( chalk.italic( chalk.gray( error ) ) ); } + delete pendingErrors[ reportId ]; } - await cleanupWorker( reportId, verbose ); - cleanupJSDOM( reportId, verbose ); - break; + return getNextBrowserTest( reportId ); } case "ack": { - touchWorker( message.id ); - if ( verbose ) { - console.log( `\nWorker for test ${ message.id } acknowledged.` ); - } + const report = reports[ message.id ]; + touchBrowser( report.browser ); break; } default: @@ -165,8 +175,8 @@ export async function run( { } } - await cleanupAllWorkers( verbose ); - cleanupAllJSDOM( verbose ); + await cleanupAllBrowsers( { verbose } ); + cleanupAllJSDOM( { verbose } ); } asyncExitHook( @@ -210,13 +220,16 @@ export async function run( { const latestMatch = await getLatestBrowser( browser ); if ( !latestMatch ) { - throw new Error( `Browser not found: ${ getBrowserString( browser ) }.` ); + console.error( + chalk.red( `Browser not found: ${ getBrowserString( browser ) }.` ) + ); + gracefulExit( 1 ); } return latestMatch; } ) ); - tunnel = await localTunnel( runId ); + tunnel = await localTunnel( tunnelId ); if ( verbose ) { console.log( "Started BrowserStackLocal." ); @@ -230,22 +243,29 @@ export async function run( { reports[ reportId ] = { browser, headless, modules }; const url = buildTestUrl( modules, { + amd, browserstack, - esm, jsdom: browser.browser === "jsdom", port, reportId } ); - addRun( url, browser, { - debug, - headless, - modules, - reportId, - retries, - runId, - verbose - } ); + if ( browserstack ) { + addBrowserStackRun( url, browser, { + ...options, + modules, + reportId, + runId, + tunnelId + } ); + } else { + addSeleniumRun( url, browser, { + ...options, + modules, + reportId, + runId + } ); + } } for ( const browser of browsers ) { @@ -260,7 +280,11 @@ export async function run( { try { console.log( `Starting Run ${ runId }...` ); - await runFullQueue( { browserstack, concurrency, verbose } ); + if ( browserstack ) { + await runAllBrowserStack( { options, ...runId } ); + } else { + await runAllSelenium( options ); + } } catch ( error ) { console.error( error ); if ( !debug ) { diff --git a/test/runner/selenium/createDriver.js b/test/runner/selenium/createDriver.js index 765ebb847f..d1680b22d0 100644 --- a/test/runner/selenium/createDriver.js +++ b/test/runner/selenium/createDriver.js @@ -55,7 +55,8 @@ export default async function createDriver( { browserName, headless, verbose } ) edgeOptions.addArguments( "--headless=new" ); if ( !browserSupportsHeadless( browserName ) ) { console.log( - `Headless mode is not supported for ${ browserName }. Running in normal mode instead.` + `Headless mode is not supported for ${ browserName }.` + + "Running in normal mode instead." ); } } diff --git a/test/runner/queue.js b/test/runner/selenium/queue.js similarity index 57% rename from test/runner/queue.js rename to test/runner/selenium/queue.js index 4c9d66d8fe..3ea4b36be1 100644 --- a/test/runner/queue.js +++ b/test/runner/selenium/queue.js @@ -3,33 +3,27 @@ // and refills the queue when one promise resolves. import chalk from "chalk"; -import { getAvailableSessions } from "./browserstack/api.js"; -import { runWorker } from "./browserstack/workers.js"; -import { getBrowserString } from "./lib/getBrowserString.js"; -import { runSelenium } from "./selenium/runSelenium.js"; -import { runJSDOM } from "./jsdom.js"; +import { getBrowserString } from "../lib/getBrowserString.js"; +import { runSelenium } from "./runSelenium.js"; +import { runJSDOM } from "../jsdom.js"; -const queue = []; const promises = []; +const queue = []; const SELENIUM_WAIT_TIME = 100; -const BROWSERSTACK_WAIT_TIME = 5000; -const WORKER_WAIT_TIME = 30000; // Limit concurrency to 8 by default in selenium // BrowserStack defaults to the max allowed by the plan // More than this will log MaxListenersExceededWarning const MAX_CONCURRENCY = 8; -export function addRun( url, browser, options ) { +export function addSeleniumRun( url, browser, options ) { queue.push( { url, browser, options } ); } -export async function runFullQueue( { - browserstack, - concurrency: defaultConcurrency, - verbose -} ) { +// TODO: Possibly move this to selenium-only +// and have a queue per browser +export async function runAllSelenium( { concurrency = MAX_CONCURRENCY, verbose } ) { while ( queue.length ) { const next = queue.shift(); const { url, browser, options } = next; @@ -43,39 +37,15 @@ export async function runFullQueue( { // Wait enough time between requests // to give concurrency a chance to update. // In selenium, this helps avoid undici connect timeout errors. - await new Promise( ( resolve ) => - setTimeout( - resolve, - browserstack ? BROWSERSTACK_WAIT_TIME : SELENIUM_WAIT_TIME - ) - ); - - const concurrency = - browserstack && !defaultConcurrency ? - await getAvailableSessions() : - defaultConcurrency || MAX_CONCURRENCY; + await new Promise( ( resolve ) => setTimeout( resolve, SELENIUM_WAIT_TIME ) ); if ( verbose ) { - console.log( - `\nConcurrency: ${ concurrency }. Tests remaining: ${ queue.length + 1 }.` - ); - } - - // If concurrency is 0, wait a bit and try again - if ( concurrency <= 0 ) { - if ( verbose ) { - console.log( "\nWaiting for available sessions..." ); - } - queue.unshift( next ); - await new Promise( ( resolve ) => setTimeout( resolve, WORKER_WAIT_TIME ) ); - continue; + console.log( `\nTests remaining: ${ queue.length + 1 }.` ); } let promise; if ( browser.browser === "jsdom" ) { promise = runJSDOM( url, options ); - } else if ( browserstack ) { - promise = runWorker( url, browser, options ); } else { promise = runSelenium( url, browser, options ); } diff --git a/test/runner/selenium/runSelenium.js b/test/runner/selenium/runSelenium.js index 247cd8472e..848db36c72 100644 --- a/test/runner/selenium/runSelenium.js +++ b/test/runner/selenium/runSelenium.js @@ -1,4 +1,3 @@ -import chalk from "chalk"; import createDriver from "./createDriver.js"; export async function runSelenium( From aea5039af1724c0a1870f40b6e8f595014f5e202 Mon Sep 17 00:00:00 2001 From: Timmy Willison Date: Mon, 4 Mar 2024 08:47:34 -0500 Subject: [PATCH 2/6] Tests: use minutes in acknowledge timeout message --- test/runner/browserstack/browsers.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/runner/browserstack/browsers.js b/test/runner/browserstack/browsers.js index 01ce94c41e..1261bf7a70 100644 --- a/test/runner/browserstack/browsers.js +++ b/test/runner/browserstack/browsers.js @@ -58,8 +58,8 @@ async function waitForAck( worker, { fullBrowser, verbose } ) { reject( new Error( `${ fullBrowser } not acknowledged after ${ - ACKNOWLEDGE_TIMEOUT / 1000 - }s.` + ACKNOWLEDGE_TIMEOUT / 1000 / 60 + }min.` ) ); }, ACKNOWLEDGE_TIMEOUT ); @@ -73,6 +73,7 @@ async function ensureAcknowledged( worker, restarts ) { await waitForAck( worker, { fullBrowser, verbose } ); return worker; } catch ( error ) { + console.error( error.message ); await cleanupWorker( worker, { verbose } ); await createBrowserWorker( worker.url, From da3540c6d27cb5f21161ee256eec42d73a02a65c Mon Sep 17 00:00:00 2001 From: Timmy Willison Date: Tue, 5 Mar 2024 10:24:15 -0500 Subject: [PATCH 3/6] fixup! PR feedback --- test/data/testinit.js | 1 - test/index.html | 2 +- test/runner/browserstack/browsers.js | 2 +- test/runner/browserstack/queue.js | 15 ++++++-- test/runner/run.js | 56 ++++++++++++++-------------- test/runner/selenium/queue.js | 2 - 6 files changed, 41 insertions(+), 37 deletions(-) diff --git a/test/data/testinit.js b/test/data/testinit.js index a04bc49c7f..80c1911e45 100644 --- a/test/data/testinit.js +++ b/test/data/testinit.js @@ -433,7 +433,6 @@ this.loadTests = function() { } } else { - QUnit.load(); /** * Run in noConflict mode diff --git a/test/index.html b/test/index.html index 523cc1eb27..87fe941642 100644 --- a/test/index.html +++ b/test/index.html @@ -35,7 +35,7 @@ // We need to read both. var esmodules = QUnit.config.esmodules || QUnit.urlParams.esmodules; - // `loadTests()` will call `QUnit.load()` because tests + // `loadTests()` will call `QUnit.start()` because tests // such as unit/ready.js should run after document ready. if ( !esmodules ) { loadTests(); diff --git a/test/runner/browserstack/browsers.js b/test/runner/browserstack/browsers.js index 1261bf7a70..f5e022a831 100644 --- a/test/runner/browserstack/browsers.js +++ b/test/runner/browserstack/browsers.js @@ -190,7 +190,7 @@ export async function cleanupAllBrowsers( { verbose } ) { ); if ( verbose ) { console.log( - `Stopped ${ numRemaining } browser${ numRemaining > 1 ? "s" : "" }...` + `Stopped ${ numRemaining } browser${ numRemaining > 1 ? "s" : "" }.` ); } } diff --git a/test/runner/browserstack/queue.js b/test/runner/browserstack/queue.js index 6107e89d8b..10ef14a2b8 100644 --- a/test/runner/browserstack/queue.js +++ b/test/runner/browserstack/queue.js @@ -11,12 +11,21 @@ export function getNextBrowserTest( reportId ) { if ( index === -1 ) { return; } + + // Remove the completed test from the queue const previousTest = queue[ index ]; queue.splice( index, 1 ); + + // Find the next test for the same browser for ( const test of queue.slice( index ) ) { if ( test.fullBrowser === previousTest.fullBrowser ) { + + // Set the URL for our tracking setBrowserWorkerUrl( test.browser, test.url ); test.running = true; + + // Return the URL for the next test. + // listeners.js will use this to set the browser URL. return { url: test.url }; } } @@ -49,16 +58,16 @@ export function addBrowserStackRun( url, browser, options ) { } ); } -export async function runAllBrowserStack( options ) { +export async function runAllBrowserStack() { return new Promise( async( resolve, reject )=> { while ( queue.length ) { try { - await checkLastTouches( options ); + await checkLastTouches(); } catch ( error ) { reject( error ); } - // Run one test per browser at a time + // Run one test URL per browser at a time const browsersTaken = []; for ( const test of queue ) { if ( browsersTaken.indexOf( test.fullBrowser ) > -1 ) { diff --git a/test/runner/run.js b/test/runner/run.js index 610d933f52..52f80e40c1 100644 --- a/test/runner/run.js +++ b/test/runner/run.js @@ -24,20 +24,19 @@ const EXIT_HOOK_WAIT_TIMEOUT = 60 * 1000; /** * Run modules in parallel in different browser instances. */ -export async function run( options = {} ) { - let { - amd, - browsers: browserNames, - browserstack, - debug, - headless, - isolate, - modules = [], - retries = 0, - runId, - verbose - } = options; - +export async function run( { + amd, + browsers: browserNames, + browserstack, + concurrency, + debug, + headless, + isolate, + modules = [], + retries = 0, + runId, + verbose +} ) { if ( !browserNames || !browserNames.length ) { browserNames = [ "chrome" ]; } @@ -250,21 +249,20 @@ export async function run( options = {} ) { reportId } ); + const options = { + debug, + headless, + modules, + reportId, + runId, + tunnelId, + verbose + }; + if ( browserstack ) { - addBrowserStackRun( url, browser, { - ...options, - modules, - reportId, - runId, - tunnelId - } ); + addBrowserStackRun( url, browser, options ); } else { - addSeleniumRun( url, browser, { - ...options, - modules, - reportId, - runId - } ); + addSeleniumRun( url, browser, options ); } } @@ -281,9 +279,9 @@ export async function run( options = {} ) { try { console.log( `Starting Run ${ runId }...` ); if ( browserstack ) { - await runAllBrowserStack( { options, ...runId } ); + await runAllBrowserStack( { verbose } ); } else { - await runAllSelenium( options ); + await runAllSelenium( { concurrency, verbose } ); } } catch ( error ) { console.error( error ); diff --git a/test/runner/selenium/queue.js b/test/runner/selenium/queue.js index 3ea4b36be1..863db4d9b1 100644 --- a/test/runner/selenium/queue.js +++ b/test/runner/selenium/queue.js @@ -21,8 +21,6 @@ export function addSeleniumRun( url, browser, options ) { queue.push( { url, browser, options } ); } -// TODO: Possibly move this to selenium-only -// and have a queue per browser export async function runAllSelenium( { concurrency = MAX_CONCURRENCY, verbose } ) { while ( queue.length ) { const next = queue.shift(); From 2b6f86d65d4752ae6437f1011a12ad254c1a1297 Mon Sep 17 00:00:00 2001 From: Timmy Willison Date: Tue, 5 Mar 2024 13:22:53 -0500 Subject: [PATCH 4/6] fixup! amd -> back to esm --- test/runner/lib/buildTestUrl.js | 6 +++--- test/runner/run.js | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/runner/lib/buildTestUrl.js b/test/runner/lib/buildTestUrl.js index 9d1a4b413c..6e0f1a9b02 100644 --- a/test/runner/lib/buildTestUrl.js +++ b/test/runner/lib/buildTestUrl.js @@ -1,6 +1,6 @@ import { generateModuleId } from "./generateHash.js"; -export function buildTestUrl( modules, { amd, browserstack, jsdom, port, reportId } ) { +export function buildTestUrl( modules, { browserstack, esm, jsdom, port, reportId } ) { if ( !port ) { throw new Error( "No port specified." ); } @@ -10,8 +10,8 @@ export function buildTestUrl( modules, { amd, browserstack, jsdom, port, reportI query.append( "moduleId", generateModuleId( module ) ); } - if ( amd ) { - query.append( "amd", "true" ); + if ( esm ) { + query.append( "esmodules", "true" ); } if ( jsdom ) { diff --git a/test/runner/run.js b/test/runner/run.js index 52f80e40c1..c2e4f70581 100644 --- a/test/runner/run.js +++ b/test/runner/run.js @@ -25,11 +25,11 @@ const EXIT_HOOK_WAIT_TIMEOUT = 60 * 1000; * Run modules in parallel in different browser instances. */ export async function run( { - amd, browsers: browserNames, browserstack, concurrency, debug, + esm, headless, isolate, modules = [], @@ -242,8 +242,8 @@ export async function run( { reports[ reportId ] = { browser, headless, modules }; const url = buildTestUrl( modules, { - amd, browserstack, + esm, jsdom: browser.browser === "jsdom", port, reportId From 8c9b629884128eb594c29e6e1e79b57035672625 Mon Sep 17 00:00:00 2001 From: Timmy Willison Date: Tue, 5 Mar 2024 13:29:40 -0500 Subject: [PATCH 5/6] fixup! more PR feedback --- test/runner/browserstack/browsers.js | 6 ++++-- test/runner/run.js | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/test/runner/browserstack/browsers.js b/test/runner/browserstack/browsers.js index f5e022a831..957c9aac8d 100644 --- a/test/runner/browserstack/browsers.js +++ b/test/runner/browserstack/browsers.js @@ -109,8 +109,10 @@ export async function createBrowserWorker( url, browser, options, restarts = 0 ) project: "jquery", build: `Run ${ runId }`, - // Set the max here, so that we can - // control the timeout + // This is the maximum timeout allowed + // by BrowserStack. We do this because + // we control the timeout in the runner. + // See https://github.com/browserstack/api/blob/b324a6a5bc1b6052510d74e286b8e1c758c308a7/README.md#timeout300 timeout: 1800, // Not documented in the API docs, diff --git a/test/runner/run.js b/test/runner/run.js index c2e4f70581..2c90863b0e 100644 --- a/test/runner/run.js +++ b/test/runner/run.js @@ -79,7 +79,7 @@ export async function run( { const report = reports[ reportId ]; touchBrowser( report.browser ); const errors = reportTest( message.data, reportId, report ); - pendingErrors[ reportId ] ||= {}; + pendingErrors[ reportId ] ??= Object.create( null ); if ( errors ) { pendingErrors[ reportId ][ message.data.name ] = errors; } else { From 70e31ceac455e5a9bf5312e1b370273f8da78b18 Mon Sep 17 00:00:00 2001 From: Timmy Willison Date: Tue, 5 Mar 2024 13:45:36 -0500 Subject: [PATCH 6/6] fixup! enable strict mode for yargs command; fix npm test --- package.json | 2 +- test/runner/command.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 147df86624..e27ea8a40b 100644 --- a/package.json +++ b/package.json @@ -67,7 +67,7 @@ "test:selector-native": "npm run pretest && npm run build -- -e selector && npm run test:unit -- -h", "test:slim": "npm run pretest && npm run build -- --slim && npm run test:unit -- -h", "test:unit": "node test/runner/command.js", - "test": "npm run build:all && npm run lint && npm run test:browserless && npm run test:browser && npm run test:esmodules && npm run test:slim && npm run test:no-deprecated && npm run test:selector-native" + "test": "npm run build:all && npm run lint && npm run test:browserless && npm run test:browser && npm run test:esm && npm run test:slim && npm run test:no-deprecated && npm run test:selector-native" }, "homepage": "https://jquery.com", "author": { diff --git a/test/runner/command.js b/test/runner/command.js index d5b67e8600..83c90066a8 100644 --- a/test/runner/command.js +++ b/test/runner/command.js @@ -7,6 +7,7 @@ import { run } from "./run.js"; const argv = yargs( process.argv.slice( 2 ) ) .version( false ) + .strict() .command( { command: "[options]", describe: "Run jQuery tests in a browser"