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.

Sign up for GitHub

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

chore(sql_lab): Added Unit Test for stop query exception #17464

Merged
merged 2 commits into from
Nov 19, 2021
+22 −0
Merged

chore(sql_lab): Added Unit Test for stop query exception #17464

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
402a6c7
added unit test
AAfghahi Nov 17, 2021
8d98d14
feedback implemented
AAfghahi Nov 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions tests/integration_tests/core_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1548,6 +1548,30 @@ def test_dashboard_injected_exceptions(self, mock_db_connection_mutator):
data = self.get_resp(url)
self.assertIn("Error message", data)

@mock.patch("superset.sql_lab.cancel_query")
def test_stop_query_no_cancel_query(self, mock_sql_lab_cancel_query):
"""
Handles stop query when the DB engine spec does not
have a cancel query method.
"""
form_data = {"client_id": "foo"}
query_mock = mock.Mock()
query_mock.sql = "SELECT *"
query_mock.database = 1
query_mock.schema = "superset"
query_mock.client_id = "foo"
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to mock these attributes?

Copy link
Member Author

Choose a reason for hiding this comment

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

No I deleted them.

query_mock.status = QueryStatus.RUNNING
self.login(username="admin")

with mock.patch("superset.views.core.db") as mock_superset_db:
Copy link
Member

Choose a reason for hiding this comment

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

Why not also patch the method with a decorator, like you did for cancel_query?

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

mock_superset_db.session.query().filter_by().one().return_value = query_mock
Copy link
Member

Choose a reason for hiding this comment

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

Oh, I didn't know you could use () instead of return_value. Nice!

mock_sql_lab_cancel_query.return_value = False
rv = self.client.post(
"/superset/stop_query/", data={"form_data": json.dumps(form_data)},
)

self.assertEqual(rv.status_code, 422)
Copy link
Member

Choose a reason for hiding this comment

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

Since me started using pylint naked asserts are preferred:

Suggested change
self.assertEqual(rv.status_code, 422)
assert rv.status_code == 422

Copy link
Member Author

Choose a reason for hiding this comment

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

changed! I forgot about this.



if __name__ == "__main__":
unittest.main()