-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Require an explicit knob to allow uploading ReplayIO recordings #42649
Conversation
@iethree something weird is happening now. As far as I can tell, the browser is installed But then Cypress fails due to the "invalid browser" For some reason, it doesn't see it anymore. |
2ab68ce
to
8da958e
Compare
8da958e
to
1a81844
Compare
Even though we're not using it in this workflow. 🤷
e2e/support/config.js
Outdated
@@ -22,6 +22,7 @@ const sourceVersion = process.env["CROSS_VERSION_SOURCE"]; | |||
const targetVersion = process.env["CROSS_VERSION_TARGET"]; | |||
|
|||
const runWithReplay = process.env["CYPRESS_REPLAYIO_ENABLED"]; | |||
const uploadReplayRecordings = process.env["CYPRESS_REPLAYIO_ENABLE_UPLOAD"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest removing CYPRESS_REPLAYIO_ENABLE_UPLOAD: 0
from the other file and combining it with this change:
const uploadReplayRecordings = process.env["CYPRESS_REPLAYIO_ENABLE_UPLOAD"]; | |
const uploadReplayRecordings = !!process.env["CYPRESS_REPLAYIO_ENABLE_UPLOAD"]; |
42c17e7
to
0109c60
Compare
0109c60
to
4199789
Compare
@@ -47,9 +47,6 @@ jobs: | |||
QA_DB_ENABLED: true | |||
CYPRESS_QA_DB_MONGO: true | |||
CYPRESS_REPLAYIO_ENABLED: 1 | |||
CYPRESS_REPLAYIO_ENABLE_UPLOAD: 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't work.
false
doesn't work either.
The only solution was to omit it.
|
@nemanjaglumac Did you forget to add a milestone to the issue for this PR? When and where should I add a milestone? |
I upgraded the Replay's Cypress plugin in #42507
They introduced a breaking change that I didn't know about - the runner will now throw if the API key is missing but the
runWithReplay
is true in the Cypress config.The solution is to explicitly set the
upload
to true or false, depending on the context.We're now instructing the main E2E workflow to upload recordings, and we're omitting this env var from the stress-test workflow.
Slack context
Test
Successful run using the latest commit:
https://github.com/metabase/metabase/actions/runs/9095182474/job/24997844000