-
Notifications
You must be signed in to change notification settings - Fork 40
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
Improve tests coverage for BigQuery
and Sharepoint
source classes
#716
Conversation
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.
It could be good to add docstrings to test function, so in future it could be easier and faster to understand what these functions are testing.
total_received = df["total"].values | ||
|
||
assert total_received == [4924235, 4818746, 4703680, 4280040] | ||
|
||
|
||
def test_wrong_query(BIGQ): |
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.
Please add docstring with raises section
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.
We are checking here if the exception is raised and not raising a new exception.
@@ -29,7 +29,13 @@ def FILE_NAME(sharepoint): | |||
os.remove(path) | |||
|
|||
|
|||
def test_credentials(): | |||
def test_credentials_not_found(): |
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.
Please add docstring with raises section
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.
We are checking here if the exception is raised and not raising a new exception.
tests/integration/test_bigquery.py
Outdated
LIMIT 4 | ||
""" | ||
with pytest.raises(DBDataAccessError): | ||
_ = BIGQ.query_to_df(fake_query) |
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 am not sure if there is need of assigning function to variable
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.
Changed.
Sharepoint(credentials=none_credentials) | ||
|
||
|
||
def test_get_connection_credentials(): |
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.
Please add docstring with raises section
tests/integration/test_sharepoint.py
Outdated
@@ -57,6 +63,11 @@ def test_sharepoint_to_df_task(): | |||
os.remove("Questionnaires.xlsx") | |||
|
|||
|
|||
def test_download_file(sharepoint): |
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.
Please add docstring with raises section
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.
We are checking here if the exception is raised and not raising a new exception.
BigQuery
and Sharepoint
source classes
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.
looks good, only one small suggestion
Summary
The task is to improve the coverage of source classes. For a lot of source classes the coverage is on low level.
Importance
To have as high coverage as possible.
Checklist
This PR:
CONTRIBUTING.md
CHANGELOG.md