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
feat: Switch from nosetest to pytest #10177
Conversation
7bf9ade
to
a5094ce
Compare
|
Ah, sorry for missing that. Are those nosetests options in |
781aca0
to
2a41041
Compare
@ktmud taking into account that we want to modify the local tests vs CI behavior that section is probably not needed. |
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.
Code LGTM! Do you know why did Codecov say overall coverage dropped by 17.79%?
scripts/python_tests.sh
Outdated
@@ -23,5 +23,5 @@ echo "Superset config module: $SUPERSET_CONFIG" | |||
|
|||
superset db upgrade | |||
superset init | |||
nosetests --stop tests/load_examples_test.py | |||
nosetests --stop --exclude=load_examples_test tests | |||
pytest --maxfail=1 tests/load_examples_test.py |
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.
Do we want coverage for this file as well? I don't think it's needed but it was covered in previous setup.
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.
my preference is not to count it, to me this is more a fixture setup rather than an actual test.
However do not have a preference here.
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.
https://codecov.io/gh/apache/incubator-superset/pull/10177/commits - looks like it was the reason for dropped coverage - will keep playing with it
|
06d4a35
to
2a41041
Compare
4124402
to
86342e6
Compare
85296ad
to
eaafbb7
Compare
@ktmud and @john-bodley could you please take another look? |
b5a8955
to
b3ffb83
Compare
Codecov seems to be stuck at "waiting for CI to complete", just like in #10142 . If you can get it fixed, that'd be great. If not, we can probably just merge this as is and see if it's a recurring issue in the next PR. There were also some deprecation warnings in the logs that might worth fixing (could be in another PR):
|
@ktmud this is interesting, report is generated on their side: https://codecov.io/gh/apache/incubator-superset/pull/10177 |
Fix schedule tests Collect pytest coverage Move pytest config into pytest.ini Move cov to the pytest.ini
6a695f9
to
706c85a
Compare
Codecov Report
@@ Coverage Diff @@
## master #10177 +/- ##
==========================================
- Coverage 65.84% 65.69% -0.15%
==========================================
Files 594 594
Lines 31486 31492 +6
Branches 3222 3223 +1
==========================================
- Hits 20732 20689 -43
- Misses 10574 10623 +49
Partials 180 180
Continue to review full report at Codecov.
|
ec64614
to
0eb5138
Compare
0eb5138
to
d26ba3c
Compare
* Switch from nosetest to pytest Fix schedule tests Collect pytest coverage Move pytest config into pytest.ini Move cov to the pytest.ini * Append coverage for the 2nd run * Add coverage to all commands * Coverage only for tests * Get coverage from 1 place * Rename classes to be pytest compatible * Test coverage for examples and tests * Max diff to -1 * Explain how to run pytest for the whole project * Do not append code coverage for the main run * Do not run coverage on examples Co-authored-by: bogdan kyryliuk <bogdankyryliuk@dropbox.com>
Changed test runner from nosetests to pytest
Tox locally seems to be working just fine and contributor instructions don't need to change.
Changes:
Naming conventions:
Testing