Skip to content

Simplify IndexFree tests #790

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

Merged
merged 2 commits into from
Sep 12, 2019
Merged

Simplify IndexFree tests #790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f61b614
Simplify test
schmidt-sebastian Sep 11, 2019
554d273
Run test with eager GC
schmidt-sebastian Sep 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 23 additions & 50 deletions firebase-firestore/src/test/java/com/google/firebase/firestore/local/LocalStoreTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1073,49 +1073,31 @@ public void testUsesTargetMappingToExecuteQueries() {

@Test
public void testLastLimboFreeSnapshotIsAdvancedDuringViewProcessing() {
assumeFalse(garbageCollectorIsEager());
assumeTrue(queryEngine instanceof IndexFreeQueryEngine);

// This test verifies that the `lastLimboFreeSnapshot` version for QueryData is advanced when
// we compute a limbo-free free view and that the mapping is persisted when we release a query.

writeMutation(setMutation("foo/ignored", map("matches", false)));
acknowledgeMutation(10);

Query query =
Query.atPath(ResourcePath.fromString("foo")).filter(filter("matches", "==", true));
Query query = Query.atPath(ResourcePath.fromString("foo"));
int targetId = allocateQuery(query);

// Mark the query as current.
applyRemoteEvent(
addedRemoteEvent(
asList(doc("foo/a", 10, map("matches", true)), doc("foo/b", 10, map("matches", true))),
asList(targetId),
emptyList()));
// Advance the query snapshot.
applyRemoteEvent(noChangeEvent(targetId, 10));

// At this point, we have not yet confirmed that the query is limbo free.
Assert.assertEquals(
SnapshotVersion.NONE, localStore.getQueryData(query).getLastLimboFreeSnapshotVersion());
QueryData cachedQueryData = localStore.getQueryData(query);
Assert.assertEquals(SnapshotVersion.NONE, cachedQueryData.getLastLimboFreeSnapshotVersion());

// Update the view, but don't mark the view synced.
Assert.assertEquals(
SnapshotVersion.NONE, localStore.getQueryData(query).getLastLimboFreeSnapshotVersion());
// Mark the view synced, which updates the last limbo free snapshot version.
udpateViews(targetId, /* fromCache=*/ false);

// The query is marked limbo-free only when we mark the view synced.
udpateViews(targetId, /* fromCache=*/ false);
Assert.assertNotEquals(
SnapshotVersion.NONE, localStore.getQueryData(query).getLastLimboFreeSnapshotVersion());
cachedQueryData = localStore.getQueryData(query);
Assert.assertEquals(version(10), cachedQueryData.getLastLimboFreeSnapshotVersion());

// The last limbo free snapshot version is persisted even if we release the query.
releaseQuery(query);
allocateQuery(query);

// Verify that we only read the two documents that match the query.
executeQuery(query);
assertRemoteDocumentsRead(2);
assertQueryReturned("foo/a", "foo/b");
if (!garbageCollectorIsEager()) {
cachedQueryData = localStore.getQueryData(query);
Assert.assertEquals(version(10), cachedQueryData.getLastLimboFreeSnapshotVersion());
}
}

@Test
Expand All @@ -1130,28 +1112,26 @@ public void testQueriesIncludeLocallyModifiedDocuments() {

applyRemoteEvent(
addedRemoteEvent(
asList(doc("foo/a", 10, map("matches", true)), doc("foo/b", 10, map("matches", true))),
asList(targetId),
emptyList()));
asList(doc("foo/a", 10, map("matches", true))), asList(targetId), emptyList()));
applyRemoteEvent(noChangeEvent(targetId, 10));
udpateViews(targetId, /* fromCache= */ false);

// Execute the query based on the RemoteEvent.
executeQuery(query);
assertQueryReturned("foo/a", "foo/b");
assertQueryReturned("foo/a");

// Write a document.
writeMutation(setMutation("foo/c", map("matches", true)));
writeMutation(setMutation("foo/b", map("matches", true)));

// Execute the query and make sure that the pending mutation is included in the result.
executeQuery(query);
assertQueryReturned("foo/a", "foo/b", "foo/c");
assertQueryReturned("foo/a", "foo/b");

acknowledgeMutation(11);

// Execute the query and make sure that the acknowledged mutation is included in the result.
executeQuery(query);
assertQueryReturned("foo/a", "foo/b", "foo/c");
assertQueryReturned("foo/a", "foo/b");
}

@Test
Expand All @@ -1167,23 +1147,17 @@ public void testQueriesIncludeDocumentsFromOtherQueries() {

applyRemoteEvent(
addedRemoteEvent(
asList(doc("foo/a", 10, map("matches", true)), doc("foo/b", 10, map("matches", true))),
asList(targetId),
emptyList()));
asList(doc("foo/a", 10, map("matches", true))), asList(targetId), emptyList()));
applyRemoteEvent(noChangeEvent(targetId, 10));
udpateViews(targetId, /* fromCache=*/ false);
releaseQuery(filteredQuery);

// Start another query and add more matching documents to the collection.
Query fullQuery =
Query.atPath(ResourcePath.fromString("foo")).filter(filter("matches", "==", true));
targetId = allocateQuery(filteredQuery);
Query fullQuery = Query.atPath(ResourcePath.fromString("foo"));
targetId = allocateQuery(fullQuery);
applyRemoteEvent(
addedRemoteEvent(
asList(
doc("foo/a", 10, map("matches", true)),
doc("foo/b", 10, map("matches", true)),
doc("foo/c", 20, map("matches", true))),
asList(doc("foo/a", 10, map("matches", true)), doc("foo/b", 20, map("matches", true))),
asList(targetId),
emptyList()));
releaseQuery(fullQuery);
Expand All @@ -1192,7 +1166,7 @@ public void testQueriesIncludeDocumentsFromOtherQueries() {
// matches are included in the result set.
allocateQuery(filteredQuery);
executeQuery(filteredQuery);
assertQueryReturned("foo/a", "foo/b", "foo/c");
assertQueryReturned("foo/a", "foo/b");
}

@Test
Expand All @@ -1217,9 +1191,8 @@ public void testQueriesFilterDocumentsThatNoLongerMatch() {
releaseQuery(filteredQuery);

// Modify one of the documents to no longer match while the filtered query is inactive.
Query fullQuery =
Query.atPath(ResourcePath.fromString("foo")).filter(filter("matches", "==", true));
targetId = allocateQuery(filteredQuery);
Query fullQuery = Query.atPath(ResourcePath.fromString("foo"));
targetId = allocateQuery(fullQuery);
applyRemoteEvent(
addedRemoteEvent(
asList(doc("foo/a", 10, map("matches", true)), doc("foo/b", 20, map("matches", false))),
Expand Down
4 changes: 3 additions & 1 deletion firebase-firestore/src/testUtil/java/com/google/firebase/firestore/testutil/TestUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,13 +418,15 @@ public QueryData getQueryDataForTarget(int targetId) {
}
});

SnapshotVersion version = SnapshotVersion.NONE;

for (MaybeDocument doc : docs) {
DocumentChange change =
new DocumentChange(updatedInTargets, removedFromTargets, doc.getKey(), doc);
aggregator.handleDocumentChange(change);
version = doc.getVersion().compareTo(version) > 0 ? doc.getVersion() : version;
Copy link
Contributor Author

@schmidt-sebastian schmidt-sebastian Sep 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure why this worked before but testQueriesIncludeDocumentsFromOtherQueries() needs to use 20 as the snapshot version (from the last doc) in order to use a higher read time for the newly added doc.

}

SnapshotVersion version = docs.get(0).getVersion();
return aggregator.createRemoteEvent(version);
}

Expand Down