refactor: pave the way to add recording to measure command (#281)

* chore: fix husky install deprecation * fix(tools): fix videoFixMetadata command * refactor: start to move recorder instantiation in measurer * refactor: move more functions into measurer * refactor: fix some typings * refactor: move all recording logic to measurer
bamlab · May 20, 2024 · 372a311 · 372a311
1 parent ac00255
commit 372a311
Show file tree

Hide file tree

Showing 7 changed files with 77 additions and 49 deletions.
diff --git a/package.json b/package.json
@@ -28,7 +28,7 @@
     "build": "rm -rf .parcel-cache && yarn clean-dist && tsc --build && yarn workspace @perf-profiler/web-reporter build && yarn workspace @perf-profiler/measure build",
     "release": "yarn build && lerna publish",
     "test:e2e": "mkdir -p report && npx @perf-profiler/aws-device-farm runTest --apkPath .github/workflows/example.apk --projectName 'Flashlight-Serverless' --reportDestinationPath report --testCommand 'npx ts-node examples/e2e/appium-ci.test.ts' --testFolder .",
-    "prepare": "husky install"
+    "prepare": "husky"
   },
   "homepage": "https://github.com/bamlab/flashlight#readme",
   "devDependencies": {

diff --git a/packages/commands/measure/src/server/ServerSocketConnectionApp.tsx b/packages/commands/measure/src/server/ServerSocketConnectionApp.tsx
@@ -43,7 +43,11 @@ export const ServerSocketConnectionApp = ({ socket, url }: { socket: SocketType;
         return;
       }
 
-      performanceMeasureRef.current = new PerformanceMeasurer(state.bundleId);
+      performanceMeasureRef.current = new PerformanceMeasurer(state.bundleId, {
+        recordOptions: {
+          record: false,
+        },
+      });
 
       addNewResult(state.bundleId);
       performanceMeasureRef.current?.start(() =>

diff --git a/packages/commands/test/src/PerformanceMeasurer.ts b/packages/commands/test/src/PerformanceMeasurer.ts
@@ -1,24 +1,40 @@
 import { Logger } from "@perf-profiler/logger";
 import { profiler, waitFor } from "@perf-profiler/profiler";
+import { basename, dirname } from "path";
 import { Trace } from "./Trace";
-import { Measure, POLLING_INTERVAL } from "@perf-profiler/types";
+import { Measure, POLLING_INTERVAL, TestCaseIterationResult } from "@perf-profiler/types";
 
 export class PerformanceMeasurer {
   measures: Measure[] = [];
   polling?: { stop: () => void };
-  bundleId: string;
   shouldStop = false;
   timingTrace?: Trace;
 
-  constructor(bundleId: string) {
-    this.bundleId = bundleId;
-  }
+  constructor(
+    private bundleId: string,
+    private options: {
+      recordOptions:
+        | { record: false }
+        | {
+            record: true;
+            size?: string;
+            bitRate?: number;
+            videoPath: string;
+          };
+    }
+  ) {}
+
+  private recorder = this.options.recordOptions.record
+    ? profiler.getScreenRecorder(basename(this.options.recordOptions.videoPath))
+    : null;
 
   async start(
     onMeasure: (measure: Measure) => void = () => {
       // noop by default
     }
   ) {
+    await this.maybeStartRecording();
+
     this.polling = profiler.pollPerformanceMeasures(this.bundleId, {
       onMeasure: (measure) => {
         if (this.shouldStop) {
@@ -40,7 +56,7 @@ export class PerformanceMeasurer {
     this.polling?.stop();
   }
 
-  async stop(duration?: number) {
+  async stop(duration?: number): Promise<TestCaseIterationResult> {
     const time = this.timingTrace?.stop();
 
     if (duration) {
@@ -61,10 +77,36 @@ export class PerformanceMeasurer {
     // Ensure polling has stopped
     this.polling?.stop();
 
+    await this.maybeStopRecording();
+
+    const startTime = this.timingTrace?.startTime ?? 0;
+
     return {
       time: time ?? 0,
-      startTime: this.timingTrace?.startTime ?? 0,
+      startTime,
       measures: this.measures,
+      status: "SUCCESS",
+      videoInfos:
+        this.options.recordOptions.record && this.recorder
+          ? {
+              path: this.options.recordOptions.videoPath,
+              startOffset: Math.floor(startTime - this.recorder.getRecordingStartTime()),
+            }
+          : undefined,
     };
   }
+
+  private async maybeStartRecording() {
+    if (this.options.recordOptions.record && this.recorder) {
+      const { bitRate, size } = this.options.recordOptions;
+      await this.recorder.startRecording({ bitRate, size });
+    }
+  }
+
+  private async maybeStopRecording() {
+    if (this.options.recordOptions.record && this.recorder) {
+      await this.recorder.stopRecording();
+      await this.recorder.pullRecording(dirname(this.options.recordOptions.videoPath));
+    }
+  }
 }
diff --git a/packages/commands/test/src/SingleIterationTester.ts b/packages/commands/test/src/SingleIterationTester.ts
@@ -1,12 +1,9 @@
 import {
   AveragedTestCaseResult,
-  Measure,
   TestCaseIterationResult,
   TestCaseIterationStatus,
 } from "@perf-profiler/types";
 import { PerformanceMeasurer } from "./PerformanceMeasurer";
-import { profiler } from "@perf-profiler/profiler";
-import { basename, dirname } from "path";
 
 export interface TestCase {
   beforeTest?: () => Promise<void> | void;
@@ -39,11 +36,15 @@ export class SingleIterationTester {
   ) {}
 
   private currentTestCaseIterationResult: TestCaseIterationResult | undefined = undefined;
-  private performanceMeasurer: PerformanceMeasurer = new PerformanceMeasurer(this.bundleId);
   private videoPath = `${this.options.resultsFileOptions.path.replace(".json", "")}_iteration_${
     this.iterationIndex
   }_${new Date().getTime()}.mp4`;
-  private recorder = profiler.getScreenRecorder(basename(this.videoPath));
+  private performanceMeasurer: PerformanceMeasurer = new PerformanceMeasurer(this.bundleId, {
+    recordOptions: {
+      ...this.options.recordOptions,
+      videoPath: this.videoPath,
+    },
+  });
 
   public getCurrentTestCaseIterationResult() {
     return this.currentTestCaseIterationResult;
@@ -55,62 +56,34 @@ export class SingleIterationTester {
     try {
       if (beforeTest) await beforeTest();
 
-      await this.maybeStartRecording();
-      this.performanceMeasurer.start();
+      await this.performanceMeasurer.start();
       await run();
       const measures = await this.performanceMeasurer.stop(duration);
-      await this.maybeStopRecording();
 
       if (afterTest) await afterTest();
 
       this.setCurrentTestCaseIterationResult(measures, "SUCCESS");
     } catch (error) {
       const measures = await this.performanceMeasurer.stop();
-      await this.maybeStopRecording();
       this.setCurrentTestCaseIterationResult(measures, "FAILURE");
       this.performanceMeasurer.forceStop();
       throw error;
     }
   }
 
-  private async maybeStartRecording() {
-    if (this.options.recordOptions.record && this.recorder) {
-      const { bitRate, size } = this.options.recordOptions;
-      await this.recorder.startRecording({ bitRate, size });
-    }
-  }
-
   public setIsRetry(isRetry: boolean) {
     if (this.currentTestCaseIterationResult) {
       this.currentTestCaseIterationResult.isRetriedIteration = isRetry;
     }
   }
 
-  private async maybeStopRecording() {
-    if (this.options.recordOptions.record && this.recorder) {
-      await this.recorder.stopRecording();
-      await this.recorder.pullRecording(dirname(this.options.resultsFileOptions.path));
-    }
-  }
-
   private setCurrentTestCaseIterationResult(
-    measures: {
-      time: number;
-      startTime: number;
-      measures: Measure[];
-    },
+    measures: TestCaseIterationResult,
     status: TestCaseIterationStatus
   ) {
     this.currentTestCaseIterationResult = {
       ...measures,
       status,
-      videoInfos:
-        this.options.recordOptions.record && this.recorder
-          ? {
-              path: this.videoPath,
-              startOffset: Math.floor(measures.startTime - this.recorder.getRecordingStartTime()),
-            }
-          : undefined,
     };
   }
 }
diff --git a/packages/commands/test/src/__tests__/PerformanceMeasurer.test.ts b/packages/commands/test/src/__tests__/PerformanceMeasurer.test.ts
@@ -9,9 +9,13 @@ const loggerDebug = jest.spyOn(Logger, "debug");
 const loggerError = jest.spyOn(Logger, "error");
 
 describe("PerformanceMeasurer", () => {
-  it("handles c++ errors correctly", () => {
-    const measurer = new PerformanceMeasurer("com.example");
-    measurer.start();
+  it("handles c++ errors correctly", async () => {
+    const measurer = new PerformanceMeasurer("com.example", {
+      recordOptions: {
+        record: false,
+      },
+    });
+    await measurer.start();
     emitMeasure(0);
     emitMeasure(1);
     perfProfilerMock.stderr?.emit("data", "CPP_ERROR_CANNOT_OPEN_FILE /proc/1234/tasks/578/stat");

diff --git a/packages/commands/tools/src/bin.ts b/packages/commands/tools/src/bin.ts
@@ -3,6 +3,7 @@
 import { program } from "commander";
 import { processVideoFile } from "@perf-profiler/shell";
 import { profiler } from "@perf-profiler/profiler";
+import fs from "fs";
 
 const toolsCommand = program.command("tools").description("Utility tools related to Flashlight");
 
@@ -18,6 +19,8 @@ toolsCommand
   .description(
     "When coming from AWS Device Farm or certain devices, it seems the video from flashlight test is not encoded properly"
   )
-  .action((options) => {
-    processVideoFile(options.video_file_path, "destinationPath");
+  .action((videoFilePath) => {
+    const backupFilePath = `${videoFilePath}.bak`;
+    fs.cpSync(videoFilePath, backupFilePath);
+    processVideoFile(backupFilePath, videoFilePath);
   });
diff --git a/packages/core/types/index.ts b/packages/core/types/index.ts
@@ -19,6 +19,8 @@ export type TestCaseIterationStatus = "SUCCESS" | "FAILURE";
 
 export interface TestCaseIterationResult {
   time: number;
+  // we probably don't need this but this is added by the PerformanceMeasurer
+  startTime?: number;
   measures: Measure[];
   status: TestCaseIterationStatus;
   videoInfos?: {