From 712c0a6f144fdb4e009ac567bf7c1a192f6024db Mon Sep 17 00:00:00 2001
From: Harsha Nalluru <sanallur@microsoft.com>
Date: Thu, 15 Jul 2021 00:55:38 -0700
Subject: [PATCH] [Perf framework] Remove run method - cleaning up (#16391)

---
 sdk/test-utils/perfstress/CHANGELOG.md        |  6 +++
 sdk/test-utils/perfstress/src/options.ts      |  5 --
 sdk/test-utils/perfstress/src/program.ts      | 52 ++-----------------
 sdk/test-utils/perfstress/src/tests.ts        |  1 -
 .../perfstress/test/exception.spec.ts         |  8 ---
 sdk/test-utils/perfstress/test/noop.spec.ts   |  2 -
 .../perfstress/test/options.spec.ts           |  2 +-
 .../perfstress/test/setupCleanup.spec.ts      |  2 +-
 8 files changed, 12 insertions(+), 66 deletions(-)

diff --git a/sdk/test-utils/perfstress/CHANGELOG.md b/sdk/test-utils/perfstress/CHANGELOG.md
index 9f379765cbbb..58b60062f7d5 100644
--- a/sdk/test-utils/perfstress/CHANGELOG.md
+++ b/sdk/test-utils/perfstress/CHANGELOG.md
@@ -2,4 +2,10 @@
 
 ## 1.0.0 (Unreleased)
 
+### 2021-07-14
+
+- Removed the run method in the `PerfStressTest` class as we only deal with the async methods when it comes to performance.
+
+### 2020-04-22
+
 - Merged the first working implementation of perfstress.
diff --git a/sdk/test-utils/perfstress/src/options.ts b/sdk/test-utils/perfstress/src/options.ts
index 9e6bd5c4df47..ad3e91c6cc6a 100644
--- a/sdk/test-utils/perfstress/src/options.ts
+++ b/sdk/test-utils/perfstress/src/options.ts
@@ -65,7 +65,6 @@ export interface DefaultPerfStressOptions {
   iterations: number;
   "no-cleanup": boolean;
   "milliseconds-to-log": number;
-  sync: boolean;
 }
 
 /**
@@ -96,10 +95,6 @@ export const defaultPerfStressOptions: PerfStressOptionDictionary<DefaultPerfStr
     shortName: "i",
     defaultValue: 1
   },
-  sync: {
-    description: "Only runs the 'run' method instead of the 'runAsync' method",
-    defaultValue: false
-  },
   "no-cleanup": {
     description: "Disables test cleanup"
   },
diff --git a/sdk/test-utils/perfstress/src/program.ts b/sdk/test-utils/perfstress/src/program.ts
index 943a947e4f4f..a97ac93b7ca3 100644
--- a/sdk/test-utils/perfstress/src/program.ts
+++ b/sdk/test-utils/perfstress/src/program.ts
@@ -111,47 +111,6 @@ export class PerfStressProgram {
     );
   }
 
-  /**
-   * Runs the test in scope repeatedly, without waiting for any promises to finish,
-   * as many times as possible until durationMilliseconds is reached.
-   * For each test run, it will report one more completedOperations on the PerfStressParallel given,
-   * as well as the lastMillisecondsElapsed that reports the last test execution's elapsed time in comparison
-   * to the beginning of the execution of runLoop.
-   *
-   * @param parallel Object where to log the results from each execution.
-   * @param durationMilliseconds When to abort any execution.
-   * @param abortController Allows us to send through a signal determining when to abort any execution.
-   */
-  private runLoopSync(
-    test: PerfStressTest,
-    parallel: PerfStressParallel,
-    durationMilliseconds: number,
-    abortController: AbortController
-  ): void {
-    if (!test.run) {
-      throw new Error(`The "run" method is missing in the test ${this.testName}`);
-    }
-    const start = process.hrtime();
-    while (!abortController.signal.aborted) {
-      test.run(abortController.signal);
-
-      const elapsed = process.hrtime(start);
-      const elapsedMilliseconds = elapsed[0] * 1000 + elapsed[1] / 1000000;
-
-      parallel.completedOperations += 1;
-      parallel.lastMillisecondsElapsed = elapsedMilliseconds;
-
-      // In runTest we create a setTimeout that is intended to abort the abortSignal
-      // once the durationMilliseconds have elapsed. That setTimeout might not get queued
-      // on time through the event loop, depending on the number of operations we might be executing.
-      // For this reason, we're also manually checking the elapsed time here.
-      if (abortController.signal.aborted || elapsedMilliseconds > durationMilliseconds) {
-        abortController.abort();
-        break;
-      }
-    }
-  }
-
   /**
    * Runs the test in scope repeatedly, without waiting for any promises to finish,
    * as many times as possible until durationMilliseconds is reached.
@@ -201,7 +160,7 @@ export class PerfStressProgram {
     title: string
   ): Promise<void> {
     const parallels: PerfStressParallel[] = new Array<PerfStressParallel>(this.parallelNumber);
-    const parallelTestResults: Promise<void>[] | void[] = new Array<void>(this.parallelNumber);
+    const parallelTestResults: Array<Promise<void>> = new Array<Promise<void>>(this.parallelNumber);
 
     const abortController = new AbortController();
     const durationMilliseconds = durationSeconds * 1000;
@@ -231,8 +190,7 @@ export class PerfStressProgram {
       console.log(`${currentCompleted}\t\t${totalCompleted}\t\t${averageCompleted.toFixed(2)}`);
     }, millisecondsToLog);
 
-    const isAsync = !this.parsedDefaultOptions.sync.value;
-    const runLoop = isAsync ? this.runLoopAsync : this.runLoopSync;
+    const runLoop = this.runLoopAsync;
 
     // Unhandled exceptions should stop the whole PerfStress process.
     process.on("unhandledRejection", (error) => {
@@ -260,10 +218,8 @@ export class PerfStressProgram {
       );
     }
 
-    if (isAsync) {
-      for (const promise of parallelTestResults) {
-        await promise;
-      }
+    for (const promise of parallelTestResults) {
+      await promise;
     }
 
     // Once we finish, we clear the log interval.
diff --git a/sdk/test-utils/perfstress/src/tests.ts b/sdk/test-utils/perfstress/src/tests.ts
index 12a0b10f3ad6..994a36f677be 100644
--- a/sdk/test-utils/perfstress/src/tests.ts
+++ b/sdk/test-utils/perfstress/src/tests.ts
@@ -47,7 +47,6 @@ export abstract class PerfStressTest<TOptions = {}> {
   public setup?(): void | Promise<void>;
   public cleanup?(): void | Promise<void>;
 
-  public run?(abortSignal?: AbortSignalLike): void;
   public async runAsync?(abortSignal?: AbortSignalLike): Promise<void>;
 }
 
diff --git a/sdk/test-utils/perfstress/test/exception.spec.ts b/sdk/test-utils/perfstress/test/exception.spec.ts
index 5221a7ec725f..38f9351dade9 100644
--- a/sdk/test-utils/perfstress/test/exception.spec.ts
+++ b/sdk/test-utils/perfstress/test/exception.spec.ts
@@ -11,14 +11,6 @@ import { PerfStressTest } from "../src";
 export class Exception extends PerfStressTest {
   public options = {};
 
-  run(): void {
-    try {
-      throw new Error();
-    } catch (e) {
-      // Nothing to do here
-    }
-  }
-
   async runAsync(): Promise<void> {
     try {
       throw new Error();
diff --git a/sdk/test-utils/perfstress/test/noop.spec.ts b/sdk/test-utils/perfstress/test/noop.spec.ts
index f2a2d137ed5f..a3f525094c79 100644
--- a/sdk/test-utils/perfstress/test/noop.spec.ts
+++ b/sdk/test-utils/perfstress/test/noop.spec.ts
@@ -9,7 +9,5 @@ import { PerfStressTest } from "../src";
 export class NoOp extends PerfStressTest {
   public options = {};
 
-  run(): void {}
-
   async runAsync(): Promise<void> {}
 }
diff --git a/sdk/test-utils/perfstress/test/options.spec.ts b/sdk/test-utils/perfstress/test/options.spec.ts
index 2846e1161d86..3c8c913d414e 100644
--- a/sdk/test-utils/perfstress/test/options.spec.ts
+++ b/sdk/test-utils/perfstress/test/options.spec.ts
@@ -71,7 +71,7 @@ export class OptionsTest extends PerfStressTest<OptionsTestOptions> {
     }
   }
 
-  run(): void {
+  async runAsync() {
     for (const key in this.options) {
       this.compare(key as keyof OptionsTestOptions);
     }
diff --git a/sdk/test-utils/perfstress/test/setupCleanup.spec.ts b/sdk/test-utils/perfstress/test/setupCleanup.spec.ts
index af8dc6cc02b9..c404cd3082a6 100644
--- a/sdk/test-utils/perfstress/test/setupCleanup.spec.ts
+++ b/sdk/test-utils/perfstress/test/setupCleanup.spec.ts
@@ -47,5 +47,5 @@ export class SetupCleanupTest extends PerfStressTest {
     }
   }
 
-  run(): void {}
+  async runAsync() {}
 }