Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correctly identify test executions by utilizing repeatEachIndex #536

Merged
merged 17 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/eight-tips-sin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@replayio/test-utils": major
---

Breaking changes in the reporter's public API
5 changes: 5 additions & 0 deletions .changeset/empty-peaches-rhyme.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"replayio": minor
---

Export a new version of the new test run metadata validator
5 changes: 5 additions & 0 deletions .changeset/plenty-hotels-dress.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@replayio/playwright": minor
---

Correctly correlate recordings with tests utilizing `repeatEach`
5 changes: 5 additions & 0 deletions .changeset/wicked-numbers-rhyme.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@replayio/test-utils": major
---

Bumped the minimum supported node version to 18
3 changes: 3 additions & 0 deletions packages/cypress/src/reporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -204,6 +206,7 @@ class CypressReporter {
}

let testsWithoutSteps = getTestsFromResults(
spec,
result.tests,
this.steps.filter(s => s.event === "test:start")
);
Expand Down
6 changes: 5 additions & 1 deletion packages/cypress/src/steps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ function simplifyArgs(args?: any[]) {
}

function getTestsFromResults(
spec: Cypress.Spec,
resultTests: CypressCommandLine.TestResult[],
testStartSteps: StepEvent[]
) {
Expand All @@ -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,
Expand Down
18 changes: 8 additions & 10 deletions packages/jest/src/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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);
}
Expand All @@ -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[]) {
Expand All @@ -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,
Expand Down
18 changes: 11 additions & 7 deletions packages/playwright/src/fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -42,7 +44,7 @@ export interface TestIdData {

interface FixtureStepStartEvent extends FixtureStepStart {
event: "step:start";
test: TestIdData;
test: TestExecutionIdData;
}

interface StepEndDetail {
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
70 changes: 40 additions & 30 deletions packages/playwright/src/reporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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) {
Expand All @@ -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(
Expand All @@ -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[]
Expand Down Expand Up @@ -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;

Expand All @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions packages/playwright/src/server.ts
Original file line number Diff line number Diff line change
@@ -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");
Expand All @@ -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,
Expand Down
10 changes: 6 additions & 4 deletions packages/replay/src/metadata/test/v2.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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(),
})
);
Expand Down
Loading