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

[FIrestore Samples] Update tests to be self-contained and verified for correctness. #1788

Closed
frankyn opened this issue Oct 24, 2018 · 8 comments · Fixed by #11533
Closed

[FIrestore Samples] Update tests to be self-contained and verified for correctness. #1788

frankyn opened this issue Oct 24, 2018 · 8 comments · Fixed by #11533
Assignees
@irudykss @triplequark
Labels
api: firestore Issues related to the Firestore API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: cleanup An internal cleanup or hygiene concern.

Comments

@frankyn
Copy link
Member

frankyn commented Oct 24, 2018

In which file did you encounter the issue?

https://github.com/GoogleCloudPlatform/python-docs-samples/blob/master/firestore/cloud-client/snippets_test.py

Describe the issue

Tests aren't self contained and also not being verified for correctness. At the moment, tests only run the samples and verify that they execute without failure. Although, tests do provide some level of support using this method, they aren't verifying state correctness of Firestore post after testing a given sample.
@frankyn frankyn mentioned this issue Oct 24, 2018
Merged
@JustinBeckwith JustinBeckwith added triage me I really want to be triaged. 🚨 This issue needs some love. labels Oct 3, 2019
@BenWhitehead BenWhitehead added priority: p2 Moderately-important priority. Fix may not be included in next release. and removed 🚨 This issue needs some love. triage me I really want to be triaged. labels Oct 7, 2019
@gguuss
Copy link
Contributor

gguuss commented Oct 10, 2019

This may be fixed in the updated tests from @BenWhitehead.

@BenWhitehead
Copy link
Contributor

There are a few older tests that need to have assertions added to them.

@leahecole leahecole added the type: cleanup An internal cleanup or hygiene concern. label Nov 15, 2019
@JustinBeckwith JustinBeckwith added the api: firestore Issues related to the Firestore API. label Nov 16, 2019
@crwilcox crwilcox mentioned this issue Dec 31, 2019
Merged
@crwilcox crwilcox self-assigned this Dec 31, 2019
@crwilcox
Copy link
Contributor

@BenWhitehead do we have the individual tests we should fix?

@BenWhitehead
Copy link
Contributor

@crwilcox I think with your updated bootstrapping for the tests that should hopefully transparently take care of this.

@frankyn Do you have specific tests that we should double check to make sure the fix from @crwilcox addresses things?

@andrewferlitsch
Copy link
Contributor

@crwilcox PTAL

@frankyn
Copy link
Member Author

frankyn commented Mar 17, 2020

Apologies, I missed this in email.

The tests are not validating correctness and only making sure samples execute without failures.

Here's an example of a test being verified: https://github.com/GoogleCloudPlatform/python-docs-samples/blob/master/firestore/cloud-client/snippets_test.py#L74-L80

Here's an example of test not being verified: https://github.com/GoogleCloudPlatform/python-docs-samples/blob/master/firestore/cloud-client/snippets_test.py#L70-L71

@leahecole
Copy link
Collaborator

I've been in touch with @BenWhitehead recently, and it is still in the pipeline.

@BenWhitehead BenWhitehead removed their assignment Feb 10, 2021
@crwilcox crwilcox assigned dmahugh and unassigned crwilcox and laylasells Nov 1, 2021
@meredithslota
Copy link
Contributor

A partial fix was implemented in #2667 but work remains (see last comment from @frankyn).

@meredithslota meredithslota assigned triplequark and unassigned dmahugh Mar 7, 2023
@irudykss irudykss self-assigned this Apr 15, 2024
@irudykss irudykss mentioned this issue Apr 17, 2024
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@irudykss irudykss

@triplequark triplequark

Labels
api: firestore Issues related to the Firestore API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: cleanup An internal cleanup or hygiene concern.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[issue-1788] added firedbase tests GoogleCloudPlatform/python-docs-samples