feat(merge-tree): explicitly test partial lengths

packages/dds/merge-tree/src/partialLengths.ts

@@ -216,7 +216,7 @@ export class PartialSequenceLengths {
         }
 
         if (PartialSequenceLengths.options.verify) {
-            combinedPartialLengths.verify();
+            verify(combinedPartialLengths);
         }
 
         return combinedPartialLengths;
@@ -272,7 +272,7 @@ export class PartialSequenceLengths {
             combinedPartialLengths.addClientSeqNumberFromPartial(seqPartials[i]);
         }
         if (PartialSequenceLengths.options.verify) {
-            combinedPartialLengths.verify();
+            verify(combinedPartialLengths);
         }
         return combinedPartialLengths;
     }
@@ -497,7 +497,7 @@ export class PartialSequenceLengths {
             this.zamboni(collabWindow);
         }
         if (PartialSequenceLengths.options.verify) {
-            this.verify();
+            verify(this);
         }
     }
 
@@ -634,83 +634,88 @@ export class PartialSequenceLengths {
             return -1;
         }
     }
+}
 
-    // Debug only
-    private verifyPartialLengths(partialLengths: PartialSequenceLength[], clientPartials: boolean) {
-        if (partialLengths.length === 0) { return 0; }
-
-        let lastSeqNum = 0;
-        let accumSegLen = 0;
-        let count = 0;
-
-        for (const partialLength of partialLengths) {
-            // Count total number of partial length
-            count++;
-
-            // Sequence number should be larger or equal to minseq
-            assert(this.minSeq <= partialLength.seq, 0x054 /* "Sequence number less than minSeq!" */);
-
-            // Sequence number should be sorted
-            assert(lastSeqNum < partialLength.seq, 0x055 /* "Sequence number is not sorted!" */);
-            lastSeqNum = partialLength.seq;
-
-            // Len is a accumulation of all the seglen adjustments
-            accumSegLen += partialLength.seglen;
-            if (accumSegLen !== partialLength.len) {
-                assert(false, 0x056 /* "Unexpected total for accumulation of all seglen adjustments!" */);
-            }
-
-            if (clientPartials) {
-                // Client partials used to track local edits so we can account for them some refSeq.
-                // But the information we keep track of are since minSeq, so we keep track of more history
-                // then needed, and some of them doesn't make sense to be used for length calculations
-                // e.g. if you have this sequence, where the minSeq is #5 because of other clients
-                //    seq 10: client 1: insert seg #1
-                //    seq 11: client 2: delete seg #2 refseq: 10
-                // minLength is 0, we would have keep a record of seglen: -1 for clientPartialLengths for client 2
-                // So if you ask for partial length for client 2 @ seq 5, we will have return -1.
-                // However, that combination is invalid, since we should never see any ops with refseq < 10 for
-                // client 2 after seq 11.
-            } else {
-                // Len adjustment should not make length negative
-                if (this.minLength + partialLength.len < 0) {
-                    assert(false, 0x057 /* "Negative length after length adjustment!" */);
-                }
+/* eslint-disable @typescript-eslint/dot-notation */
+function verifyPartialLengths(
+    partialSeqLengths: PartialSequenceLengths,
+    partialLengths: PartialSequenceLength[],
+    clientPartials: boolean,
+) {
+    if (partialLengths.length === 0) { return 0; }
+
+    let lastSeqNum = 0;
+    let accumSegLen = 0;
+    let count = 0;
+
+    for (const partialLength of partialLengths) {
+        // Count total number of partial length
+        count++;
+
+        // Sequence number should be larger or equal to minseq
+        assert(partialSeqLengths.minSeq <= partialLength.seq, 0x054 /* "Sequence number less than minSeq!" */);
+
+        // Sequence number should be sorted
+        assert(lastSeqNum < partialLength.seq, 0x055 /* "Sequence number is not sorted!" */);
+        lastSeqNum = partialLength.seq;
+
+        // Len is a accumulation of all the seglen adjustments
+        accumSegLen += partialLength.seglen;
+        if (accumSegLen !== partialLength.len) {
+            assert(false, 0x056 /* "Unexpected total for accumulation of all seglen adjustments!" */);
+        }
+
+        if (clientPartials) {
+            // Client partials used to track local edits so we can account for them some refSeq.
+            // But the information we keep track of are since minSeq, so we keep track of more history
+            // then needed, and some of them doesn't make sense to be used for length calculations
+            // e.g. if you have this sequence, where the minSeq is #5 because of other clients
+            //    seq 10: client 1: insert seg #1
+            //    seq 11: client 2: delete seg #2 refseq: 10
+            // minLength is 0, we would have keep a record of seglen: -1 for clientPartialLengths for client 2
+            // So if you ask for partial length for client 2 @ seq 5, we will have return -1.
+            // However, that combination is invalid, since we should never see any ops with refseq < 10 for
+            // client 2 after seq 11.
+        } else {
+            // Len adjustment should not make length negative
+            if (partialSeqLengths["minLength"] + partialLength.len < 0) {
+                assert(false, 0x057 /* "Negative length after length adjustment!" */);
             }
+        }
 
-            if (partialLength.overlapRemoveClients) {
-                // Only the flat partialLengths can have overlapRemoveClients, the per client view shouldn't
-                assert(!clientPartials, 0x058 /* "Both overlapRemoveClients and clientPartials are set!" */);
+        if (partialLength.overlapRemoveClients) {
+            // Only the flat partialLengths can have overlapRemoveClients, the per client view shouldn't
+            assert(!clientPartials, 0x058 /* "Both overlapRemoveClients and clientPartials are set!" */);
 
-                // Each overlap client count as one
-                count += partialLength.overlapRemoveClients.size();
-            }
+            // Each overlap client count as one
+            count += partialLength.overlapRemoveClients.size();
         }
-        return count;
     }
+    return count;
+}
 
-    private verify() {
-        if (this.clientSeqNumbers) {
-            let cliCount = 0;
-            for (const cliSeq of this.clientSeqNumbers) {
-                if (cliSeq) {
-                    cliCount += this.verifyPartialLengths(cliSeq, true);
-                }
+function verify(partialSeqLengths: PartialSequenceLengths) {
+    if (partialSeqLengths["clientSeqNumbers"]) {
+        let cliCount = 0;
+        for (const cliSeq of partialSeqLengths["clientSeqNumbers"]) {
+            if (cliSeq) {
+                cliCount += verifyPartialLengths(partialSeqLengths, cliSeq, true);
             }
+        }
 
-            // If we have client view, we should have the flat view
-            assert(!!this.partialLengths, 0x059 /* "Client view exists but flat view does not!" */);
-            const flatCount = this.verifyPartialLengths(this.partialLengths, false);
+        // If we have client view, we should have the flat view
+        assert(!!partialSeqLengths["partialLengths"], 0x059 /* "Client view exists but flat view does not!" */);
+        const flatCount = verifyPartialLengths(partialSeqLengths, partialSeqLengths["partialLengths"], false);
 
-            // The number of partial lengths on the client view and flat view should be the same
-            assert(flatCount === cliCount,
-                0x05a /* "Mismatch between number of partial lengths on client and flat views!" */);
-        } else {
-            // If we don't have a client view, we shouldn't have the flat view either
-            assert(!this.partialLengths, 0x05b /* "Flat view exists but client view does not!" */);
-        }
+        // The number of partial lengths on the client view and flat view should be the same
+        assert(flatCount === cliCount,
+            0x05a /* "Mismatch between number of partial lengths on client and flat views!" */);
+    } else {
+        // If we don't have a client view, we shouldn't have the flat view either
+        assert(!partialSeqLengths["partialLengths"], 0x05b /* "Flat view exists but client view does not!" */);
     }
 }
+/* eslint-enable @typescript-eslint/dot-notation */
 
 /**
  * Clones an `overlapRemoveClients` red-black tree.

packages/dds/merge-tree/src/test/partialLength.spec.ts

@@ -0,0 +1,174 @@
+/*!
+ * Copyright (c) Microsoft Corporation and contributors. All rights reserved.
+ * Licensed under the MIT License.
+ */
+
+import { strict as assert } from "assert";
+import { MergeTree } from "../mergeTree";
+import { MergeTreeDeltaType } from "../ops";
+import { PartialSequenceLengths } from "../partialLengths";
+import { TextSegment } from "../textSegment";
+import { insertText } from "./testUtils";
+
+describe("partial lengths", () => {
+    let mergeTree: MergeTree;
+    const localClientId = 17;
+    const remoteClientId = 18;
+
+    function validatePartialLengths(
+        clientId: number,
+        seq: number,
+        expectedLen?: number,
+    ): void {
+        const partialLen = mergeTree.root.partialLengths?.getPartialLength(
+            seq,
+            clientId,
+        );
+
+        let len = 0;
+
+        mergeTree.walkAllSegments(mergeTree.root, (segment) => {
+            if (segment.isLeaf() && !segment.removedClientIds?.length) {
+                len += segment.cachedLength;
+            }
+            return true;
+        });
+
+        assert.equal(partialLen, len);
+
+        if (expectedLen !== undefined) {
+            assert.equal(partialLen, expectedLen);
+        }
+    }
+
+    beforeEach(() => {
+        PartialSequenceLengths.options.verify = true;
+        mergeTree = new MergeTree();
+        mergeTree.insertSegments(
+            0,
+            [TextSegment.make("hello world!")],
+            0,
+            localClientId,
+            0,
+            undefined);
+
+        mergeTree.startCollaboration(
+            localClientId,
+            /* minSeq: */ 0,
+            /* currentSeq: */ 0);
+    });
+
+    it("passes with no additional ops", () => {
+        validatePartialLengths(localClientId, 0);
+    });
+
+    describe("insert", () => {
+        it("local insert, local view", () => {
+            insertText(
+                mergeTree,
+                0,
+                0,
+                localClientId,
+                1,
+                "more ",
+                undefined,
+                { op: { type: MergeTreeDeltaType.INSERT } },
+            );
+
+            validatePartialLengths(localClientId, 0);
+        });
+        it("local insert, remote view", () => {
+            insertText(
+                mergeTree,
+                0,
+                0,
+                localClientId,
+                1,
+                "more ",
+                undefined,
+                { op: { type: MergeTreeDeltaType.INSERT } },
+            );
+
+            validatePartialLengths(remoteClientId, 1);
+        });
+        it("remote insert, local view", () => {
+            insertText(
+                mergeTree,
+                0,
+                0,
+                remoteClientId,
+                1,
+                "more ",
+                undefined,
+                { op: { type: MergeTreeDeltaType.INSERT } },
+            );
+
+            validatePartialLengths(localClientId, 1);
+        });
+        it("remote insert, remote view", () => {
+            insertText(
+                mergeTree,
+                0,
+                0,
+                remoteClientId,
+                1,
+                "more ",
+                undefined,
+                { op: { type: MergeTreeDeltaType.INSERT } },
+            );
+
+            validatePartialLengths(remoteClientId, 0);
+        });
+    });
+
+    describe("delete", () => {
+        it("local delete, local view", () => {
+            mergeTree.markRangeRemoved(
+                0,
+                12,
+                0,
+                localClientId,
+                1,
+                false,
+                undefined as any);
+
+            validatePartialLengths(localClientId, 0, 0);
+        });
+        it("local delete, remote view", () => {
+            mergeTree.markRangeRemoved(
+                0,
+                12,
+                0,
+                localClientId,
+                1,
+                false,
+                undefined as any);
+
+            validatePartialLengths(remoteClientId, 1, 0);
+        });
+        it("remote delete, local view", () => {
+            mergeTree.markRangeRemoved(
+                0,
+                12,
+                0,
+                remoteClientId,
+                1,
+                false,
+                undefined as any);
+
+            validatePartialLengths(localClientId, 1, 0);
+        });
+        it("remote delete, remote view", () => {
+            mergeTree.markRangeRemoved(
+                0,
+                12,
+                0,
+                remoteClientId,
+                1,
+                false,
+                undefined as any);
+
+            validatePartialLengths(remoteClientId, 0, 0);
+        });
+    });
+});