Firestore spec tests: Bundle the readTime and resumeToken into a data structure (#7021)

dconeybe · NhienLam · commit fb4711e6d4bf · 2023-02-14T22:44:09.000Z
diff --git a/packages/firestore/test/unit/specs/existence_filter_spec.test.ts b/packages/firestore/test/unit/specs/existence_filter_spec.test.ts
@@ -31,7 +31,7 @@ describeSpec('Existence Filters:', [], () => {
       .userListens(query1)
       .watchAcksFull(query1, 1000, doc1)
       .expectEvents(query1, { added: [doc1] })
-      .watchFilters([query1], doc1.key)
+      .watchFilters([query1], [doc1.key])
       .watchSnapshots(2000);
   });
 
@@ -45,7 +45,7 @@ describeSpec('Existence Filters:', [], () => {
       .watchSnapshots(2000)
       .expectEvents(query1, {})
       .watchSends({ affects: [query1] }, doc1)
-      .watchFilters([query1], doc1.key)
+      .watchFilters([query1], [doc1.key])
       .watchSnapshots(2000)
       .expectEvents(query1, { added: [doc1] });
   });
@@ -59,7 +59,7 @@ describeSpec('Existence Filters:', [], () => {
       .watchCurrents(query1, 'resume-token-1000')
       .watchSnapshots(2000)
       .expectEvents(query1, {})
-      .watchFilters([query1], doc1.key)
+      .watchFilters([query1], [doc1.key])
       .watchSnapshots(2000)
       .expectEvents(query1, { fromCache: true });
   });
@@ -96,7 +96,7 @@ describeSpec('Existence Filters:', [], () => {
         .userListens(query1)
         .watchAcksFull(query1, 1000, doc1, doc2)
         .expectEvents(query1, { added: [doc1, doc2] })
-        .watchFilters([query1], doc1.key) // in the next sync doc2 was deleted
+        .watchFilters([query1], [doc1.key]) // in the next sync doc2 was deleted
         .watchSnapshots(2000)
         // query is now marked as "inconsistent" because of filter mismatch
         .expectEvents(query1, { fromCache: true })
@@ -130,7 +130,7 @@ describeSpec('Existence Filters:', [], () => {
           resumeToken: 'existence-filter-resume-token'
         })
         .watchAcks(query1)
-        .watchFilters([query1], doc1.key) // in the next sync doc2 was deleted
+        .watchFilters([query1], [doc1.key]) // in the next sync doc2 was deleted
         .watchSnapshots(2000)
         // query is now marked as "inconsistent" because of filter mismatch
         .expectEvents(query1, { fromCache: true })
@@ -159,7 +159,7 @@ describeSpec('Existence Filters:', [], () => {
         // Send a mismatching existence filter with two documents, but don't
         // send a new global snapshot. We should not see an event until we
         // receive the snapshot.
-        .watchFilters([query1], doc1.key, doc2.key)
+        .watchFilters([query1], [doc1.key, doc2.key])
         .watchSends({ affects: [query1] }, doc3)
         .watchSnapshots(2000)
         // The query result includes doc3, but is marked as "inconsistent"
@@ -193,7 +193,7 @@ describeSpec('Existence Filters:', [], () => {
         .userListens(query1)
         .watchAcksFull(query1, 1000, doc1, doc2)
         .expectEvents(query1, { added: [doc1, doc2] })
-        .watchFilters([query1], doc1.key) // in the next sync doc2 was deleted
+        .watchFilters([query1], [doc1.key]) // in the next sync doc2 was deleted
         .watchSnapshots(2000)
         // query is now marked as "inconsistent" because of filter mismatch
         .expectEvents(query1, { fromCache: true })
@@ -229,7 +229,7 @@ describeSpec('Existence Filters:', [], () => {
           .userListens(query1)
           .watchAcksFull(query1, 1000, doc1, doc2)
           .expectEvents(query1, { added: [doc1, doc2] })
-          .watchFilters([query1], doc1.key) // doc2 was deleted
+          .watchFilters([query1], [doc1.key]) // doc2 was deleted
           .watchSnapshots(2000)
           .expectEvents(query1, { fromCache: true })
           // The SDK is unable to re-run the query, and does not remove doc2
diff --git a/packages/firestore/test/unit/specs/limbo_spec.test.ts b/packages/firestore/test/unit/specs/limbo_spec.test.ts
@@ -880,7 +880,7 @@ describeSpec('Limbo Documents:', [], () => {
           // documents that changed since the resume token. This will cause it
           // to just send the docBs with an existence filter with a count of 3.
           .watchSends({ affects: [query1] }, docB1, docB2, docB3)
-          .watchFilters([query1], docB1.key, docB2.key, docB3.key)
+          .watchFilters([query1], [docB1.key, docB2.key, docB3.key])
           .watchSnapshots(1001)
           .expectEvents(query1, {
             added: [docB1, docB2, docB3],
diff --git a/packages/firestore/test/unit/specs/limit_spec.test.ts b/packages/firestore/test/unit/specs/limit_spec.test.ts
@@ -341,7 +341,7 @@ describeSpec('Limits:', [], () => {
           // we receive an existence filter, which indicates that our view is
           // out of sync.
           .watchSends({ affects: [limitQuery] }, secondDocument)
-          .watchFilters([limitQuery], secondDocument.key)
+          .watchFilters([limitQuery], [secondDocument.key])
           .watchSnapshots(1004)
           .expectActiveTargets({ query: limitQuery, resumeToken: '' })
           .watchRemoves(limitQuery)
diff --git a/packages/firestore/test/unit/specs/spec_builder.ts b/packages/firestore/test/unit/specs/spec_builder.ts
@@ -81,6 +81,11 @@ export interface ActiveTargetMap {
   [targetId: string]: ActiveTargetSpec;
 }
 
+export interface ResumeSpec {
+  resumeToken?: string;
+  readTime?: TestSnapshotVersion;
+}
+
 /**
  * Tracks the expected memory state of a client (e.g. the expected active watch
  * targets based on userListens(), userUnlistens(), and watchRemoves()
@@ -255,10 +260,7 @@ export class SpecBuilder {
     return this;
   }
 
-  userListens(
-    query: Query,
-    resume?: { resumeToken?: string; readTime?: TestSnapshotVersion }
-  ): this {
+  userListens(query: Query, resume?: ResumeSpec): this {
     this.nextStep();
 
     const target = queryToTarget(query);
@@ -277,12 +279,7 @@ export class SpecBuilder {
       }
 
       this.queryMapping.set(target, targetId);
-      this.addQueryToActiveTargets(
-        targetId,
-        query,
-        resume?.resumeToken,
-        resume?.readTime
-      );
+      this.addQueryToActiveTargets(targetId, query, resume);
       this.currentStep = {
         userListen: { targetId, query: SpecBuilder.queryToSpec(query) },
         expectedState: { activeTargets: { ...this.activeTargets } }
@@ -302,7 +299,7 @@ export class SpecBuilder {
       throw new Error("Can't restore an unknown query: " + query);
     }
 
-    this.addQueryToActiveTargets(targetId!, query, resumeToken);
+    this.addQueryToActiveTargets(targetId!, query, { resumeToken });
 
     const currentStep = this.currentStep!;
     currentStep.expectedState = currentStep.expectedState || {};
@@ -536,12 +533,10 @@ export class SpecBuilder {
     const currentStep = this.currentStep!;
     this.clientState.activeTargets = {};
     targets.forEach(({ query, resumeToken, readTime }) => {
-      this.addQueryToActiveTargets(
-        this.getTargetId(query),
-        query,
+      this.addQueryToActiveTargets(this.getTargetId(query), query, {
         resumeToken,
         readTime
-      );
+      });
     });
     currentStep.expectedState = currentStep.expectedState || {};
     currentStep.expectedState.activeTargets = { ...this.activeTargets };
@@ -572,7 +567,7 @@ export class SpecBuilder {
       this.addQueryToActiveTargets(
         this.limboMapping[path],
         newQueryForPath(key.path),
-        ''
+        { resumeToken: '' }
       );
     });
 
@@ -769,18 +764,15 @@ export class SpecBuilder {
     return this;
   }
 
-  watchFilters(queries: Query[], ...docs: DocumentKey[]): this {
+  watchFilters(queries: Query[], docs: DocumentKey[] = []): this {
     this.nextStep();
     const targetIds = queries.map(query => {
       return this.getTargetId(query);
     });
     const keys = docs.map(key => {
       return key.path.canonicalString();
     });
-    const filter: SpecWatchFilter = [targetIds] as SpecWatchFilter;
-    for (const key of keys) {
-      filter.push(key);
-    }
+    const filter: SpecWatchFilter = { targetIds, keys };
     this.currentStep = {
       watchFilter: filter
     };
@@ -910,22 +902,14 @@ export class SpecBuilder {
   }
 
   /** Registers a query that is active in another tab. */
-  expectListen(
-    query: Query,
-    resume?: { resumeToken?: string; readTime?: TestSnapshotVersion }
-  ): this {
+  expectListen(query: Query, resume?: ResumeSpec): this {
     this.assertStep('Expectations require previous step');
 
     const target = queryToTarget(query);
     const targetId = this.queryIdGenerator.cachedId(target);
     this.queryMapping.set(target, targetId);
 
-    this.addQueryToActiveTargets(
-      targetId,
-      query,
-      resume?.resumeToken,
-      resume?.readTime
-    );
+    this.addQueryToActiveTargets(targetId, query, resume);
 
     const currentStep = this.currentStep!;
     currentStep.expectedState = currentStep.expectedState || {};
@@ -1093,8 +1077,7 @@ export class SpecBuilder {
   private addQueryToActiveTargets(
     targetId: number,
     query: Query,
-    resumeToken?: string,
-    readTime?: TestSnapshotVersion
+    resume?: ResumeSpec
   ): void {
     if (this.activeTargets[targetId]) {
       const activeQueries = this.activeTargets[targetId].queries;
@@ -1106,21 +1089,21 @@ export class SpecBuilder {
         // `query` is not added yet.
         this.activeTargets[targetId] = {
           queries: [SpecBuilder.queryToSpec(query), ...activeQueries],
-          resumeToken: resumeToken || '',
-          readTime
+          resumeToken: resume?.resumeToken || '',
+          readTime: resume?.readTime
         };
       } else {
         this.activeTargets[targetId] = {
           queries: activeQueries,
-          resumeToken: resumeToken || '',
-          readTime
+          resumeToken: resume?.resumeToken || '',
+          readTime: resume?.readTime
         };
       }
     } else {
       this.activeTargets[targetId] = {
         queries: [SpecBuilder.queryToSpec(query)],
-        resumeToken: resumeToken || '',
-        readTime
+        resumeToken: resume?.resumeToken || '',
+        readTime: resume?.readTime
       };
     }
   }
diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts
@@ -694,12 +694,11 @@ abstract class TestRunner {
   }
 
   private doWatchFilter(watchFilter: SpecWatchFilter): Promise<void> {
-    const targetIds: TargetId[] = watchFilter[0];
+    const { targetIds, keys } = watchFilter;
     debugAssert(
       targetIds.length === 1,
       'ExistenceFilters currently support exactly one target only.'
     );
-    const keys = watchFilter.slice(1);
     const filter = new ExistenceFilter(keys.length);
     const change = new ExistenceFilterChange(targetIds[0], filter);
     return this.doWatchEvent(change);
@@ -1589,14 +1588,11 @@ export interface SpecClientState {
 }
 
 /**
- * [[<target-id>, ...], <key>, ...]
- * Note that the last parameter is really of type ...string (spread operator)
  * The filter is based of a list of keys to match in the existence filter
  */
-export interface SpecWatchFilter
-  extends Array<TargetId[] | string | undefined> {
-  '0': TargetId[];
-  '1': string | undefined;
+export interface SpecWatchFilter {
+  targetIds: TargetId[];
+  keys: string[];
 }
 
 export type SpecLimitType = 'LimitToFirst' | 'LimitToLast';
