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.

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

Jump to bottom

test(db_api): increase coverage of db_api #231

Merged
merged 4 commits into from Mar 10, 2021
Merged

test(db_api): increase coverage of db_api #231

merged 4 commits into from Mar 10, 2021

Conversation

HemangChothani
Copy link
Contributor

No description provided.

@HemangChothani HemangChothani requested a review from a team as a code owner February 10, 2021 12:01
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Feb 10, 2021
@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/python-spanner API. label Feb 10, 2021
@larkee larkee changed the title pref: increase coverage of db_api test(db_api): increase coverage of db_api Feb 15, 2021
@larkee
Copy link
Contributor

larkee commented Feb 15, 2021

From the coverage in the Kokoro build:

google/cloud/spanner_dbapi/connection.py      152      2     48      1    98%   305->306, 306-307
google/cloud/spanner_dbapi/cursor.py          188      6     54      4    96%   260->262, 278->280, 281-283, 307->309, 310->312, 312-314

Connection.run_statement() is missing a homogenous insert test.
Cursor.fetchone(), Cursor.fetchall() and Cursor.fetchmany() are missing autocommit=True tests.
Cursor.fetchall() and Cursor.fetchmany() are missing Aborted error tests.
Adding these ~6 tests should bring the coverage to 100%

@larkee larkee added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 15, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 15, 2021
@larkee
Copy link
Contributor

larkee commented Feb 15, 2021

Disregard the Aborted error and homogenous insert test suggestions. I can see they're added in #228 and #233 respectively.

larkee
larkee reviewed Feb 22, 2021
View reviewed changes
Copy link
Contributor

@larkee larkee left a comment

Choose a reason for hiding this comment

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

Please rebase this PR

larkee
larkee reviewed Mar 10, 2021
View reviewed changes
Copy link
Contributor

@larkee larkee left a comment

Choose a reason for hiding this comment

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

LGTM, just need the missing test to be added back in 👍

tests/unit/spanner_dbapi/test_connection.py
@@ -383,14 +383,14 @@ def test_run_statement_dont_remember_retried_statements(self):

self.assertEqual(len(connection._statements), 0)

def test_run_statement_w_heterogenous_insert_statements(self):
"""Check that Connection executed heterogenous insert statements."""
def test_run_statement_w_homogeneous_insert_statements(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like you overwrote test_run_statement_w_heterogenous_insert_statements during the conflict resolution. Please add it back in 👍

@larkee larkee merged commit 489ac0a into googleapis:master Mar 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@larkee larkee larkee approved these changes

Assignees
No one assigned
Labels
api: spanner Issues related to the googleapis/python-spanner API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@HemangChothani @larkee @yoshi-kokoro