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

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

Merged
merged 3 commits into from
Jul 3, 2024
Merged
Show file tree
Hide file tree
Changes from all 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/late-cars-doubt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@replayio/playwright": patch
---

Only save test data for tests run with Replay browser
73 changes: 44 additions & 29 deletions packages/playwright/src/reporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
Expand Down Expand Up @@ -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}`
Expand All @@ -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,
Expand All @@ -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));
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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"
Expand All @@ -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]:");
}
bvaughn marked this conversation as resolved.
Show resolved Hide resolved
console.warn(`[replay.io]: ${line}`);
});
}
Expand Down
26 changes: 15 additions & 11 deletions packages/test-utils/src/reporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -446,12 +446,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> {
Expand Down Expand Up @@ -673,6 +668,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 || {}),
Expand Down Expand Up @@ -1148,7 +1150,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) {
Expand Down Expand Up @@ -1230,14 +1232,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}`
Comment on lines -1233 to +1242
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was leaving a goofy trailing line too, after the last test.

);
});
}
Expand All @@ -1256,7 +1258,9 @@ export default class ReplayReporter<
numUploaded,
});

log(output.join("\n"));
if (output.length > 0) {
log(output.join("\n"));
}
bvaughn marked this conversation as resolved.
Show resolved Hide resolved

return results;
}
Expand Down