From 63940093bee5913d047529ee178931a29a6f06dd Mon Sep 17 00:00:00 2001
From: Dario Piotrowicz <dario@cloudflare.com>
Date: Tue, 5 Nov 2024 11:54:27 +0000
Subject: [PATCH] apply minor PR suggestions

---
 benchmarking/src/benchmarking.ts | 40 ++++++++++++++++----------------
 benchmarking/src/cloudflare.ts   |  8 ++++---
 2 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/benchmarking/src/benchmarking.ts b/benchmarking/src/benchmarking.ts
index 5bf01f8..1da6a75 100644
--- a/benchmarking/src/benchmarking.ts
+++ b/benchmarking/src/benchmarking.ts
@@ -4,7 +4,7 @@ import nodePath from "node:path";
 import { getPercentile } from "./utils";
 
 export type FetchBenchmark = {
-  callDurationsMs: number[];
+  iterationsMs: number[];
   averageMs: number;
   p90Ms: number;
 };
@@ -15,6 +15,17 @@ export type BenchmarkingResults = {
   fetchBenchmark: FetchBenchmark;
 }[];
 
+type BenchmarkFetchOptions = {
+  numberOfIterations?: number;
+  maxRandomDelayMs?: number;
+  fetch: (deploymentUrl: string) => Promise<Response>;
+};
+
+const defaultOptions: Required<Omit<BenchmarkFetchOptions, "fetch">> = {
+  numberOfIterations: 20,
+  maxRandomDelayMs: 15_000,
+};
+
 /**
  * Benchmarks the response time of an application end-to-end by:
  *  - building the application
@@ -40,17 +51,6 @@ export async function benchmarkApplicationResponseTime({
   return benchmarkFetch(deploymentUrl, { fetch });
 }
 
-type BenchmarkFetchOptions = {
-  numberOfCalls?: number;
-  maxRandomDelayMs?: number;
-  fetch: (deploymentUrl: string) => Promise<Response>;
-};
-
-const defaultOptions: Required<Omit<BenchmarkFetchOptions, "fetch">> = {
-  numberOfCalls: 20,
-  maxRandomDelayMs: 15_000,
-};
-
 /**
  * Benchmarks a fetch operation by running it multiple times and computing the average time (in milliseconds) such fetch operation takes.
  *
@@ -71,23 +71,23 @@ async function benchmarkFetch(url: string, options: BenchmarkFetchOptions): Prom
     return postTimeMs - preTimeMs;
   };
 
-  const callDurationsMs = await Promise.all(
-    new Array(options?.numberOfCalls ?? defaultOptions.numberOfCalls).fill(null).map(async () => {
+  const resolvedOptions = { ...defaultOptions, ...options };
+
+  const iterationsMs = await Promise.all(
+    new Array(resolvedOptions.numberOfIterations).fill(null).map(async () => {
       // let's add a random delay before we make the fetch
-      await nodeTimesPromises.setTimeout(
-        Math.round(Math.random() * (options?.maxRandomDelayMs ?? defaultOptions.maxRandomDelayMs))
-      );
+      await nodeTimesPromises.setTimeout(Math.round(Math.random() * resolvedOptions.maxRandomDelayMs));
 
       return benchmarkFetchCall();
     })
   );
 
-  const averageMs = callDurationsMs.reduce((time, sum) => sum + time) / callDurationsMs.length;
+  const averageMs = iterationsMs.reduce((time, sum) => sum + time) / iterationsMs.length;
 
-  const p90Ms = getPercentile(callDurationsMs, 90);
+  const p90Ms = getPercentile(iterationsMs, 90);
 
   return {
-    callDurationsMs,
+    iterationsMs,
     averageMs,
     p90Ms,
   };
diff --git a/benchmarking/src/cloudflare.ts b/benchmarking/src/cloudflare.ts
index ba63bc2..4c482cd 100644
--- a/benchmarking/src/cloudflare.ts
+++ b/benchmarking/src/cloudflare.ts
@@ -53,11 +53,13 @@ export async function buildApp(dir: string): Promise<void> {
 
   const packageJsonContent = JSON.parse(await nodeFsPromises.readFile(packageJsonPath, "utf8"));
 
-  if (!("scripts" in packageJsonContent) || !("build:worker" in packageJsonContent.scripts)) {
-    throw new Error(`Error: package.json for app at "${dir}" does not include a "build:worker" script`);
+  const buildScript = "build:worker";
+
+  if (!packageJsonContent.scripts?.[buildScript]) {
+    throw new Error(`Error: package.json for app at "${dir}" does not include a "${buildScript}" script`);
   }
 
-  const command = "pnpm build:worker";
+  const command = `pnpm ${buildScript}`;
 
   await promiseExec(command, { cwd: dir });
 }