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

tests: mitigate flaky snippets tests #432

Merged
merged 4 commits into from Jun 23, 2021
Merged

Conversation

plamut
Copy link
Contributor

@plamut plamut commented Jun 21, 2021

Fixes #430. (maybe)

I was not able to reproduce flakiness locally, thus this PR is a best guess.

The test failure suggests that more than three messages were received, even though we only publish 3 messages in the test. This could be due to message re-delivery or because a topic might be shared between tests.

This PR reduces the total wait time in the message callback. It used to be 5 + 5 seconds, which is just the default ACK deadline, thus reducing that to 8 seconds might help. In addition, the test now prints the log output if one of the assertions fail, which will help with debugging, should the flakiness continue.

We could also use auto retries by flaky as a last resort, but let's see if we can fix the root cause first.
(update: Yet I did this for the unrelated delete schema test, because it's quite often been flaky recently)

PR checklist:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

@plamut plamut requested a review from a team as a code owner June 21, 2021 12:38
@plamut plamut requested review from busunkim96 and removed request for a team June 21, 2021 12:38
@product-auto-label product-auto-label bot added the api: pubsub Issues related to the googleapis/python-pubsub API. label Jun 21, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jun 21, 2021
@plamut plamut changed the title fix(tests): flaky blocking shutdown snippet test tests: mitigate flaky snippets tests Jun 21, 2021
@plamut plamut added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 21, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 21, 2021
@plamut
Copy link
Contributor Author

plamut commented Jun 21, 2021

And now the docs, oh my. :)
(inventory downloads)

@plamut
Copy link
Contributor Author

plamut commented Jun 21, 2021

This test_delete_schema sample test is terribly flaky... too many of these 500 errors...

@plamut plamut added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 21, 2021
@yoshi-kokoro yoshi-kokoro removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jun 21, 2021
@plamut plamut added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 21, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 21, 2021
@plamut
Copy link
Contributor Author

plamut commented Jun 21, 2021

test_delete_schema seems to be by far the worst offender here. If we cannot get to the bottom of it, we might have to disable it, because it's been getting in the way too much.

@plamut plamut added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 21, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 21, 2021
# Yet, waiting on the stream shutdown should have completed *after*
# the processing of received messages has ended.
assert msg_done_lines[-1] < shutdown_done_waiting_lines[0]
except AssertionError: # pragma: NO COVER
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't know about "pragma: NO COVER". Nice.

Copy link
Contributor Author

@plamut plamut Jun 23, 2021

Choose a reason for hiding this comment

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

Yeah, but it just shouldn't be overused. :)
This is one of the cases where it's the right tool, though, it makes the coverage check happy, since a passing test does never hit the "unhappy" path.

Copy link
Contributor

Choose a reason for hiding this comment

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

Absolutely :)

@anguillanneuf anguillanneuf merged commit 05e3f2f into googleapis:master Jun 23, 2021
@plamut plamut deleted the iss-430 branch June 23, 2021 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsub Issues related to the googleapis/python-pubsub API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

samples.snippets.subscriber_test: test_receive_with_blocking_shutdown failed
3 participants