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

Reduce data loaded before loading tests #6298

Merged
merged 2 commits into from
Nov 19, 2018

Conversation

timifasubaa
Copy link
Contributor

@timifasubaa timifasubaa commented Nov 8, 2018

This PR pushes down the CI build times for python tests by reducing the tables loaded before running the tests.

@john-bodley @mistercrunch @michellethomas

@timifasubaa timifasubaa force-pushed the reduce_tests_surface branch 9 times, most recently from a6234ed to d84db1c Compare November 8, 2018 07:40
@timifasubaa timifasubaa changed the title [WIP] reduce data loaded before loading tests Reduce data loaded before loading tests Nov 8, 2018
@codecov-io
Copy link

codecov-io commented Nov 8, 2018

Codecov Report

Merging #6298 into master will decrease coverage by 3.93%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6298      +/-   ##
==========================================
- Coverage   77.31%   73.38%   -3.94%     
==========================================
  Files          67       67              
  Lines        9581     9582       +1     
==========================================
- Hits         7408     7032     -376     
- Misses       2173     2550     +377
Impacted Files Coverage Δ
superset/cli.py 35.43% <0%> (-0.15%) ⬇️
superset/data/deck.py 11.29% <0%> (-87.1%) ⬇️
superset/data/flights.py 25.8% <0%> (-74.2%) ⬇️
superset/data/misc_dashboard.py 27.27% <0%> (-72.73%) ⬇️
superset/data/country_map.py 27.58% <0%> (-72.42%) ⬇️
superset/data/random_time_series.py 28.57% <0%> (-71.43%) ⬇️
superset/data/long_lat.py 29.72% <0%> (-70.28%) ⬇️
superset/data/multiformat_time_series.py 21.62% <0%> (-67.57%) ⬇️
superset/data/bart_lines.py 38.46% <0%> (-61.54%) ⬇️
superset/data/sf_population_polygons.py 39.13% <0%> (-60.87%) ⬇️
... and 6 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 c17de39...db142d6. Read the comment docs.

@mistercrunch
Copy link
Member

Ballpark before/after times?

@timifasubaa
Copy link
Contributor Author

@mistercrunch Shaved off about ~ 1 minute from the Python tests. From 5-6 mutes to 4 minutes

@michellethomas
Copy link
Contributor

Not sure how others feel about removing the loading of these data sets completely. I wonder if we can have a flag for loading all data vs loading basic data sets. That way if some tests want to use all data they can.

@timifasubaa
Copy link
Contributor Author

@michellethomas Agree. I updated the PR to only remove it completely from the tests loading.
The superset load_examples command will continue to load all the data but when the -t flag or equivalent is added, it will add only the subset that's used in the tests.

@timifasubaa
Copy link
Contributor Author

PING

@mistercrunch
Copy link
Member

@michellethomas superset load_examples still loads everything, people can add more datasets to load if they want to testing purposes. LGTM if @michellethomas is cool with it

superset/cli.py Outdated
if load_test_data:
print('Loading [Unicode test data]')
data.load_unicode_test_data()
if load_test_data:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is nested under if not load_test_data:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@michellethomas
Copy link
Contributor

lgtm

@timifasubaa timifasubaa merged commit 91b758f into apache:master Nov 19, 2018
bipinsoniguavus pushed a commit to ThalesGroup/incubator-superset that referenced this pull request Dec 26, 2018
* reduce data loaded before loading tests

* make cypress only load needed tests
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.34.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.34.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants