Skip to content

Conversation

@HemangChothani
Copy link
Contributor

No description provided.

@HemangChothani HemangChothani requested a review from AVaksman April 27, 2021 13:12
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Apr 27, 2021

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

Choose a reason for hiding this comment

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

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

Безымянный

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 HemangChothani changed the title test: skip autoincrement test test: fix and enable insert test Apr 28, 2021
@HemangChothani HemangChothani changed the title test: fix and enable insert test test: fix insert test Apr 28, 2021
@HemangChothani HemangChothani requested a review from larkee April 28, 2021 15:51
@larkee larkee merged commit aa67b08 into main May 12, 2021
@IlyaFaer IlyaFaer deleted the autoincrement_on_insert branch May 12, 2021 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants