-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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
fix: do not import superset_config on tests #11193
Conversation
Codecov Report
@@ Coverage Diff @@
## master #11193 +/- ##
==========================================
- Coverage 65.60% 61.35% -4.25%
==========================================
Files 828 832 +4
Lines 39167 39449 +282
Branches 3589 3598 +9
==========================================
- Hits 25694 24205 -1489
- Misses 13361 15062 +1701
- Partials 112 182 +70
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@dpgaspar do you have any idea why this test is failing? I noticed you have disabled it for Presto. |
I created #11196 to fix the failing unit test. |
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.
LGTM, but I would suggest using strtobool
superset/utils/core.py
Outdated
|
||
|
||
def is_test() -> bool: | ||
return "SUPERSET_TESTENV" in os.environ |
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.
Here it might be worth considering using strtobool
, which will parse the string value for known string representations of boolean values so the env variable can also explicitly be set to True
:
from distutils.util import strtobool
strtobool(os.environ.get("SUPERSET_TESTENV", "false"))
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.
Thanks, @villebro! I'll update the PR.
* Do not import superset_config on tests * Use strtobool
SUMMARY
When running unit tests we import any
superset_config.py
files that are in the path, resulting in inconsistent local tests.I modified the code so that we set an environment flag when running unit tests, and skip importing
superset_config.py
when the flag is set.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
NA
TEST PLAN
I had this custom configuration file:
Running SQLite unit tests will then fail:
With this PR,
superset_config.py
is ignored and the tests pass.ADDITIONAL INFORMATION