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

[utils.py] gathering/refactoring into a "utils/" folder #6095

Merged
merged 4 commits into from
Oct 17, 2018

Conversation

mistercrunch
Copy link
Member

@mistercrunch mistercrunch commented Oct 13, 2018

Moving current utils.py into utils/core.py and moving other util
modules under this new "utils/" as well.

Following steps include eroding at "utils/core.py" and breaking it down
into smaller modules.

Unit tests speed up

I did a bunch of unrelated changes that lead to shaving a lot of time off the Python unit test suite. It went for 15-ish minutes to 5-ish minutes. Somehow the examples were getting loaded over and over for each class that was getting triggered in parallel.

On the down side, load_examples now falls off of coverage accounting, meaning we loose 6% of coverage for the piece of code that loads the example. Somehow it's not easy to get nosetests to force a certain order. Personally I think it's worth the tradeoff.

screen shot 2018-10-15 at 3 32 23 pm

@mistercrunch mistercrunch force-pushed the utils_folder branch 5 times, most recently from dd72b3f to d2f3b4f Compare October 14, 2018 05:33
@codecov-io
Copy link

codecov-io commented Oct 14, 2018

Codecov Report

Merging #6095 into master will decrease coverage by 0.32%.
The diff coverage is 85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6095      +/-   ##
==========================================
- Coverage   77.23%   76.91%   -0.33%     
==========================================
  Files          47       47              
  Lines        9349     9362      +13     
==========================================
- Hits         7221     7201      -20     
- Misses       2128     2161      +33
Impacted Files Coverage Δ
superset/utils/import_datasource.py 100% <ø> (ø)
superset/utils/cache.py 48.14% <ø> (ø)
superset/utils/core.py 88.3% <ø> (ø)
superset/utils/dict_import_export.py 21.05% <ø> (ø)
superset/models/sql_lab.py 98.5% <100%> (ø) ⬆️
superset/connectors/druid/views.py 67.58% <100%> (+0.22%) ⬆️
superset/db_engine_specs.py 56.17% <100%> (+0.06%) ⬆️
superset/sql_lab.py 71.59% <100%> (ø) ⬆️
superset/views/base.py 67.44% <100%> (+0.19%) ⬆️
superset/connectors/sqla/views.py 63.39% <100%> (+0.32%) ⬆️
... and 14 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 af0ffa4...0bc0b13. Read the comment docs.

@mistercrunch mistercrunch force-pushed the utils_folder branch 3 times, most recently from c955690 to 9f384b9 Compare October 16, 2018 03:50
@@ -23,22 +20,12 @@ class SupersetTestCase(unittest.TestCase):
examples_loaded = False
Copy link
Member

Choose a reason for hiding this comment

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

I think you can remove this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call

Moving current utils.py into utils/core.py and moving other *util*
modules under this new "utils/" as well.

Following steps include eroding at "utils/core.py" and breaking it down
into smaller modules.
@betodealmeida betodealmeida merged commit bbfd69a into apache:master Oct 17, 2018
@mistercrunch mistercrunch deleted the utils_folder branch October 17, 2018 01:01
bipinsoniguavus pushed a commit to ThalesGroup/incubator-superset that referenced this pull request Dec 26, 2018
* [utils] gathering/refactoring into a "utils/" folder

Moving current utils.py into utils/core.py and moving other *util*
modules under this new "utils/" as well.

Following steps include eroding at "utils/core.py" and breaking it down
into smaller modules.

* Improve tests

* Make loading examples in scope for tests

* Remove test class attrs examples_loaded and requires_examples
@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.

None yet

4 participants