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

[cypress] Combine multiple tests under visualizations into single test to save running time #6019

Merged
merged 3 commits into from
Oct 2, 2018

Conversation

kristw
Copy link
Contributor

@kristw kristw commented Oct 2, 2018

Extended the work from #6007

tl;dr Based on the hypothesis that is there is an overhead to start up browser for each test, this PR nests all vis tests another level of describe() in all.js and tell cypress to ignore the individual test files.

  • Make each visualization test file export a function instead of declaring a test immediately.
  • Rename vis test files to _xxx.js and add cypress rule to ignore any file that begins with underscore (_).
  • Create all.js which wraps all the test under a single run using nested describe()

Good speed up both locally and on ci. First attempt on ci is 10mins compared to previous run of 15mins.

@graceguo-supercat @michellethomas @williaster

@kristw
Copy link
Contributor Author

kristw commented Oct 2, 2018

I am going to re-run to get more data point.

@kristw kristw closed this Oct 2, 2018
@kristw kristw reopened this Oct 2, 2018
@codecov-io
Copy link

Codecov Report

Merging #6019 into master will increase coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6019      +/-   ##
==========================================
+ Coverage    64.6%   64.65%   +0.04%     
==========================================
  Files         446      446              
  Lines       23814    23814              
  Branches     2639     2639              
==========================================
+ Hits        15386    15396      +10     
+ Misses       8415     8405      -10     
  Partials       13       13
Impacted Files Coverage Δ
superset/connectors/sqla/models.py 81.13% <0%> (+0.75%) ⬆️
superset/db_engine_specs.py 55.7% <0%> (+0.87%) ⬆️

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 7388294...059dcf0. Read the comment docs.

Copy link

@graceguo-supercat graceguo-supercat left a comment

Choose a reason for hiding this comment

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

LGTM

@graceguo-supercat graceguo-supercat merged commit f9344f1 into apache:master Oct 2, 2018
@kristw kristw deleted the kristw-cypress-merge branch October 3, 2018 00:18
betodealmeida pushed a commit to lyft/incubator-superset that referenced this pull request Oct 12, 2018
…t to save running time (apache#6019)

* add histogram test

* add compare test

* merge all vis tests under single test
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.28.0 labels Feb 27, 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 🚢 0.28.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants