-
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
ci: make presto hive tests to cover only chartData and sqljson #17782
Conversation
Codecov Report
@@ Coverage Diff @@
## master #17782 +/- ##
==========================================
+ Coverage 66.73% 67.08% +0.34%
==========================================
Files 1609 1609
Lines 64897 64897
Branches 6866 6866
==========================================
+ Hits 43306 43533 +227
+ Misses 19725 19498 -227
Partials 1866 1866
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Presto/Hive tests are running only 2 suites of tests now chartData nd sql_lab. |
are there database creation/api tests we should include here too? |
Can you explain a bit why you think it is related? |
@etr2460 can you point the relevant tests that you want to be covered here? |
f4ac502
to
883aff6
Compare
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.
i don't know the specific ones (not super familiar with the backend) so i'll stamp to unblock here. @bkyryliuk @villebro and @john-bodley would probably be the ones who know most about the backend to tag other tests. but this seems like a decent step (and could allow us to make these tests required for PRs too)
Hive and presto bottle tests bottle neck shrink to 6-7 minutes ✌️ |
just noticed the comment, I think we can enable presto / hive tests later on for the cases that are critical or if we will notice breakages. |
…e#17782) * chore: make presto hive tests to cover only chartData and sqljson * fix: single quote * fix: add eval for arguments * fix: add double quotes on args * chore: add @pytest.mark.sql_json_flow * fix: pre-commit
…e#17782) * chore: make presto hive tests to cover only chartData and sqljson * fix: single quote * fix: add eval for arguments * fix: add double quotes on args * chore: add @pytest.mark.sql_json_flow * fix: pre-commit
SUMMARY
there have been a few attempts already to avoid the flakiness of the presto hive tests
in addition, we know that the original intention of that workflow was to test out the engine part of presto and hive
therefore we can just limit the tests the chart/data and sql_json API's which are executing the data querying part of superset
#17772
resolves #17750
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION