Skip to content
This repository was archived by the owner on Mar 31, 2026. It is now read-only.

test: make system test timeout configurable and default to 60 seconds#217

Merged
gcf-merge-on-green[bot] merged 5 commits intomasterfrom
timeout
Feb 2, 2021
Merged

test: make system test timeout configurable and default to 60 seconds#217
gcf-merge-on-green[bot] merged 5 commits intomasterfrom
timeout

Conversation

@arithmetic1728
Copy link
Copy Markdown
Contributor

@arithmetic1728 arithmetic1728 commented Jan 29, 2021 •
edited
Loading

b/173067462

It seems 30 seconds are too short for mtls test (which uses the system test, and runs on an internal platform). This makes the mtls test very flaky. This PR introduces a SPANNER_OPERATION_TIMEOUT_IN_SECONDS env var to make the timeout configurable.

The default value is now 60 seconds.

@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/python-spanner API. label Jan 29, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jan 29, 2021
@arithmetic1728 arithmetic1728 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 29, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 29, 2021
@arithmetic1728 arithmetic1728 marked this pull request as ready for review January 29, 2021 02:19
@arithmetic1728 arithmetic1728 requested a review from a team January 29, 2021 02:19
larkee
larkee reviewed Feb 2, 2021
View reviewed changes
Copy link
Copy Markdown
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 with a minor change to the flag name 👍

We have actually also encountered this flake on the repo but I haven't had time to investigate. In light of this, I would maybe suggest doubling the default value to 60.

Thank you very much for making this PR!

@arithmetic1728
Copy link
Copy Markdown
Contributor Author

arithmetic1728 commented Feb 2, 2021

LGTM with a minor change to the flag name 👍

We have actually also encountered this flake on the repo but I haven't had time to investigate. In light of this, I would maybe suggest doubling the default value to 60.

Thank you very much for making this PR!

I changed the default value to 60 seconds, and renamed the env var. Please take another look. Thank you!

larkee
larkee approved these changes Feb 2, 2021
View reviewed changes
Copy link
Copy Markdown
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. Please update the PR description to mention the default is now 60 seconds.

@arithmetic1728 arithmetic1728 changed the title test: make system test timeout configurable test: make system test timeout configurable and default to 60 seconds Feb 2, 2021
@larkee larkee added the automerge Merge the pull request once unit tests and other checks pass. label Feb 2, 2021
@gcf-merge-on-green gcf-merge-on-green bot merged commit d790cfb into master Feb 2, 2021
@gcf-merge-on-green gcf-merge-on-green bot deleted the timeout branch February 2, 2021 08:00
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Feb 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@thiagotnunes thiagotnunes Awaiting requested review from thiagotnunes

1 more reviewer

@larkee larkee larkee approved these changes

Reviewers whose approvals may not affect merge requirements

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.

3 participants

@arithmetic1728 @larkee @yoshi-kokoro