diff --git a/packages/dds/merge-tree/src/test/obliterate.rangeExpansion.spec.ts b/packages/dds/merge-tree/src/test/obliterate.rangeExpansion.spec.ts index 0d3dc1aa61d9..d358d5e418fd 100644 --- a/packages/dds/merge-tree/src/test/obliterate.rangeExpansion.spec.ts +++ b/packages/dds/merge-tree/src/test/obliterate.rangeExpansion.spec.ts @@ -13,28 +13,25 @@ function itCorrectlyObliterates({ title, action, expectedText, - expectedEventCount, }: { title: string; action: (helper: ReconnectTestHelper) => void; expectedText: string; - expectedEventCount: number; }): Mocha.Test { return it(title, () => { const events: number[] = []; - const helper = new ReconnectTestHelper(); + const helper = new ReconnectTestHelper({ + // TODO: uncomment this once the flag is checked in + /* mergeTreeEnableSidedObliterate: true */ + }); helper.clients.A.on("delta", (opArgs, deltaArgs) => { events.push(deltaArgs.operation); }); action(helper); helper.processAllOps(); - assert.equal(helper.clients.A.getText(), expectedText); - assert.equal(helper.clients.B.getText(), expectedText); - assert.equal(helper.clients.C.getText(), expectedText); - assert.equal(events.length, expectedEventCount, `events: ${events.join(", ")}`); - helper.logger.validate(); + helper.logger.validate({ baseText: expectedText }); }); } @@ -44,15 +41,14 @@ describe.skip("obliterate", () => { action: (helper) => { helper.insertText("A", 0, "|ABC>"); helper.processAllOps(); - helper.obliterateRange("A", { pos: 0, side: Side.After }, { pos: 4, side: Side.After }); + helper.obliterateRange("A", { pos: 0, side: Side.After }, { pos: 4, side: Side.Before }); // not concurrent to A's obliterate - ops on the same client are never concurrent to one another // because they are all sequenced locally - helper.insertText("A", 1, "XYZ"); - helper.obliterateRange("B", { pos: 0, side: Side.After }, { pos: 4, side: Side.After }); - helper.insertText("B", 1, "XYZ"); + helper.insertText("A", 1, "AAA"); + helper.obliterateRange("B", { pos: 0, side: Side.After }, { pos: 4, side: Side.Before }); + helper.insertText("B", 1, "BBB"); }, - expectedText: "|XYZ>", - expectedEventCount: 3, + expectedText: "|BBB>", }); itCorrectlyObliterates({ title: "does not obliterate non-adjacent insert", @@ -63,8 +59,48 @@ describe.skip("obliterate", () => { // do not obliterate the XYZ - outside the obliterated range without expansion helper.insertText("B", 0, "XYZ"); }, - expectedText: "XYZheo world", - expectedEventCount: 3, + expectedText: "XYZhe world", + }); + itCorrectlyObliterates({ + title: "zero length obliterate in the middle of the string", + action: (helper) => { + helper.insertText("A", 0, "hello world"); + helper.processAllOps(); + + helper.obliterateRange("A", { pos: 0, side: Side.After }, { pos: 1, side: Side.Before }); + helper.insertText("B", 0, "more "); + }, + expectedText: "hello world", + }); + itCorrectlyObliterates({ + title: "obliterate, then insert at the end of the string", + action: (helper) => { + helper.insertText("A", 0, "hello world"); + helper.processAllOps(); + + helper.obliterateRange( + "A", + { pos: 5, side: Side.Before }, + { pos: 10, side: Side.After }, + ); + helper.insertText("B", 10, "123"); + }, + expectedText: "hello", + }); + itCorrectlyObliterates({ + title: "insert, then obliterate at the end of the string", + action: (helper) => { + helper.insertText("A", 0, "hello world"); + helper.processAllOps(); + + helper.insertText("A", 10, "123"); + helper.obliterateRange( + "B", + { pos: 5, side: Side.Before }, + { pos: 10, side: Side.After }, + ); + }, + expectedText: "hello", }); itCorrectlyObliterates({ title: "zero length obliterate in the middle of the string", @@ -76,8 +112,6 @@ describe.skip("obliterate", () => { helper.insertText("B", 0, "more "); }, expectedText: "hello world", - // TODO: remove this after merging sided obliterates - expectedEventCount: 3, }); itCorrectlyObliterates({ title: "obliterate, then insert at the end of the string", @@ -93,8 +127,6 @@ describe.skip("obliterate", () => { helper.insertText("B", 10, "123"); }, expectedText: "hello", - // TODO: remove this after merging sided obliterates - expectedEventCount: 3, }); itCorrectlyObliterates({ title: "insert, then obliterate at the end of the string", @@ -110,8 +142,6 @@ describe.skip("obliterate", () => { ); }, expectedText: "hello", - // TODO: remove this after merging sided obliterates - expectedEventCount: 3, }); }); @@ -126,27 +156,25 @@ describe.skip("overlapping edits", () => { helper.obliterateRange( "A", { pos: 0, side: Side.Before }, - { pos: text.length, side: Side.After }, + { pos: text.length - 1, side: Side.After }, ); helper.obliterateRange( "B", { pos: 0, side: Side.Before }, - { pos: text.length, side: Side.After }, + { pos: text.length - 1, side: Side.After }, ); }, expectedText: "", - expectedEventCount: 2, }); itCorrectlyObliterates({ title: "adjacent obliterates", action: (helper) => { helper.insertText("A", 0, "hello world"); helper.processAllOps(); - helper.obliterateRange("A", { pos: 2, side: Side.Before }, { pos: 4, side: Side.After }); - helper.obliterateRange("B", { pos: 4, side: Side.Before }, { pos: 6, side: Side.After }); + helper.obliterateRange("A", { pos: 2, side: Side.Before }, { pos: 3, side: Side.After }); + helper.obliterateRange("B", { pos: 4, side: Side.Before }, { pos: 5, side: Side.After }); }, expectedText: "heworld", - expectedEventCount: 2, }); itCorrectlyObliterates({ title: "remove within obliterated range", @@ -156,8 +184,7 @@ describe.skip("overlapping edits", () => { helper.obliterateRange("A", { pos: 2, side: Side.Before }, { pos: 5, side: Side.After }); helper.removeRange("B", 3, 4); }, - expectedText: "he world", - expectedEventCount: 2, + expectedText: "heworld", }); itCorrectlyObliterates({ title: "obliterate, then remove adjacent to range start", @@ -167,8 +194,7 @@ describe.skip("overlapping edits", () => { helper.obliterateRange("A", { pos: 1, side: Side.After }, { pos: 5, side: Side.After }); helper.removeRange("B", 1, 2); }, - expectedText: "he world", - expectedEventCount: 2, + expectedText: "hworld", }); itCorrectlyObliterates({ title: "obliterate, then remove adjacent to range end", @@ -178,8 +204,7 @@ describe.skip("overlapping edits", () => { helper.obliterateRange("A", { pos: 2, side: Side.Before }, { pos: 4, side: Side.After }); helper.removeRange("B", 4, 6); }, - expectedText: "he world", - expectedEventCount: 2, + expectedText: "heworld", }); itCorrectlyObliterates({ title: "remove, then obliterate adjacent to range start", @@ -190,7 +215,6 @@ describe.skip("overlapping edits", () => { helper.obliterateRange("B", { pos: 2, side: Side.Before }, { pos: 4, side: Side.After }); }, expectedText: "heworld", - expectedEventCount: 3, }); itCorrectlyObliterates({ title: "remove, then obliterate adjacent to range end", @@ -200,8 +224,7 @@ describe.skip("overlapping edits", () => { helper.removeRange("A", 2, 4); helper.obliterateRange("B", { pos: 3, side: Side.After }, { pos: 6, side: Side.After }); }, - expectedText: "heworld", - expectedEventCount: 3, + expectedText: "heorld", }); }); @@ -224,7 +247,6 @@ describe.skip("reconnect", () => { helper.insertText("A", 2, "123"); }, expectedText: "heo world", - expectedEventCount: 2, }); itCorrectlyObliterates({ title: "add text, disconnect, obliterate, insert adjacent to obliterated range, reconnect", @@ -243,7 +265,6 @@ describe.skip("reconnect", () => { helper.submitDisconnectedOp("C", op); }, expectedText: "heo world", - expectedEventCount: 2, }); }); @@ -270,10 +291,9 @@ describe.skip("sided obliterates", () => { // [2, 4]: before 2, after 4 => h e [l l o] _ w o r l d helper.obliterateRange("B", { pos: 2, side: Side.Before }, { pos: 4, side: Side.After }); helper.insertText("C", 2, "123"); - helper.insertText("C", 5, "456"); + helper.insertText("C", 8, "456"); }, expectedText: "he world", - expectedEventCount: 2, }); itCorrectlyObliterates({ title: "2. A expand start endpoint, B expand end endpoint", @@ -284,15 +304,18 @@ describe.skip("sided obliterates", () => { helper.insertText("A", 0, "ABC"); helper.processAllOps(); helper.insertText("A", 2, "D"); - // ( 1]: after 0, after 1 => A( B] C - helper.obliterateRange("A", { pos: 0, side: Side.After }, { pos: 2, side: Side.After }); + // ( 1]: after 0, after 1 => A( B] D C + helper.obliterateRange("A", { pos: 0, side: Side.After }, { pos: 1, side: Side.After }); // included in the range -- should get obliterated - helper.insertText("B", 2, "E"); - // [1 ): before 1, before 2 => A [B )C - helper.obliterateRange("B", { pos: 1, side: Side.After }, { pos: 3, side: Side.Before }); + helper.insertText("B", 1, "E"); + // [1 ): before 1, before 2 => A E [B )C + helper.obliterateRange( + "B", + { pos: 1, side: Side.Before }, + { pos: 3, side: Side.Before }, + ); }, - expectedText: "ADC", - expectedEventCount: 2, + expectedText: "AC", }); itCorrectlyObliterates({ title: "3. A expand both endpoints, B expand start", @@ -309,7 +332,6 @@ describe.skip("sided obliterates", () => { helper.insertText("C", 4, "456"); }, expectedText: "he world", - expectedEventCount: 2, }); itCorrectlyObliterates({ title: "4. A expand both endpoints, B expand end", @@ -325,12 +347,11 @@ describe.skip("sided obliterates", () => { { pos: 2, side: Side.Before }, { pos: 5, side: Side.Before }, ); - helper.insertText("C", 2, "123"); + helper.insertText("C", 2, "123"); // he123llo world // for this to be interesting, might want to insert at 5 - helper.insertText("C", 4, "456"); + helper.insertText("C", 8, "456"); // he123llo456 world }, expectedText: "he world", - expectedEventCount: 3, }); itCorrectlyObliterates({ title: "5. A expand neither endpoint, B expand start", @@ -342,8 +363,9 @@ describe.skip("sided obliterates", () => { helper.obliterateRange("A", { pos: 2, side: Side.Before }, { pos: 4, side: Side.After }); // ( 2, 4]: after 1, after 4 => h e( l l o] _ w o r l d helper.obliterateRange("B", { pos: 1, side: Side.After }, { pos: 4, side: Side.After }); - helper.insertText("C", 2, "123"); - helper.insertText("C", 5, "456"); + + helper.insertText("C", 2, "123"); // h e( 123 l l o] _ w o r l d + helper.insertText("C", 8, "456"); // h e( 123 l l o) 456 _ w o r l d helper.processAllOps(); assert.equal(helper.clients.A.getText(), "he456 world"); @@ -353,7 +375,6 @@ describe.skip("sided obliterates", () => { helper.logger.validate(); }, expectedText: "he456 world", - expectedEventCount: 3, }); itCorrectlyObliterates({ title: "6. A expand neither endpoint, B expand end", @@ -369,8 +390,10 @@ describe.skip("sided obliterates", () => { { pos: 2, side: Side.Before }, { pos: 5, side: Side.Before }, ); + helper.insertText("C", 2, "123"); - helper.insertText("C", 5, "456"); + helper.insertText("C", 8, "456"); + helper.processAllOps(); assert.equal(helper.clients.A.getText(), "he123 world"); @@ -380,6 +403,5 @@ describe.skip("sided obliterates", () => { helper.logger.validate(); }, expectedText: "he123 world", - expectedEventCount: 3, }); }); diff --git a/packages/dds/merge-tree/src/test/reconnectHelper.ts b/packages/dds/merge-tree/src/test/reconnectHelper.ts index df3169302c2a..5b9a8bc8eaf2 100644 --- a/packages/dds/merge-tree/src/test/reconnectHelper.ts +++ b/packages/dds/merge-tree/src/test/reconnectHelper.ts @@ -7,7 +7,13 @@ import { strict as assert } from "node:assert"; import { ISequencedDocumentMessage } from "@fluidframework/driver-definitions/internal"; -import { SegmentGroup, endpointPosAndSide, type SequencePlace } from "../index.js"; +import { + SegmentGroup, + endpointPosAndSide, + type IMergeTreeOptions, + type InteriorSequencePlace, + type SequencePlace, +} from "../index.js"; import { IMergeTreeDeltaOp, type IMergeTreeInsertMsg, @@ -15,31 +21,42 @@ import { type IMergeTreeRemoveMsg, } from "../ops.js"; +import type { TestClient } from "./testClient.js"; import { TestClientLogger, createClientsAtInitialState } from "./testClientLogger.js"; const ClientIds = ["A", "B", "C", "D"] as const; type ClientName = (typeof ClientIds)[number]; export class ReconnectTestHelper { - clients = createClientsAtInitialState( - { - initialState: "", - options: { mergeTreeEnableObliterate: true, mergeTreeEnableObliterateReconnect: true }, - }, - ...ClientIds, - ); + clients: Record & { all: TestClient[] }; idxFromName(name: ClientName): number { return (name.codePointAt(0) ?? 0) - ("A".codePointAt(0) ?? 0); } - logger = new TestClientLogger(this.clients.all); + logger: TestClientLogger; ops: ISequencedDocumentMessage[] = []; - perClientOps: ISequencedDocumentMessage[][] = this.clients.all.map(() => []); + perClientOps: ISequencedDocumentMessage[][]; seq: number = 0; + public constructor(options: IMergeTreeOptions = {}) { + this.clients = createClientsAtInitialState( + { + initialState: "", + options: { + mergeTreeEnableObliterate: true, + mergeTreeEnableObliterateReconnect: true, + ...options, + }, + }, + ...ClientIds, + ); + this.logger = new TestClientLogger(this.clients.all); + this.perClientOps = this.clients.all.map(() => []); + } + public insertText(clientName: ClientName, pos: number, text: string): void { const client = this.clients[clientName]; this.ops.push(client.makeOpMessage(client.insertTextLocal(pos, text), ++this.seq)); @@ -52,23 +69,16 @@ export class ReconnectTestHelper { public obliterateRange( clientName: ClientName, - start: SequencePlace, - end: SequencePlace, + start: number | InteriorSequencePlace, + end: number | InteriorSequencePlace, ): void { const client = this.clients[clientName]; - let { startPos, endPos } = endpointPosAndSide(start, end); - assert( - startPos !== undefined && endPos !== undefined, - "start and end positions must be defined", - ); - startPos = startPos === "start" ? 0 : startPos; - endPos = endPos === "end" ? client.getLength() : endPos; - assert( - startPos !== "end" && endPos !== "start", - "start cannot be end and end cannot be start", - ); this.ops.push( - client.makeOpMessage(client.obliterateRangeLocal(startPos, endPos), ++this.seq), + client.makeOpMessage( + // TODO: remove type assertions when sidedness is enabled + client.obliterateRangeLocal(start as number, end as number), + ++this.seq, + ), ); }