build: switch to macos-13 to fix missing Firefox

macOS 14 and macOS 15 are exclusively available as Arm64, which Microsoft
has catered to Google and its Chrome browser only, omitting Firefox.

The paid "large" GitHub Action runners do offer macOS 14/15 on Intel
and those do include Firefox, even on the latest version.

Ref actions/runner-images#9974

.github/workflows/CI.yaml

@@ -27,9 +27,9 @@ jobs:
             node: 20.x
 
           # Includes Firefox, Google Chrome, Safari
-          # https://github.com/actions/runner-images/blob/macos-15/20241217.493/images/macos/macos-15-Readme.md
+          # https://github.com/actions/runner-images/blob/macos-13/20241216.479/images/macos/macos-13-Readme.md
           - name: "macOS: Node 20"
-            os: macos-15
+            os: macos-13
             node: 20.x
 
     name: ${{ matrix.name }}

src/browsers.js

@@ -33,15 +33,16 @@ class LocalBrowser {
     let exe;
     for (const candidate of candidates) {
       if (candidate !== null) {
-        logger.debug('browser_exe_check', candidate);
         // Optimization: Use fs.existsSync. It is on par with accessSync and statSync,
         // and beats concurrent fs/promises.access(cb) via Promise.all().
         // Starting the promise chain alone takes the same time as a loop with
         // 5x existsSync(), not even counting the await and boilerplate to manage it all.
         if (fs.existsSync(candidate)) {
-          logger.debug('browser_exe_found');
+          logger.debug('browser_exe_found', candidate);
           exe = candidate;
           break;
+        } else {
+          logger.debug('browser_exe_check', candidate);
         }
       }
     }
@@ -162,7 +163,7 @@ class LocalBrowser {
 }
 
 class FirefoxBrowser {
-  * getCandidates () {
+  static * #getCandidates () {
     if (process.env.FIREFOX_BIN) yield process.env.FIREFOX_BIN;
 
     // Find /usr/bin/firefox on platforms like linux (including WSL), freebsd, openbsd.
@@ -183,7 +184,7 @@ class FirefoxBrowser {
     // if (LocalBrowser.isWsl()) { }
   }
 
-  static createPrefsJs (prefs) {
+  static #createPrefsJs (prefs) {
     let js = '';
     for (const key in prefs) {
       js += 'user_pref("' + key + '", ' + JSON.stringify(prefs[key]) + ');\n';
@@ -203,7 +204,7 @@ class FirefoxBrowser {
     // https://github.com/airtap/the-last-browser-launcher/blob/v0.1.1/lib/launch/index.js
     // https://github.com/karma-runner/karma-firefox-launcher/blob/v2.1.3/index.js
     logger.debug('firefox_prefs_create', 'Creating temporary prefs.js file');
-    fs.writeFileSync(path.join(profileDir, 'prefs.js'), FirefoxBrowser.createPrefsJs({
+    fs.writeFileSync(path.join(profileDir, 'prefs.js'), FirefoxBrowser.#createPrefsJs({
       'app.update.disabledForTesting': true, // Disable auto-updates
       'browser.sessionstore.resume_from_crash': false,
       'browser.shell.checkDefaultBrowser': false,
@@ -234,9 +235,22 @@ class FirefoxBrowser {
     }));
 
     try {
-      await LocalBrowser.spawn(this.getCandidates(), args, signal, logger);
+      await LocalBrowser.spawn(FirefoxBrowser.#getCandidates(), args, signal, logger);
     } finally {
-      fs.rmSync(profileDir, { recursive: true, force: true });
+      // On Windows, when spawn() returns after firefox.exe has stopped, it sometimes can't delete
+      // some temporary files yet as they're somehow still in use (EBUSY). Perhaps a race condition,
+      // or an lagged background process?
+      // > BUSY: resource busy or locked,
+      // > unlink 'C:\Users\RUNNER~1\AppData\Local\Temp\qtap_EZ4IoO\bounce-tracking-protection.sqlite'
+      // Enable `maxRetries` to cover the common case, and beyond that try-catch
+      // as it is not critical for test completion.
+      // TODO: Can we abstract this so that it happens automatically for any directory
+      // obtained via LocalBrowser.makeTempDir?
+      try {
+        fs.rmSync(profileDir, { recursive: true, force: true, maxRetries: 2 });
+      } catch (e) {
+        logger.warning('firefox_cleanup_fail', e);
+      }
     }
   }
 }

src/qtap.js

@@ -87,6 +87,12 @@ async function run (browserNames, files, options) {
     // exits naturally by itself.
     // TODO: Consider just calling process.exit after this await.
     // Is that faster and safe? What if any important clean up would we miss?
+    // 1. Removing of temp directories is generally done in browser "launch" functions
+    //    after the child process has properly ended (and has to, as otherwise the
+    //    files are likely still locked and/or may end up re-created). If we were to
+    //    exit earlier, we may leave temp directories behind. This is fine when running
+    //    in an ephemeral environment (e.g. CI), but not great for local dev.
+    //
     await Promise.allSettled(browserLaunches);
     // Await again, so that any error gets thrown accordingly,
     // we don't do this directly because we first want to wait for all tests