Only save test data for tests run with Replay browser (#586)

Only upload test metadata for tests that were run with the Replay browser. If a given test run has no tests that match this criteria, then we should not even upload any metadata for it.
replayio · Jul 3, 2024 · 2d78191 · 2d78191
1 parent f0f488d
commit 2d78191
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 40 deletions.
diff --git a/.changeset/late-cars-doubt.md b/.changeset/late-cars-doubt.md
@@ -0,0 +1,5 @@
+---
+"@replayio/playwright": patch
+---
+
+Only save test data for tests run with Replay browser
diff --git a/packages/playwright/src/reporter.ts b/packages/playwright/src/reporter.ts
@@ -79,7 +79,8 @@ export default class ReplayPlaywrightReporter implements Reporter {
     string,
     { steps: FixtureStep[]; stacks: Record<string, StackFrame[]>; filenames: Set<string> }
   > = {};
-  private _projects: Record<string, { executed: boolean; usingReplay: boolean }> = {};
+
+  private _executedProjects: Record<string, { usesReplayBrowser: boolean }> = {};
 
   constructor(config: ReplayPlaywrightConfig) {
     setUserAgent(`${packageName}/${packageVersion}`);
@@ -152,7 +153,7 @@ export default class ReplayPlaywrightReporter implements Reporter {
     return this.fixtureData[id];
   }
 
-  // Playwright alrady provides a unique test id:
+  // Playwright already provides a unique test id:
   // https://github.com/microsoft/playwright/blob/6fb214de2378a9d874b46df6ea99d04da5765cba/packages/playwright/src/common/suiteUtils.ts#L56-L57
   // this is different because it includes `repeatEachIndex` and `attempt`
   // TODO(PRO-667): this could be simplified to `${test.testId}-${test.repeatEachIndex}-${test.attempt}`
@@ -175,25 +176,31 @@ export default class ReplayPlaywrightReporter implements Reporter {
     };
   }
 
-  onBegin({ version, projects }: FullConfig) {
-    const replayBrowserPath = getRuntimePath();
-    for (const project of projects) {
-      this._projects[project.name] = {
-        executed: false,
-        usingReplay: project.use.launchOptions?.executablePath === replayBrowserPath,
-      };
-    }
+  onBegin({ version }: FullConfig) {
     this.reporter.setTestRunnerVersion(version);
     this.reporter.onTestSuiteBegin();
   }
 
+  private _registerExecutedProject(test: TestCase) {
+    const project = test.parent.project();
+    if (project) {
+      let projectMetadata = this._executedProjects[project.name];
+      if (!projectMetadata) {
+        projectMetadata = this._executedProjects[project.name] = {
+          usesReplayBrowser: project.use.launchOptions?.executablePath === getRuntimePath(),
+        };
+      }
+      return projectMetadata;
+    }
+
+    return null;
+  }
+
   onTestBegin(test: TestCase, testResult: TestResult) {
-    const projectName = test.parent.project()?.name;
+    const projectMetadata = this._registerExecutedProject(test);
 
-    // it's important to handle the root project's name here and that's an empty string
-    if (typeof projectName === "string") {
-      this._projects[projectName].executed = true;
-    }
+    // Don't save metadata for non-Replay projects
+    if (!projectMetadata?.usesReplayBrowser) return;
 
     const testExecutionId = this._getTestExecutionId({
       filePath: test.location.file,
@@ -202,6 +209,7 @@ export default class ReplayPlaywrightReporter implements Reporter {
       attempt: testResult.retry + 1,
       source: this.getSource(test),
     });
+
     this.reporter.onTestBegin(testExecutionId, getMetadataFilePath(testResult.workerIndex));
   }
 
@@ -266,9 +274,15 @@ export default class ReplayPlaywrightReporter implements Reporter {
 
   onTestEnd(test: TestCase, result: TestResult) {
     const status = result.status;
-    // skipped tests won't have a reply so nothing to do here
+
+    // Skipped tests won't have a reply so nothing to do here
     if (status === "skipped") return;
 
+    const projectMetadata = this._registerExecutedProject(test);
+
+    // Don't save metadata for non-Replay projects
+    if (!projectMetadata?.usesReplayBrowser) return;
+
     const testExecutionIdData = {
       filePath: test.location.file,
       projectName: test.parent.project()?.name,
@@ -351,25 +365,23 @@ export default class ReplayPlaywrightReporter implements Reporter {
     try {
       await this.reporter.onEnd();
 
-      const output: string[] = [];
+      const didUseReplayBrowser = Object.values(this._executedProjects).some(
+        ({ usesReplayBrowser }) => usesReplayBrowser
+      );
+      const isReplayBrowserInstalled = existsSync(getRuntimePath());
 
-      const executedProjectWithReplay = !Object.keys(this._projects).some(projectName => {
-        const { executed, usingReplay } = this._projects[projectName];
-        return executed && usingReplay;
-      });
+      const output: string[] = [];
 
-      if (!executedProjectWithReplay) {
+      if (!didUseReplayBrowser) {
         mixpanelAPI.trackEvent("playwright.warning.reporter-used-without-replay-project");
         output.push(emphasize("None of the configured projects ran using Replay Chromium."));
       }
 
-      if (!existsSync(getRuntimePath())) {
-        if (executedProjectWithReplay) {
+      if (!isReplayBrowserInstalled) {
+        if (didUseReplayBrowser) {
           mixpanelAPI.trackEvent("playwright.warning.replay-browser-not-installed");
         }
-        if (output.length) {
-          output.push("");
-        }
+
         output.push(
           `To record tests with Replay, you need to install the Replay browser: ${highlight(
             "npx replayio install"
@@ -378,13 +390,16 @@ export default class ReplayPlaywrightReporter implements Reporter {
       }
 
       if (output.length) {
-        output.push("");
         output.push(
           `Learn more at ${link(
             "https://docs.replay.io/reference/test-runners/playwright/overview"
           )}`
         );
-        output.forEach(line => {
+
+        output.forEach((line, index) => {
+          if (index > 0) {
+            console.log("[replay.io]:");
+          }
           console.warn(`[replay.io]: ${line}`);
         });
       }

diff --git a/packages/test-utils/src/reporter.ts b/packages/test-utils/src/reporter.ts
@@ -443,12 +443,7 @@ export default class ReplayReporter<
       return;
     }
 
-    if (this._testRunShardIdPromise) {
-      return;
-    }
-
-    this._testRunShardIdPromise = this._startTestRunShard();
-    this._pendingWork.push(this._testRunShardIdPromise);
+    // Don't even record test metadata yet unless/until a test is run with the Replay browser (see onTestBegin)
   }
 
   private async _startTestRunShard(): Promise<TestRunPendingWork> {
@@ -670,6 +665,13 @@ export default class ReplayReporter<
   onTestBegin(testExecutionId?: string, metadataFilePath = getMetadataFilePath("REPLAY_TEST", 0)) {
     logger.info("OnTestBegin:Started", { testExecutionId });
 
+    if (this._apiKey && !this._testRunShardIdPromise) {
+      // This method won't be called until a test is run with the Replay browser
+      // We shouldn't save any test metadata until that happens
+      this._testRunShardIdPromise = this._startTestRunShard();
+      this._pendingWork.push(this._testRunShardIdPromise);
+    }
+
     this._errors = [];
     const metadata = {
       ...(this._baseMetadata || {}),
@@ -1145,7 +1147,7 @@ export default class ReplayReporter<
     let completedWork: PromiseSettledResult<PendingWork | undefined>[] = [];
 
     if (this._pendingWork.length) {
-      log("🕑 Completing some outstanding work ...");
+      log("Finishing up. This should only take a moment ...");
     }
 
     while (this._pendingWork.length) {
@@ -1227,14 +1229,14 @@ export default class ReplayReporter<
       numUploaded = uploaded.length;
 
       if (uploaded.length > 0) {
-        output.push(`\n🚀 Successfully uploaded ${uploads.length} recordings:\n`);
+        output.push(`\n🚀 Successfully uploaded ${uploads.length} recordings:`);
         const sortedUploads = sortRecordingsByResult(uploads);
         sortedUploads.forEach(r => {
           output.push(
-            `   ${getTestResultEmoji(r)} ${(r.metadata.title as string | undefined) || "Unknown"}`
+            `\n   ${getTestResultEmoji(r)} ${(r.metadata.title as string | undefined) || "Unknown"}`
           );
           output.push(
-            `      ${process.env.REPLAY_VIEW_HOST || "https://app.replay.io"}/recording/${r.id}\n`
+            `      ${process.env.REPLAY_VIEW_HOST || "https://app.replay.io"}/recording/${r.id}`
           );
         });
       }
@@ -1253,7 +1255,9 @@ export default class ReplayReporter<
       numUploaded,
     });
 
-    log(output.join("\n"));
+    if (output.length > 0) {
+      log(output.join("\n"));
+    }
 
     return results;
   }