chore(sql_lab): Added Unit Test for stop query exception #17464
Conversation
Codecov Report
@@ Coverage Diff @@
## master #17464 +/- ##
==========================================
- Coverage 76.94% 76.74% -0.21%
==========================================
Files 1042 1042
Lines 56312 56312
Branches 7793 7793
==========================================
- Hits 43329 43216 -113
- Misses 12727 12840 +113
Partials 256 256
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| self.login(username="admin") | ||
|
|
||
| with mock.patch("superset.views.core.db") as mock_superset_db: | ||
| mock_superset_db.session.query().filter_by().one().return_value = query_mock |
There was a problem hiding this comment.
Oh, I didn't know you could use () instead of return_value. Nice!
| query_mock.sql = "SELECT *" | ||
| query_mock.database = 1 | ||
| query_mock.schema = "superset" | ||
| query_mock.client_id = "foo" |
There was a problem hiding this comment.
Do we need to mock these attributes?
| query_mock.status = QueryStatus.RUNNING | ||
| self.login(username="admin") | ||
|
|
||
| with mock.patch("superset.views.core.db") as mock_superset_db: |
There was a problem hiding this comment.
Why not also patch the method with a decorator, like you did for cancel_query?
There was a problem hiding this comment.
initially it was because it was giving me a 404 error and I saw this way of doing it repeated in various other integration tests. However, I played around with it, and if I @mock.patch(superset.views.core.db.session then I can use it as a decorator in the same way that I use cancel_query.
So i changed it to that, because ti does make it a lot more consistent and readable.
| "/superset/stop_query/", data={"form_data": json.dumps(form_data)}, | ||
| ) | ||
|
|
||
| self.assertEqual(rv.status_code, 422) |
There was a problem hiding this comment.
Since me started using pylint naked asserts are preferred:
| self.assertEqual(rv.status_code, 422) | |
| assert rv.status_code == 422 |
There was a problem hiding this comment.
changed! I forgot about this.
* added unit test * feedback implemented
mistercrunch
added
🏷️ bot
SUMMARY
This PR adds a unit test for the changes made here: #17292
It checks to make sure that we are getting a 422 error when an engine does not support cancel_query in sql_lab.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION