moved cypress out of container #6931
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6931 +/- ##
=======================================
Coverage 63.76% 63.76%
=======================================
Files 161 161
Lines 13085 13085
Branches 1812 1812
=======================================
Hits 8344 8344
Misses 4438 4438
Partials 303 303 |
AndrewChubatiuk
force-pushed
the
move-cypress-out-of-container
branch
from
8661182
to
75c7719
Compare
|
@justinclift @eradman could you please review? |
AndrewChubatiuk
force-pushed
the
move-cypress-out-of-container
branch
from
75c7719
to
2905b1b
Compare
|
@AndrewChubatiuk I can't do it tonight (end of day for me already), and I probably can't do it tomorrow either (paperwork day). Can look at it after that though, if no-one has gotten to it. 😄 One question though, as it's not obvious from the PR description, but what's the benefit from moving Cypress out of a container? I can understand upgrading old Cypress version to newer, that's a good thing. 😄 I'm just unsure what the purpose/benefit/etc of removing the container bit is? Note, that's not me arguing, I'm actually asking. 😄 |
|
Moving cypress out of container decreases e2e test run up to 5 minutes, also for local setup no need to build container and install same deps. Also cypress builds from the start once there were changes in viz-lib, which it actually doesn't rely on |
Awesome, that sounds like a good improvement. 😄 |
| const LOGIN_PASSWORD = process.env.CYPRESS_LOGIN_PASSWORD || "password"; | ||
| const ORG_NAME = process.env.CYPRESS_ORG_NAME || "Redash"; | ||
|
|
||
| Cypress.Commands.add("setup", () => { |
There was a problem hiding this comment.
How can I call this from terminal?
I got stuck so it never pass login screen on any test after I typed npm run cypress all
There was a problem hiding this comment.
this cypress command can be called using cy.setup, it's added to client/cypress/support/commands.js
it's executed before e2e tests execution. you need to run yarn cypress run-ci
There was a problem hiding this comment.
I tested it on both node v16 and v20; this file has no access to .env file so please accept PR#24
| const LOGIN_PASSWORD = process.env.CYPRESS_LOGIN_PASSWORD || "password"; | ||
| const ORG_NAME = process.env.CYPRESS_ORG_NAME || "Redash"; | ||
|
|
||
| Cypress.Commands.add("setup", () => { |
There was a problem hiding this comment.
I tested it on both node v16 and v20; this file has no access to .env file so please accept PR#24
Update Cypress tests to use environment variables for login credentials
What type of PR is this?
Description
Moving cypress out of container decreases e2e test run up to 5 minutes, also for local setup no need to build container and install same deps. Also cypress builds from the start once there were changes in viz-lib, which it actually doesn't rely on.
beforehooks in cypressHow is this tested?