From 24fcc042f4b23153e8dce3a2315a7828800ee92b Mon Sep 17 00:00:00 2001
From: Tim van der Meij <timvandermeij@gmail.com>
Date: Sat, 4 Nov 2023 18:30:41 +0100
Subject: [PATCH] Refactor URL handling for the `startBrowsers` function in
 `test.mjs`

The current logic is more complicated than it needs to be because it's
passing a callback function to `startBrowsers` instead of a string.
This commit simplifies the logic by passing the base URL as a string to
`startBrowsers` and having it do further augmentation internally,
thereby removing all indirection of the function calls to `makeTestUrl`
and the inner function it returned.
---
 test/test.mjs | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/test/test.mjs b/test/test.mjs
index 704e5f3d4..b4edacf3b 100644
--- a/test/test.mjs
+++ b/test/test.mjs
@@ -339,7 +339,7 @@ function startRefTest(masterMode, showRefImages) {
     server.hooks.POST.push(refTestPostHandler);
     onAllSessionsClosed = finalize;
 
-    const startUrl = `http://${host}:${server.port}/test/test_slave.html`;
+    const baseUrl = `http://${host}:${server.port}/test/test_slave.html`;
     await startBrowsers(function (session) {
       session.masterMode = masterMode;
       session.taskResults = {};
@@ -358,7 +358,7 @@ function startRefTest(masterMode, showRefImages) {
       session.numEqNoSnapshot = 0;
       session.numEqFailures = 0;
       monitorBrowserTimeout(session, handleSessionTimeout);
-    }, makeTestUrl(startUrl));
+    }, baseUrl);
   }
   function checkRefsTmp() {
     if (masterMode && fs.existsSync(refsTmpDir)) {
@@ -793,29 +793,16 @@ function onAllSessionsClosedAfterTests(name) {
   };
 }
 
-function makeTestUrl(startUrl) {
-  return function (browserName) {
-    const queryParameters =
-      `?browser=${encodeURIComponent(browserName)}` +
-      `&manifestFile=${encodeURIComponent("/test/" + options.manifestFile)}` +
-      `&testFilter=${JSON.stringify(options.testfilter)}` +
-      `&xfaOnly=${options.xfaOnly}` +
-      `&delay=${options.statsDelay}` +
-      `&masterMode=${options.masterMode}`;
-    return startUrl + queryParameters;
-  };
-}
-
 async function startUnitTest(testUrl, name) {
   onAllSessionsClosed = onAllSessionsClosedAfterTests(name);
   startServer();
   server.hooks.POST.push(unitTestPostHandler);
 
-  const startUrl = `http://${host}:${server.port}${testUrl}`;
+  const baseUrl = `http://${host}:${server.port}${testUrl}`;
   await startBrowsers(function (session) {
     session.numRuns = 0;
     session.numErrors = 0;
-  }, makeTestUrl(startUrl));
+  }, baseUrl);
 }
 
 async function startIntegrationTest() {
@@ -971,7 +958,7 @@ async function startBrowser(browserName, startUrl = "") {
   return browser;
 }
 
-async function startBrowsers(initSessionCallback, makeStartUrl = null) {
+async function startBrowsers(initSessionCallback, baseUrl = null) {
   // Remove old browser revisions from Puppeteer's cache. Updating Puppeteer can
   // cause new browser revisions to be downloaded, so trimming the cache will
   // prevent the disk from filling up over time.
@@ -995,7 +982,20 @@ async function startBrowsers(initSessionCallback, makeStartUrl = null) {
       closed: false,
     };
     sessions.push(session);
-    const startUrl = makeStartUrl ? makeStartUrl(browserName) : "";
+
+    // Construct the start URL from the base URL by appending query parameters
+    // for the runner if necessary.
+    let startUrl = "";
+    if (baseUrl) {
+      const queryParameters =
+        `?browser=${encodeURIComponent(browserName)}` +
+        `&manifestFile=${encodeURIComponent("/test/" + options.manifestFile)}` +
+        `&testFilter=${JSON.stringify(options.testfilter)}` +
+        `&xfaOnly=${options.xfaOnly}` +
+        `&delay=${options.statsDelay}` +
+        `&masterMode=${options.masterMode}`;
+      startUrl = baseUrl + queryParameters;
+    }
 
     await startBrowser(browserName, startUrl)
       .then(function (browser) {