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

build(deps-dev): update cypress to 5.5.0, improvements for running locally #11603

Merged
merged 8 commits into from
Nov 10, 2020

Conversation

robdiciuccio
Copy link
Member

@robdiciuccio robdiciuccio commented Nov 6, 2020

SUMMARY

Updates cypress to latest version (5.5.0) due to patched performance issues in 5.2.0.

Improvements for running Cypress locally, including:

  • Fix OOM crashing of headed chrome test runs (via numTestsKeptInMemory)
  • Update docs and build script to use chrome as cypress browser
  • Correct documentation on manual cypress runs
  • tox: add missing cypress testenv
  • tox: cleanup orphaned background flask processes

Also 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

  • Has associated issue:
  • Changes UI
  • Requires DB Migration.
  • Confirm DB Migration upgrade and downgrade tested.
  • Introduces new feature or API
  • Removes existing feature or API

@pull-request-size pull-request-size bot added size/L and removed size/M labels Nov 6, 2020
superset load_test_users
superset load_examples
superset load_examples --load-test-data
Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

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

Copy link
Member Author

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"
Copy link
Member

@eschutho eschutho Nov 6, 2020

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

Copy link
Member

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?

Copy link
Member

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.

Copy link
Member Author

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-io
Copy link

codecov-io commented Nov 6, 2020

Codecov Report

Merging #11603 (f8274a1) into master (bd79bd2) will decrease coverage by 11.48%.
The diff coverage is n/a.

Impacted file tree graph

@@             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     
Flag Coverage Δ
cypress 54.92% <ø> (ø)
javascript ?
python ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...uperset-frontend/src/dashboard/util/dnd-reorder.js 0.00% <0.00%> (-100.00%) ⬇️
...rset-frontend/src/dashboard/util/getEmptyLayout.js 0.00% <0.00%> (-100.00%) ⬇️
...dashboard/components/resizable/ResizableHandle.jsx 0.00% <0.00%> (-100.00%) ⬇️
.../src/dashboard/util/getFilterScopeFromNodesTree.js 0.00% <0.00%> (-93.48%) ⬇️
...src/dashboard/components/gridComponents/Header.jsx 10.52% <0.00%> (-86.85%) ⬇️
...rc/dashboard/components/gridComponents/Divider.jsx 13.33% <0.00%> (-86.67%) ⬇️
...ontend/src/dashboard/util/getDashboardFilterKey.ts 14.28% <0.00%> (-85.72%) ⬇️
...set-frontend/src/views/CRUD/welcome/EmptyState.tsx 2.94% <0.00%> (-84.56%) ⬇️
...src/components/FilterableTable/FilterableTable.tsx 2.64% <0.00%> (-83.43%) ⬇️
...et-frontend/src/components/ListView/ActionsBar.tsx 11.11% <0.00%> (-81.20%) ⬇️
... and 701 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bd79bd2...f8274a1. Read the comment docs.

tox.ini Show resolved Hide resolved
tox.ini Show resolved Hide resolved
@robdiciuccio
Copy link
Member Author

cc @adam-stasiak @ktmud

@mistercrunch mistercrunch merged commit 14aa729 into apache:master Nov 10, 2020
auxten pushed a commit to auxten/incubator-superset that referenced this pull request Nov 20, 2020
…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
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.0.0 labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/L 🚢 1.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants