Spec test fixes

Author: schmidt-sebastian

packages/firestore/test/unit/specs/describe_spec.ts

@@ -26,6 +26,8 @@ import { SpecStep } from './spec_test_runner';
 const EXCLUSIVE_TAG = 'exclusive';
 // Multi-client related tests (which imply persistence).
 const MULTI_CLIENT_TAG = 'multi-client';
+// Persistence-related tests.
+const PERSISTENCE_TAG = 'persistence';
 // Explicit per-platform disable flags.
 const NO_WEB_TAG = 'no-web';
 const NO_ANDROID_TAG = 'no-android';
@@ -36,6 +38,7 @@ const KNOWN_TAGS = [
   BENCHMARK_TAG,
   EXCLUSIVE_TAG,
   MULTI_CLIENT_TAG,
+  PERSISTENCE_TAG,
   NO_WEB_TAG,
   NO_ANDROID_TAG,
   NO_IOS_TAG,

packages/firestore/test/unit/specs/persistence_spec.test.ts

@@ -21,7 +21,7 @@ import { describeSpec, specTest } from './describe_spec';
 import { client, spec } from './spec_builder';
 import { TimerId } from '../../../src/util/async_queue';
 
-describeSpec('Persistence:', [], () => {
+describeSpec('Persistence:', ['persistence'], () => {
   specTest('Local mutations are persisted and re-sent', [], () => {
     return spec()
       .userSets('collection/key1', { foo: 'bar' })

packages/firestore/test/unit/specs/spec_builder.ts

@@ -46,7 +46,9 @@ import {
   SpecQueryFilter,
   SpecQueryOrderBy,
   SpecStep,
-  SpecWatchFilter
+  SpecWatchFilter,
+  SpecWriteAck,
+  SpecWriteFailure
 } from './spec_test_runner';
 import { TimerId } from '../../../src/util/async_queue';
 
@@ -464,7 +466,7 @@ export class SpecBuilder {
   /**
    * Acks a write with a version and optional additional options.
    *
-   * expectUserCallback defaults to true if options are omitted.
+   * expectUserCallback defaults to true if omitted.
    */
   writeAcks(
     doc: string,
@@ -474,11 +476,13 @@ export class SpecBuilder {
     this.nextStep();
     options = options || {};
 
-    this.currentStep = {
-      writeAck: { version, keepInQueue: !!options.keepInQueue }
-    };
+    const writeAck: SpecWriteAck = { version };
+    if (options.keepInQueue) {
+      writeAck.keepInQueue = true;
+    }
+    this.currentStep = { writeAck };
 
-    if (options.expectUserCallback) {
+    if (options.expectUserCallback !== false) {
       return this.expectUserCallbacks({ acknowledged: [doc] });
     } else {
       return this;
@@ -488,7 +492,7 @@ export class SpecBuilder {
   /**
    * Fails a write with an error and optional additional options.
    *
-   * expectUserCallback defaults to true if options are omitted.
+   * expectUserCallback defaults to true if omitted.
    */
   failWrite(
     doc: string,
@@ -506,9 +510,13 @@ export class SpecBuilder {
         ? options.keepInQueue
         : !isPermanentFailure;
 
-    this.currentStep = { failWrite: { error, keepInQueue } };
+    const failWrite: SpecWriteFailure = { error };
+    if (keepInQueue) {
+      failWrite.keepInQueue = true;
+    }
+    this.currentStep = { failWrite };
 
-    if (options.expectUserCallback) {
+    if (options.expectUserCallback !== false) {
       return this.expectUserCallbacks({ rejected: [doc] });
     } else {
       return this;

packages/firestore/test/unit/specs/spec_test_runner.ts

@@ -947,14 +947,18 @@ abstract class TestRunner {
       if ('isPrimary' in expectation) {
         expect(this.isPrimaryClient).to.eq(expectation.isPrimary!, 'isPrimary');
       }
-      if ('userCallbacks' in expectation) {
-        expect(this.acknowledgedDocs).to.have.members(
-          expectation.userCallbacks.acknowledgedDocs
-        );
-        expect(this.rejectedDocs).to.have.members(
-          expectation.userCallbacks.rejectedDocs
-        );
-      }
+    }
+
+    if (expectation && expectation.userCallbacks) {
+      expect(this.acknowledgedDocs).to.have.members(
+        expectation.userCallbacks.acknowledgedDocs
+      );
+      expect(this.rejectedDocs).to.have.members(
+        expectation.userCallbacks.rejectedDocs
+      );
+    } else {
+      expect(this.acknowledgedDocs).to.be.empty;
+      expect(this.rejectedDocs).to.be.empty;
     }
 
     if (this.numClients === 1) {
@@ -1393,9 +1397,11 @@ export type SpecWriteAck = {
    * Whether we should keep the write in our internal queue. This should only
    * be set to 'true' if the client ignores the write (e.g. a secondary client
    * which ignores write acknowledgments).
+   *
+   * Defaults to false.
    */
   // PORTING NOTE: Multi-Tab only.
-  keepInQueue: boolean;
+  keepInQueue?: boolean;
 };
 
 export type SpecWriteFailure = {
@@ -1405,8 +1411,10 @@ export type SpecWriteFailure = {
    * Whether we should keep the write in our internal queue. This should be set
    * to 'true' for transient errors or if the client ignores the failure
    * (e.g. a secondary client which ignores write rejections).
+   *
+   * Defaults to false.
    */
-  keepInQueue: boolean;
+  keepInQueue?: boolean;
 };
 
 export interface SpecWatchEntity {

packages/firestore/test/unit/specs/write_spec.test.ts

@@ -645,7 +645,7 @@ describeSpec('Writes:', [], () => {
         expectRequestCount({ handshakes: 2, writes: 2, closes: 1 })
       )
       .expectNumOutstandingWrites(1)
-      .writeAcks('collection/key', 1, { expectUserCallback: false })
+      .writeAcks('collection/key', 1)
       .expectNumOutstandingWrites(0);
   });