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.

Sign up for GitHub

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

fix: Improve Firestore tests by separating test runs, improving cleanup #2667

Merged
merged 25 commits into from
Jan 2, 2020
Merged

fix: Improve Firestore tests by separating test runs, improving cleanup #2667

merged 25 commits into from
Jan 2, 2020

Conversation

crwilcox
Copy link
Contributor

@crwilcox crwilcox commented Dec 30, 2019 •
edited
Loading

Fixes #2529, Possibly #1788

  • Add test to cleanup distributed counters
  • Separate Collections for each test run: Add a monkeypatched version of the Firestore Client so we can add a unique string to each collection name.
  • Use stream instead of get. Get is deprecated in the API
  • Moved all delete scenarios to end of testing

@crwilcox crwilcox requested a review from a team as a code owner December 30, 2019 17:35
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 30, 2019
@leahecole leahecole added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 30, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 30, 2019
@leahecole
Copy link
Collaborator

@crwilcox it looks like SF appears sometimes, but also sometimes there are additional items in the set. It seems like these tests may be getting in the way of each other - is there a way to make them independent?

leahecole
leahecole requested changes Dec 30, 2019
View reviewed changes
Copy link
Collaborator

@leahecole leahecole left a comment

Choose a reason for hiding this comment

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

Re earlier comment on PR, I think the tests are interfering with each other and if we can, we should separate them as much as possible.

@crwilcox
Copy link
Contributor Author

Currently looking at some of the other tests as well. They are also flaky. Will update this once I have hardened them all

@crwilcox
fix: add setup to snippets for tests
dc5881a
@crwilcox
fix: rename sydney_query to denver_query as it refers to denver
a113bee
@crwilcox
fix: Use stream instead of get in samples
ddc9768
@crwilcox
fix: stop deleting LA, delete DEN instead
fda54c8
@crwilcox
fix: move delete tests to end of file
a48030d
@crwilcox
fix: move BJ to top to work with snapshot cursors
5c5dbe3
@crwilcox
fix: Beijing -> BJ
102997c
@crwilcox
Merge branch 'master' into fix-2529
968ef21
@crwilcox
during delete collection delete all collections created for these tests
1acacb8
@crwilcox
Merge branch 'fix-2529' of github.com:GoogleCloudPlatform/python-docs…
f718d62 
…-samples into fix-2529
@crwilcox
fix: add cleanup test to distributed counters
0102d89
@crwilcox
wrap db.collections calls to use unique names
7c56405
@crwilcox
fix: remove unused import
cbf00bb
@crwilcox
fix: remove more complicated fix for time now that we have separate c…
75fa62e 
…ollections
@crwilcox
Copy link
Contributor Author

@frankyn #1788 may also be resolved by this work. I have made each run create new resources to there are no longer collisions between runs.

@crwilcox crwilcox changed the title fix: harden the wait for consistency for firestore multiple listen snippet fix: Improve Firestore tests by separating test runs, improving cleanup Dec 31, 2019
@crwilcox crwilcox requested review from BenWhitehead and frankyn and removed request for frankyn December 31, 2019 00:59
BenWhitehead
BenWhitehead approved these changes Jan 2, 2020
View reviewed changes
Copy link
Contributor

@BenWhitehead BenWhitehead left a comment

Choose a reason for hiding this comment

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

Thanks @crwilcox!

leahecole
leahecole approved these changes Jan 2, 2020
View reviewed changes
Copy link
Collaborator

@leahecole leahecole left a comment

Choose a reason for hiding this comment

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

I LOVE IT

@leahecole leahecole merged commit f94bb78 into master Jan 2, 2020
@leahecole leahecole deleted the fix-2529 branch January 2, 2020 17:31
@crwilcox
Copy link
Contributor Author

crwilcox commented Jan 2, 2020

🎉

@meredithslota meredithslota mentioned this pull request Mar 7, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BenWhitehead BenWhitehead BenWhitehead approved these changes

@leahecole leahecole leahecole approved these changes

Assignees

@leahecole leahecole

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.

firestore: listen_multiple test is flakey
6 participants
@crwilcox @leahecole @BenWhitehead @googlebot @kokoro-team @anguillanneuf