Correctly identify test executions by utilizing repeatEachIndex (#536)

* Correctly identify test executions by utilizing `repeatEachIndex` * tweak advanced upload handling to accomodate for `repeatEach` * Fixed projectName * avoid spreading redundant props * avoid reuploading the same recording * revert major schema bump * include filepaths in executionIds * retract and use execution groups * update metadata structs in the new cli * add changesets * Update packages/test-utils/src/reporter.ts Co-authored-by: Brian Vaughn <[email protected]> * fixed minimized flaky updates with all threshold * cleanup the upload selection logic * always update `didUploadStatuses` * make sure that the same recording is not uloaded more than once * add early return to `_enqueueUploads` --------- Co-authored-by: Brian Vaughn <[email protected]>
replayio · Jun 19, 2024 · 8343abe · 8343abe
1 parent 4abab40
commit 8343abe
Show file tree

Hide file tree

Showing 17 changed files with 295 additions and 137 deletions.
diff --git a/.changeset/eight-tips-sin.md b/.changeset/eight-tips-sin.md
@@ -0,0 +1,5 @@
+---
+"@replayio/test-utils": major
+---
+
+Breaking changes in the reporter's public API
diff --git a/.changeset/empty-peaches-rhyme.md b/.changeset/empty-peaches-rhyme.md
@@ -0,0 +1,5 @@
+---
+"replayio": minor
+---
+
+Export a new version of the new test run metadata validator
diff --git a/.changeset/plenty-hotels-dress.md b/.changeset/plenty-hotels-dress.md
@@ -0,0 +1,5 @@
+---
+"@replayio/playwright": minor
+---
+
+Correctly correlate recordings with tests utilizing `repeatEach`
diff --git a/.changeset/wicked-numbers-rhyme.md b/.changeset/wicked-numbers-rhyme.md
@@ -0,0 +1,5 @@
+---
+"@replayio/test-utils": major
+---
+
+Bumped the minimum supported node version to 18
diff --git a/packages/cypress/src/reporter.ts b/packages/cypress/src/reporter.ts
@@ -167,6 +167,8 @@ class CypressReporter {
   private getTestResults(spec: Cypress.Spec, result: CypressCommandLine.RunResult): Test[] {
     const placeholderTest: Test = {
       id: 0,
+      executionGroupId: "single",
+      executionId: [spec.relative, 1].join("-"),
       approximateDuration: 0,
       source: {
         title: spec.relative,
@@ -204,6 +206,7 @@ class CypressReporter {
     }
 
     let testsWithoutSteps = getTestsFromResults(
+      spec,
       result.tests,
       this.steps.filter(s => s.event === "test:start")
     );

diff --git a/packages/cypress/src/steps.ts b/packages/cypress/src/steps.ts
@@ -58,6 +58,7 @@ function simplifyArgs(args?: any[]) {
 }
 
 function getTestsFromResults(
+  spec: Cypress.Spec,
   resultTests: CypressCommandLine.TestResult[],
   testStartSteps: StepEvent[]
 ) {
@@ -80,12 +81,15 @@ function getTestsFromResults(
 
     return result.attempts.map<Test>((a, attemptIndex) => {
       const startTestStep = getStartTestStep(result);
+      const attempt = attemptIndex + 1;
       return {
         id: startTestStep?.testId ?? attemptIndex,
+        executionGroupId: "single",
+        executionId: [spec.relative, attempt, ...scope, title].join("-"),
         // those properties don't exist since Cypress 13: https://github.com/cypress-io/cypress/pull/27230
         // TODO: remove it in PRO-640
         approximateDuration: (a as any).duration || (a as any).wallClockDuration || 0,
-        attempt: attemptIndex + 1,
+        attempt,
         maxAttempts: startTestStep?.maxAttempts ?? 1,
         source: {
           title,

diff --git a/packages/jest/src/runner.ts b/packages/jest/src/runner.ts
@@ -6,7 +6,6 @@ import {
   removeAnsiCodes,
   getMetadataFilePath as getMetadataFilePathBase,
   initMetadataFile,
-  TestIdContext,
 } from "@replayio/test-utils";
 import type Runtime from "jest-runtime";
 import path from "path";
@@ -63,14 +62,6 @@ const ReplayRunner = async (
     };
   }
 
-  function getTestIdContext(test: Circus.TestEntry): TestIdContext {
-    const source = getSource(test);
-    return {
-      ...source,
-      attempt: test.invocations,
-    };
-  }
-
   function getWorkerIndex() {
     return +(process.env.JEST_WORKER_ID || 0);
   }
@@ -82,7 +73,11 @@ const ReplayRunner = async (
   }
 
   function handleTestStart(test: Circus.TestEntry) {
-    reporter.onTestBegin(getTestIdContext(test), getMetadataFilePath(getWorkerIndex()));
+    const source = getSource(test);
+    reporter.onTestBegin(
+      [test.invocations, ...source.scope, source.title].join("-"),
+      getMetadataFilePath(getWorkerIndex())
+    );
   }
 
   function getErrorMessage(errors: any[]) {
@@ -95,11 +90,14 @@ const ReplayRunner = async (
 
   function handleResult(test: Circus.TestEntry, passed: boolean) {
     const errorMessage = getErrorMessage(test.errors);
+    const source = getSource(test);
 
     reporter.onTestEnd({
       tests: [
         {
           id: 0,
+          executionGroupId: "single",
+          executionId: [relativePath, 1, ...source.scope, source.title].join("-"),
           attempt: 1,
           maxAttempts: 1,
           approximateDuration: test.duration || 0,

diff --git a/packages/playwright/src/fixture.ts b/packages/playwright/src/fixture.ts
@@ -31,8 +31,10 @@ export interface FixtureStepStart extends StepStartDetail {
   id: string;
 }
 
-export interface TestIdData {
-  id: number;
+export interface TestExecutionIdData {
+  filePath: string;
+  projectName: string | undefined;
+  repeatEachIndex: number;
   attempt: number;
   source: {
     title: string;
@@ -42,7 +44,7 @@ export interface TestIdData {
 
 interface FixtureStepStartEvent extends FixtureStepStart {
   event: "step:start";
-  test: TestIdData;
+  test: TestExecutionIdData;
 }
 
 interface StepEndDetail {
@@ -55,12 +57,12 @@ export interface FixtureStepEnd extends StepEndDetail {
 
 interface FixtureStepEndEvent extends FixtureStepEnd {
   event: "step:end";
-  test: TestIdData;
+  test: TestExecutionIdData;
 }
 
 interface ReporterErrorEvent extends ReporterError {
   event: "error";
-  test: TestIdData;
+  test: TestExecutionIdData;
 }
 
 export type FixtureEvent = FixtureStepStartEvent | FixtureStepEndEvent | ReporterErrorEvent;
@@ -191,8 +193,10 @@ export async function replayFixture(
     throw error;
   }
 
-  const testIdData: TestIdData = {
-    id: 0,
+  const testIdData: TestExecutionIdData = {
+    filePath: testInfo.file,
+    projectName: testInfo.project.name,
+    repeatEachIndex: testInfo.repeatEachIndex,
     attempt: testInfo.retry + 1,
     source: {
       title: testInfo.title,

diff --git a/packages/playwright/src/reporter.ts b/packages/playwright/src/reporter.ts
@@ -10,18 +10,17 @@ import {
   removeAnsiCodes,
   ReplayReporter,
   ReplayReporterConfig,
-  TestIdContext,
   TestMetadataV2,
 } from "@replayio/test-utils";
 import dbg from "debug";
-import { existsSync, readFileSync } from "fs";
+import { readFileSync } from "fs";
 import path from "path";
 import { WebSocketServer } from "ws";
 
 type UserActionEvent = TestMetadataV2.UserActionEvent;
 
 import { getPlaywrightBrowserPath } from "@replayio/replay";
-import { FixtureStepStart, ParsedErrorFrame, TestIdData } from "./fixture";
+import { FixtureStepStart, ParsedErrorFrame, TestExecutionIdData } from "./fixture";
 import { StackFrame } from "./playwrightTypes";
 import { getServerPort, startServer } from "./server";
 
@@ -129,23 +128,31 @@ class ReplayPlaywrightReporter implements Reporter {
     });
   }
 
-  getFixtureData(test: TestIdData) {
-    const key = this.getTestKey(test);
-    this.fixtureData[key] ??= {
+  getFixtureData(test: TestExecutionIdData) {
+    const id = this._getTestExecutionId(test);
+    this.fixtureData[id] ??= {
       steps: [],
       stacks: {},
       filenames: new Set(),
     };
 
-    return this.fixtureData[key];
+    return this.fixtureData[id];
   }
 
-  getTestKey(test: TestIdData) {
-    return [test.id, test.attempt, ...test.source.scope, test.source.title].join("-");
-  }
-
-  getTestId(test: TestCase) {
-    return test.titlePath().join("-");
+  // Playwright alrady 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}`
+  // before doing that all recipients of `TestExecutionIdData` should be rechecked to see if such a change would be safe
+  private _getTestExecutionId(test: TestExecutionIdData) {
+    return [
+      test.filePath,
+      test.projectName ?? "",
+      test.repeatEachIndex,
+      test.attempt,
+      ...test.source.scope,
+      test.source.title,
+    ].join("-");
   }
 
   getSource(test: TestCase) {
@@ -155,13 +162,6 @@ class ReplayPlaywrightReporter implements Reporter {
     };
   }
 
-  getTestIdContext(test: TestCase, testResult: TestResult): TestIdContext {
-    return {
-      ...this.getSource(test),
-      attempt: testResult.retry + 1,
-    };
-  }
-
   onBegin({ version, projects }: FullConfig) {
     const replayBrowserPath = getPlaywrightBrowserPath("chromium");
     this._foundReplayBrowser = !!projects.find(
@@ -172,13 +172,17 @@ class ReplayPlaywrightReporter implements Reporter {
   }
 
   onTestBegin(test: TestCase, testResult: TestResult) {
-    this.reporter.onTestBegin(
-      this.getTestIdContext(test, testResult),
-      getMetadataFilePath(testResult.workerIndex)
-    );
+    const testExecutionId = this._getTestExecutionId({
+      filePath: test.location.file,
+      projectName: test.parent.project()?.name,
+      repeatEachIndex: test.repeatEachIndex,
+      attempt: testResult.retry + 1,
+      source: this.getSource(test),
+    });
+    this.reporter.onTestBegin(testExecutionId, getMetadataFilePath(testResult.workerIndex));
   }
 
-  getStepsFromFixture(test: TestIdData) {
+  getStepsFromFixture(test: TestExecutionIdData) {
     const hookMap: Record<
       "afterAll" | "afterEach" | "beforeAll" | "beforeEach",
       UserActionEvent[]
@@ -242,16 +246,18 @@ class ReplayPlaywrightReporter implements Reporter {
     // skipped tests won't have a reply so nothing to do here
     if (status === "skipped") return;
 
-    const testIdData = {
-      id: 0,
+    const testExecutionIdData = {
+      filePath: test.location.file,
+      projectName: test.parent.project()?.name,
+      repeatEachIndex: test.repeatEachIndex,
       attempt: result.retry + 1,
       source: this.getSource(test),
     };
 
-    const events = this.getStepsFromFixture(testIdData);
+    const events = this.getStepsFromFixture(testExecutionIdData);
 
     const relativePath = test.titlePath()[2];
-    const { stacks, filenames } = this.getFixtureData(testIdData);
+    const { stacks, filenames } = this.getFixtureData(testExecutionIdData);
     filenames.add(test.location.file);
     let playwrightMetadata: Record<string, any> | undefined;
 
@@ -275,7 +281,11 @@ class ReplayPlaywrightReporter implements Reporter {
 
     const tests = [
       {
-        ...testIdData,
+        id: 0,
+        attempt: testExecutionIdData.attempt,
+        source: testExecutionIdData.source,
+        executionGroupId: String(testExecutionIdData.repeatEachIndex),
+        executionId: this._getTestExecutionId(testExecutionIdData),
         maxAttempts: test.retries + 1,
         approximateDuration: test.results.reduce((acc, r) => acc + r.duration, 0),
         result: status === "interrupted" ? ("unknown" as const) : status,

diff --git a/packages/playwright/src/server.ts b/packages/playwright/src/server.ts
@@ -1,7 +1,7 @@
 import { ReporterError } from "@replayio/test-utils";
 import dbg from "debug";
 import { WebSocketServer } from "ws";
-import { FixtureEvent, FixtureStepEnd, FixtureStepStart, TestIdData } from "./fixture";
+import { FixtureEvent, FixtureStepEnd, FixtureStepStart, TestExecutionIdData } from "./fixture";
 
 const debug = dbg("replay:playwright:server");
 const debugMessages = debug.extend("messages");
@@ -13,9 +13,9 @@ export function startServer({
   onError,
 }: {
   port?: number;
-  onStepStart?: (test: TestIdData, stepStart: FixtureStepStart) => void;
-  onStepEnd?: (test: TestIdData, stepEnd: FixtureStepEnd) => void;
-  onError?: (test: TestIdData, error: ReporterError) => void;
+  onStepStart?: (test: TestExecutionIdData, stepStart: FixtureStepStart) => void;
+  onStepEnd?: (test: TestExecutionIdData, stepEnd: FixtureStepEnd) => void;
+  onError?: (test: TestExecutionIdData, error: ReporterError) => void;
 }) {
   debug("Starting server on %d with handlers %o", port, {
     onStepStart: !!onStepStart,

diff --git a/packages/replay/src/metadata/test/v2.ts b/packages/replay/src/metadata/test/v2.ts
@@ -1,15 +1,15 @@
 import {
+  Infer,
   array,
+  assign,
   defaulted,
   enums,
+  nullable,
   number,
   object,
   optional,
-  string,
-  nullable,
-  Infer,
-  assign,
   record,
+  string,
 } from "superstruct";
 
 import { firstEnvValueOf } from "../env";
@@ -120,6 +120,8 @@ const test_v2_1_0 = assign(
 const test_v2_2_0 = assign(
   test_v2_1_0,
   object({
+    executionId: string(),
+    executionGroupId: string(),
     maxAttempts: number(),
   })
 );