From cc90fca2f6031854f35c8ac6cf2e4ce83db39a53 Mon Sep 17 00:00:00 2001
From: Mark Stacey <markjstacey@gmail.com>
Date: Mon, 21 Jun 2021 12:46:18 -0230
Subject: [PATCH] Add retries to the benchmark script (#11319)

The benchmark script can now be set to retry upon failure, like the E2E
tests do. The default is zero, just as with the E2E tests. A retry of 2
has been set in CI to match the E2E tests as well.

The `retry` module had to be adjusted to throw an error in the case of
failure. Previously it just set the exit code, but that only worked
because it was the last thing called before the process ended. That is
no longer the case.
---
 .circleci/config.yml     |  2 +-
 development/lib/retry.js |  4 +---
 test/e2e/benchmark.js    | 25 ++++++++++++++++++-------
 3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/.circleci/config.yml b/.circleci/config.yml
index 27c4c8144..cc41155be 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -448,7 +448,7 @@ jobs:
           command: mv ./builds-test ./builds
       - run:
           name: Run page load benchmark
-          command: yarn benchmark:chrome --out test-artifacts/chrome/benchmark/pageload.json
+          command: yarn benchmark:chrome --out test-artifacts/chrome/benchmark/pageload.json --retries 2
       - store_artifacts:
           path: test-artifacts
           destination: test-artifacts
diff --git a/development/lib/retry.js b/development/lib/retry.js
index 20783aa32..356f1a08f 100644
--- a/development/lib/retry.js
+++ b/development/lib/retry.js
@@ -1,5 +1,3 @@
-const { exitWithError } = require('./exit-with-error');
-
 /**
  * Run the given function, retrying it upon failure until reaching the
  * specified number of retries.
@@ -19,7 +17,7 @@ async function retry(retries, functionToRetry) {
       attempts += 1;
     }
   }
-  exitWithError('Retry limit reached');
+  throw new Error('Retry limit reached');
 }
 
 module.exports = { retry };
diff --git a/test/e2e/benchmark.js b/test/e2e/benchmark.js
index a9d38c6fc..fc8843c58 100644
--- a/test/e2e/benchmark.js
+++ b/test/e2e/benchmark.js
@@ -5,6 +5,8 @@ const { promises: fs, constants: fsConstants } = require('fs');
 const yargs = require('yargs/yargs');
 const { hideBin } = require('yargs/helpers');
 const ttest = require('ttest');
+const { retry } = require('../../development/lib/retry');
+const { exitWithError } = require('../../development/lib/exit-with-error');
 const { withFixtures } = require('./helpers');
 const { PAGES } = require('./webdriver/driver');
 
@@ -54,12 +56,16 @@ const marginOfErrorResult = calculateResult((array) =>
   array.length === 1 ? 0 : calculateMarginOfError(array),
 );
 
-async function profilePageLoad(pages, numSamples) {
+async function profilePageLoad(pages, numSamples, retries) {
   const results = {};
   for (const pageName of pages) {
     const runResults = [];
     for (let i = 0; i < numSamples; i += 1) {
-      runResults.push(await measurePage(pageName));
+      let result;
+      await retry(retries, async () => {
+        result = await measurePage(pageName);
+      });
+      runResults.push(result);
     }
 
     if (runResults.some((result) => result.navigation.lenth > 1)) {
@@ -153,10 +159,16 @@ async function main() {
             'Output filename. Output printed to STDOUT of this is omitted.',
           type: 'string',
           normalize: true,
+        })
+        .option('retries', {
+          default: 0,
+          description:
+            'Set how many times each benchmark sample should be retried upon failure.',
+          type: 'number',
         }),
   );
 
-  const { pages, samples, out } = argv;
+  const { pages, samples, out, retries } = argv;
 
   let outputDirectory;
   let existingParentDirectory;
@@ -170,7 +182,7 @@ async function main() {
     }
   }
 
-  const results = await profilePageLoad(pages, samples);
+  const results = await profilePageLoad(pages, samples, retries);
 
   if (out) {
     if (outputDirectory !== existingParentDirectory) {
@@ -182,7 +194,6 @@ async function main() {
   }
 }
 
-main().catch((e) => {
-  console.error(e);
-  process.exit(1);
+main().catch((error) => {
+  exitWithError(error);
 });