FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: fix insert test by HemangChothani · Pull Request #62 · googleapis/python-spanner-sqlalchemy · GitHub

This repository was archived by the owner on May 14, 2026. It is now read-only.
/ python-spanner-sqlalchemy Public archive

test: fix insert test - #62

Merged
larkee merged 6 commits into
mainfrom
autoincrement_on_insert
May 12, 2021
ConversationCommits (6)ChecksFiles changed
Merged

test: fix insert test#62
larkee merged 6 commits into
mainfrom
autoincrement_on_insert

Conversation

HemangChothani commented Apr 27, 2021

Copy link
Copy Markdown
Contributor

No description provided.

HemangChothani requested a review from AVaksman April 27, 2021 13:12
google-cla Bot added the cla: yes This human has signed the Contributor License Agreement. label Apr 27, 2021
IlyaFaer reviewed Apr 28, 2021
View reviewed changes
Comment thread test/test_suite.py Outdated

@pytest.mark.skip("Spanner doesn't support auto increment")
def test_autoclose_on_insert(self):
pass

IlyaFaer Apr 28, 2021

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This test is flaky, it sometime passes. That means it can be fixed instead of skipping.

HemangChothani Apr 28, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I have tried multiple times, but it failed every time, the test is related to auto increment which spanner doesn't support. To pass this need to fill id manually which break the meaning of test.

HemangChothani Apr 28, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I don't think its flaky as in create table sql define NOT NULL to id column. CREATE TABLE autoinc_pk (id INT64 NOT NULL, data STRING(50)\n) PRIMARY KEY (id). Hence override the test and also enable and override the other related advance test.

HemangChothani changed the title test: skip autoincrement test test: fix and enable insert test Apr 28, 2021
HemangChothani changed the title test: fix and enable insert test test: fix insert test Apr 28, 2021
HemangChothani requested a review from larkee April 28, 2021 15:51
skuruppu requested a review from vi3k6i5 May 6, 2021 23:03
HemangChothani mentioned this pull request May 11, 2021
Merged
larkee merged commit aa67b08 into main May 12, 2021
IlyaFaer deleted the autoincrement_on_insert branch May 12, 2021 09:50
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

vi3k6i5 Awaiting requested review from vi3k6i5
3 more reviewers
IlyaFaer IlyaFaer left review comments
AVaksman AVaksman approved these changes
larkee larkee approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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.

4 participants

Back | FazBrowse Home | New Git URL