From c9a923066ba85cacd58900a85712e720cd161790 Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Sun, 3 Dec 2023 16:22:47 +0100 Subject: [PATCH 1/2] Modernize the `calculateMD5` 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; - uses `const` instead of `var`; - uses arrow functions for shorter code; - uses template strings for shorter string formatting code; - improves the error messages to have more details. --- test/downloadutils.mjs | 57 +++++++++++++++++------------------------- 1 file changed, 23 insertions(+), 34 deletions(-) diff --git a/test/downloadutils.mjs b/test/downloadutils.mjs index 548e07ecbd5db..988ee55056e89 100644 --- a/test/downloadutils.mjs +++ b/test/downloadutils.mjs @@ -110,23 +110,18 @@ function downloadManifestFiles(manifest, callback) { downloadNext(); } -function calculateMD5(file, callback) { - var hash = crypto.createHash("md5"); - var stream = fs.createReadStream(file); - stream.on("data", function (data) { - hash.update(data); - }); - stream.on("error", function (err) { - callback(err); - }); - stream.on("end", function () { - var result = hash.digest("hex"); - callback(null, result); +function calculateMD5(file) { + return new Promise((resolve, reject) => { + const hash = crypto.createHash("md5"); + const stream = fs.createReadStream(file); + stream.on("data", data => hash.update(data)); + stream.on("error", error => reject(error)); + stream.on("end", () => resolve(hash.digest("hex"))); }); } function verifyManifestFiles(manifest, callback) { - function verifyNext() { + async function verifyNext() { if (i >= manifest.length) { callback(error); return; @@ -150,35 +145,29 @@ function verifyManifestFiles(manifest, callback) { verifyNext(); return; } - calculateMD5(item.file, function (err, md5) { - if (err) { - console.log('WARNING: Unable to open file for reading "' + err + '".'); - error = true; - } else if (!item.md5) { + + try { + const md5 = await calculateMD5(item.file); + if (!item.md5) { console.error( - 'WARNING: Missing md5 for file "' + - item.file + - '". ' + - 'Hash for current file is "' + - md5 + - '"' + `WARNING: MD5 hash missing for "${item.file}" (computed "${md5}").` ); error = true; } else if (md5 !== item.md5) { console.error( - 'WARNING: MD5 of file "' + - item.file + - '" does not match file. Expected "' + - item.md5 + - '" computed "' + - md5 + - '"' + `WARNING: MD5 hash mismatch for "${item.file}" (expected "${item.md5}", computed "${md5}").` ); error = true; } - i++; - verifyNext(); - }); + } catch (ex) { + console.log( + `WARNING: MD5 hash calculation failed for "${item.file}" ("${ex}").` + ); + error = true; + } + + i++; + verifyNext(); } var i = 0; var error = false; From ac5667166e5a74e07918872115259ac0f418280a Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Sun, 3 Dec 2023 16:31:45 +0100 Subject: [PATCH 2/2] 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 | 34 +++++++++++++--------------------- test/test.mjs | 32 ++++++++++++++++---------------- 2 files changed, 29 insertions(+), 37 deletions(-) diff --git a/test/downloadutils.mjs b/test/downloadutils.mjs index 988ee55056e89..ab002c5d3ef43 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(); + if (error) { + throw new Error("Manifest validation failed"); } - var i = 0; - var error = false; - verifyNext(); } export { downloadManifestFiles, verifyManifestFiles }; diff --git a/test/test.mjs b/test/test.mjs index c24d6e9c37bbc..7095f9a74ca9c 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(); }); }