-
Notifications
You must be signed in to change notification settings - Fork 14.2k
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
build(deps-dev): update cypress to 5.5.0, improvements for running locally #11603
Conversation
superset load_test_users | ||
superset load_examples | ||
superset load_examples --load-test-data |
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.
👍
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.
Do we want to drop database before? It can be done with modification of cli.py
@with_appcontext
def create_db() -> None:
"""Resets database"""
db.create_all()
@superset.command()
@with_appcontext
@click.confirmation_option(prompt="Are you sure you want to drop the db?")
def drop_db() -> None:
"""Removes database"""
db.drop_all()
And then executed:
superset drop_db --yes
superset create_db
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.
@adam-stasiak definitely see the value in having a drop db (or at least a drop table) function prior to running tests, but I wouldn't necessarily make it mandatory.
@@ -624,32 +624,36 @@ We use [Cypress](https://www.cypress.io/) for integration tests. Tests can be ru | |||
|
|||
```bash | |||
export SUPERSET_CONFIG=tests.superset_test_config | |||
export SUPERSET_TESTENV=true | |||
export ENABLE_REACT_CRUD_VIEWS=true | |||
export CYPRESS_BASE_URL="http://localhost:8081" |
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.
one piece of feedback that Harrison gave on this, is that this export CYPRESS_BASE_URL
needs to be run in the same shell window as the cypress command. Should we move this line down, or else put it on the same line, such as CYPRESS_BASE_URL="http://localhost:8081" npm run cypress-run-chrome
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.
Doesn't export
effectively does that for the whole bash session?
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.
Yes, but if you open a new tab, window, or pane, it will create a new a new session.
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.
@eschutho true, but the same holds for the other env vars. tox, while a pain, does abstract all of the env vars away from the user.
Codecov Report
@@ Coverage Diff @@
## master #11603 +/- ##
===========================================
- Coverage 66.41% 54.92% -11.49%
===========================================
Files 878 412 -466
Lines 42272 14315 -27957
Branches 3949 3674 -275
===========================================
- Hits 28074 7863 -20211
+ Misses 14095 6283 -7812
- Partials 103 169 +66
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
…cally (apache#11603) * reduce numTestsKeptInMemory to 0 to prevent Chrome OOM crash * Add cypress-run-chrome script, update docs and cypress_build.sh * Upgrade Cypress to latest version * tox: add missing 'cypress' env, cleanup flask server * Fix failing dashboard edit test syntax * Reduce retry count to 1 * Prettier test
SUMMARY
Updates cypress to latest version (5.5.0) due to patched performance issues in 5.2.0.
Improvements for running Cypress locally, including:
numTestsKeptInMemory
)chrome
as cypress browsercypress
testenvAlso reduces the number of retries globally from 2 to 1 to reduce time spent on failed runs.
TEST PLAN
See updated CONTRIBUTING.md documentation.
ADDITIONAL INFORMATION