Only save test data for tests run with Replay browser #586
Conversation
🦋 Changeset detectedLatest commit: d52c6a5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| 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}` |
There was a problem hiding this comment.
This was leaving a goofy trailing line too, after the last test.
packages/playwright/src/reporter.ts
Outdated
| this._projects[projectName].executed = true; | ||
| } | ||
| // Don't save metadata for non-Replay projects | ||
| if (projectMetadata?.usesReplayBrowser === false) return; |
There was a problem hiding this comment.
It's unclear to me when it can happen but, in theory (aka according to the types), the project is optional.... In such a scenario the tested value resolves to undefined here and based on that code will continue to process this. I wonder if that's intentional behavior or not
There was a problem hiding this comment.
It was also unclear to me about when this could happen. (How would Playwright even know which browser to use if there was no project?)
I just did some local testing and realized that I can comment out the projects config block entirely and Playwright still works:
export default defineConfig({
testDir: "./tests",
fullyParallel: true,
forbidOnly: !!process.env.CI,
retries: 0,
reporter: [
["dot"] as const,
replayReporter({
apiKey: process.env.REPLAY_API_KEY,
upload: true,
}),
],
timeout: 10_000,
use: {
viewport: {
width: 1280,
height: 1024,
},
},
});Adding some logging to the reporter I see that we still get a project back in this case though:
{
grep: /.*/,
grepInvert: null,
outputDir: '/Users/bvaughn/Documents/git/replay/library/test-results',
repeatEach: 1,
retries: 0,
metadata: {},
name: '',
testDir: '/Users/bvaughn/Documents/git/replay/library/tests',
snapshotDir: '/Users/bvaughn/Documents/git/replay/library/tests',
testIgnore: [],
testMatch: '**/*.@(spec|test).?(c|m)[jt]s?(x)',
timeout: 10000,
use: { viewport: { width: 1280, height: 1024 } },
dependencies: [],
teardown: undefined,
__projectId: ''
}So the logic I have here would still hold. That being said, maybe it's safer to switch back to my original approach:
if (!projectMetadata?.usesReplayBrowser) return;
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.
Testing
I've tested the following scenarios...
Upload behavior
Only upload tests run with Replay browser
I was testing a few things with this configuration:
Here is the terminal output from the above test run:
And here are the results in our dashboard which seem to confirm that it worked as expected:
Upload all tests if all are run with Replay browser
Terminal output:
Dashboard has both tests (with recordings)
Don't upload any data if no tests are run with Replay
Same config but with
--project=chromiumso the Replay browser doesn't get run:And does not create a dashboard test run.
Logged warnings
Trying to record tests without the Replay browser being installed
Using the Replay reporter without the Replay browser installed or configured