Skip to content

tests: add coverage for a successful read-only transaction with readTime#870

Merged
cherylEnkidu merged 1 commit intomainfrom
cheryllin/readtimeTest
Feb 14, 2022
Merged

tests: add coverage for a successful read-only transaction with readTime#870
cherylEnkidu merged 1 commit intomainfrom
cheryllin/readtimeTest

Conversation

@cherylEnkidu
Copy link
Copy Markdown
Contributor

Add a test to cover the a successful transaction with readTime set before the last time changing the document.

@cherylEnkidu cherylEnkidu added the api: firestore Issues related to the googleapis/java-firestore API. label Feb 11, 2022
@cherylEnkidu cherylEnkidu requested a review from a team as a code owner February 11, 2022 21:01
@cherylEnkidu cherylEnkidu self-assigned this Feb 11, 2022
@cherylEnkidu cherylEnkidu force-pushed the cheryllin/readtimeTest branch 2 times, most recently from d01d1ed to d6dde51 Compare February 11, 2022 21:15
Copy link
Copy Markdown
Contributor

@schmidt-sebastian schmidt-sebastian left a comment

Choose a reason for hiding this comment

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

This is great! Thank you for doing this.

One thing to read up on is https://www.conventionalcommits.org/en/v1.0.0/

The TL/DR is that the repo tooling using the PR title to generate releases. This means that your PR has to follow the prescribed pattern. You could use a name as such:

tests: add coverage for a successful read-only transaction with readTime

Note that this only applies to Java and Node.

final DocumentSnapshot snapshot =
transaction.get(documentReference).get(5, TimeUnit.SECONDS);
ref.compareAndSet(null, snapshot);
return null;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You could just return the value of the counter here. Then this transaction returns an ApiFuture<Integer> and you don't have to copy the document state.

documentReference.set(Collections.singletonMap("counter", 1)).get().getUpdateTime();
documentReference.set(Collections.singletonMap("counter", 2)).get();

final TransactionOptions option =
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

options?

@cherylEnkidu cherylEnkidu changed the title Add test to cover a successful read-only transaction with readTime set tests: add coverage for a successful read-only transaction with readTime Feb 11, 2022
@cherylEnkidu cherylEnkidu force-pushed the cheryllin/readtimeTest branch from d6dde51 to 331315a Compare February 11, 2022 22:34
@cherylEnkidu cherylEnkidu merged commit a19e804 into main Feb 14, 2022
@cherylEnkidu cherylEnkidu deleted the cheryllin/readtimeTest branch February 14, 2022 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: firestore Issues related to the googleapis/java-firestore API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants