From ac5667166e5a74e07918872115259ac0f418280a Mon Sep 17 00:00:00 2001
From: Tim van der Meij <timvandermeij@gmail.com>
Date: Sun, 3 Dec 2023 16:31:45 +0100
Subject: [PATCH] Modernize the `verifyManifestFiles` test helper function

The test helper code largely predates the introduction of modern
JavaScript features and should be refactored to improve readability.
In particular callbacks and recursive function calls make the code
harder to understand and maintain.

This commit:

- replaces the callback argument with returning a promise;
- replaces the recursive function calls with a simple loop;
- uses `const`/`let` instead of `var`;
- uses template strings for shorter string formatting code;
- improves the error messages to have more details.
---
 test/downloadutils.mjs | 36 ++++++++++++++----------------------
 test/test.mjs          | 32 ++++++++++++++++----------------
 2 files changed, 30 insertions(+), 38 deletions(-)

diff --git a/test/downloadutils.mjs b/test/downloadutils.mjs
index 988ee5505..ab002c5d3 100644
--- a/test/downloadutils.mjs
+++ b/test/downloadutils.mjs
@@ -120,30 +120,24 @@ function calculateMD5(file) {
   });
 }
 
-function verifyManifestFiles(manifest, callback) {
-  async function verifyNext() {
-    if (i >= manifest.length) {
-      callback(error);
-      return;
-    }
-    var item = manifest[i];
-    if (fs.existsSync(item.file + ".error")) {
+async function verifyManifestFiles(manifest) {
+  let error = false;
+
+  for (const item of manifest) {
+    if (fs.existsSync(`${item.file}.error`)) {
       console.error(
-        'WARNING: File was not downloaded. See "' + item.file + '.error" file.'
+        `WARNING: "${item.file}" was not downloaded; see "${item.file}.error" file.`
       );
       error = true;
-      i++;
-      verifyNext();
-      return;
+      continue;
     }
-    if (item.link && !fs.existsSync(item.file + ".link")) {
+
+    if (item.link && !fs.existsSync(`${item.file}.link`)) {
       console.error(
         `WARNING: Unneeded \`"link": true\`-entry for the "${item.id}" test.`
       );
       error = true;
-      i++;
-      verifyNext();
-      return;
+      continue;
     }
 
     try {
@@ -165,13 +159,11 @@ function verifyManifestFiles(manifest, callback) {
       );
       error = true;
     }
-
-    i++;
-    verifyNext();
   }
-  var i = 0;
-  var error = false;
-  verifyNext();
+
+  if (error) {
+    throw new Error("Manifest validation failed");
+  }
 }
 
 export { downloadManifestFiles, verifyManifestFiles };
diff --git a/test/test.mjs b/test/test.mjs
index c24d6e9c3..7095f9a74 100644
--- a/test/test.mjs
+++ b/test/test.mjs
@@ -1071,23 +1071,23 @@ async function closeSession(browser) {
 
 function ensurePDFsDownloaded(callback) {
   var manifest = getTestManifest();
-  downloadManifestFiles(manifest, function () {
-    verifyManifestFiles(manifest, function (hasErrors) {
-      if (hasErrors) {
-        console.log(
-          "Unable to verify the checksum for the files that are " +
-            "used for testing."
-        );
-        console.log(
-          "Please re-download the files, or adjust the MD5 " +
-            "checksum in the manifest for the files listed above.\n"
-        );
-        if (options.strictVerify) {
-          process.exit(1);
-        }
+  downloadManifestFiles(manifest, async function () {
+    try {
+      await verifyManifestFiles(manifest);
+    } catch {
+      console.log(
+        "Unable to verify the checksum for the files that are " +
+          "used for testing."
+      );
+      console.log(
+        "Please re-download the files, or adjust the MD5 " +
+          "checksum in the manifest for the files listed above.\n"
+      );
+      if (options.strictVerify) {
+        process.exit(1);
       }
-      callback();
-    });
+    }
+    callback();
   });
 }