Fix AsyncBenchmarkStep never awaiting its step

resources/shared/step-runner.mjs

@@ -7,15 +7,13 @@ export class StepRunner {
     #params;
     #suite;
     #step;
-    #type;
 
-    constructor(frame, page, params, suite, step, type) {
+    constructor(frame, page, params, suite, step) {
         this.#suite = suite;
         this.#step = step;
         this.#params = params;
         this.#page = page;
         this.#frame = frame;
-        this.#type = type;
     }
 
     get page() {
@@ -26,6 +24,10 @@ export class StepRunner {
         return this.#step;
     }
 
+    get isAsync() {
+        return false;
+    }
+
     _runSyncStep(step, page) {
         step.run(page);
     }
@@ -54,7 +56,7 @@ export class StepRunner {
             performance.mark(syncStartLabel);
             const syncStartTime = performance.now();
 
-            if (this.#type === "async")
+            if (this.isAsync)
                 await this._runSyncStep(this.step, this.page);
             else
                 this._runSyncStep(this.step, this.page);
@@ -96,6 +98,10 @@ export class StepRunner {
 }
 
 export class AsyncStepRunner extends StepRunner {
+    get isAsync() {
+        return true;
+    }
+
     async _runSyncStep(step, page) {
         await step.run(page);
     }

resources/suite-runner.mjs

@@ -93,7 +93,7 @@ export class SuiteRunner {
 
             const stepRunnerType = this.#suite.type ?? this.params.useAsyncSteps ? "async" : "default";
             const stepRunnerClass = STEP_RUNNER_LOOKUP[stepRunnerType];
-            const stepRunner = new stepRunnerClass(this.#frame, this.#page, this.#params, this.#suite, step, stepRunnerType);
+            const stepRunner = new stepRunnerClass(this.#frame, this.#page, this.#params, this.#suite, step);
             let { syncTime, asyncTime } = await stepRunner.runStep();
             this._recordTestResults(step, syncTime, asyncTime);
         }

tests/index.html

@@ -27,6 +27,7 @@
                 },
             });
 
+            await import("./unittests/benchmark.mjs");
             await import("./unittests/benchmark-runner.mjs");
             await import("./unittests/params.mjs");
             await import("./unittests/suites.mjs");

tests/unittests/benchmark-runner.mjs

@@ -265,7 +265,7 @@ describe("BenchmarkRunner", () => {
 
         it("should run StepRunner and return { syncTime, asyncTime }", async () => {
             const step = new BenchmarkTestStep("SyncStep", sinon.stub());
-            const runner = new StepRunner(null, null, params, suite, step, "default");
+            const runner = new StepRunner(null, null, params, suite, step);
             const { syncTime, asyncTime } = await runner.runStep();
             expect(typeof syncTime).to.equal("number");
             expect(typeof asyncTime).to.equal("number");
@@ -277,7 +277,7 @@ describe("BenchmarkRunner", () => {
                 "AsyncStep",
                 sinon.stub().callsFake(async () => {})
             );
-            const runner = new AsyncStepRunner(null, null, params, suite, asyncStep, "async");
+            const runner = new AsyncStepRunner(null, null, params, suite, asyncStep);
             const { syncTime, asyncTime } = await runner.runStep();
             expect(typeof syncTime).to.equal("number");
             expect(typeof asyncTime).to.equal("number");

tests/unittests/benchmark.mjs

@@ -0,0 +1,54 @@
+import { AsyncBenchmarkStep, AsyncBenchmarkSuite, BenchmarkStep, BenchmarkSuite } from "../../resources/shared/benchmark.mjs";
+import { Params } from "../../resources/shared/params.mjs";
+import { skipInShell } from "../../resources/shared/helpers.mjs";
+
+const SLEEP_MS = 200;
+
+function sleep(ms) {
+    return new Promise((resolve) => setTimeout(resolve, ms));
+}
+
+describe("BenchmarkSuite", () => {
+    let params;
+
+    before(function () {
+        // The step schedulers need requestAnimationFrame and a document.
+        skipInShell(this);
+        params = new Params();
+    });
+
+    it("should measure the work of a sync step", async () => {
+        const suite = new BenchmarkSuite("SyncProbe", [
+            new BenchmarkStep("BusyStep", () => {
+                const start = performance.now();
+                while (performance.now() - start < SLEEP_MS)
+                    continue;
+            }),
+        ]);
+
+        const { result } = await suite.runAndRecordSuite(params);
+
+        expect(result.tests.BusyStep.tests.Sync).to.be.greaterThan(SLEEP_MS * 0.9);
+        expect(result.total).to.be.greaterThan(SLEEP_MS * 0.9);
+    });
+});
+
+describe("AsyncBenchmarkSuite", () => {
+    let params;
+
+    before(function () {
+        skipInShell(this);
+        params = new Params();
+    });
+
+    // AsyncBenchmarkStep used to construct its AsyncStepRunner without a type, leaving
+    // StepRunner on the non-awaiting branch, so this reported ~0ms.
+    it("should measure the work of a step that resolves asynchronously", async () => {
+        const suite = new AsyncBenchmarkSuite("AsyncProbe", [new AsyncBenchmarkStep("SleepingStep", () => sleep(SLEEP_MS))]);
+
+        const { result } = await suite.runAndRecordSuite(params);
+
+        expect(result.tests.SleepingStep.tests.Sync).to.be.greaterThan(SLEEP_MS * 0.9);
+        expect(result.total).to.be.greaterThan(SLEEP_MS * 0.9);
+    });
+});