Skip to content

Commit 49e5be9

Browse files
Reduce direct references to SharedTree class and its factory class (#24312)
## Description Tweak SharedTree tests to make less direct use of the SharedTreeClass and instead prefer its interfaces. Also reduce unnecessary use of its factory. These changes make the test suite more compatible with changes like #23729 which adjust how the kernel is used to implement the SharedObject.
1 parent 7ba81ea commit 49e5be9

File tree

10 files changed

+91
-131
lines changed

10 files changed

+91
-131
lines changed

packages/dds/tree/src/test/shared-tree-core/sharedTreeCore.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ describe("SharedTreeCore", () => {
251251
{
252252
deltaConnection: dataStoreRuntime2.createDeltaConnection(),
253253
objectStorage: MockStorage.createFromSummary(
254-
tree1.summarizeCore(mockSerializer).summary,
254+
tree1.kernel.summarizeCore(mockSerializer).summary,
255255
),
256256
},
257257
factory.attributes,

packages/dds/tree/src/test/shared-tree/fuzz/fuzzEditGenerators.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import type {
2424
TreeNodeSchemaIdentifier,
2525
} from "../../../core/index.js";
2626
import { type DownPath, toDownPath } from "../../../feature-libraries/index.js";
27-
import { Tree, type ITreePrivate, type SharedTree } from "../../../shared-tree/index.js";
27+
import { Tree, type ISharedTree, type ITreePrivate } from "../../../shared-tree/index.js";
2828
import { getOrCreate, makeArray } from "../../../util/index.js";
2929

3030
import {
@@ -102,7 +102,7 @@ export interface FuzzTestState extends DDSFuzzTestState<TreeFactory> {
102102
* SharedTrees undergoing a transaction will have a forked view in {@link transactionViews} instead,
103103
* which should be used in place of this view until the transaction is complete.
104104
*/
105-
clientViews?: Map<SharedTree, FuzzView>;
105+
clientViews?: Map<ISharedTree, FuzzView>;
106106
/**
107107
* Schematized view of clients undergoing transactions with their nodeSchemas.
108108
* Edits to this view are not visible to other clients until the transaction is closed.
@@ -118,7 +118,7 @@ export interface FuzzTestState extends DDSFuzzTestState<TreeFactory> {
118118
* SharedTrees undergoing a transaction will have a forked view in {@link transactionViews} instead,
119119
* which should be used in place of this view until the transaction is complete.
120120
*/
121-
forkedViews?: Map<SharedTree, FuzzView[]>;
121+
forkedViews?: Map<ISharedTree, FuzzView[]>;
122122
}
123123

124124
export function viewFromState(

packages/dds/tree/src/test/shared-tree/fuzz/fuzzEditReducers.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import type { IFluidHandle } from "@fluidframework/core-interfaces";
1212

1313
import type { Revertible } from "../../../core/index.js";
1414
import type { DownPath } from "../../../feature-libraries/index.js";
15-
import { Tree, type SharedTree } from "../../../shared-tree/index.js";
15+
import { Tree, type ISharedTree } from "../../../shared-tree/index.js";
1616
import { validateFuzzTreeConsistency } from "../../utils.js";
1717

1818
import {
@@ -190,7 +190,7 @@ export function applySchemaOp(state: FuzzTestState, operation: SchemaChange) {
190190
export function applyForkMergeOperation(state: FuzzTestState, branchEdit: ForkMergeOperation) {
191191
switch (branchEdit.contents.type) {
192192
case "fork": {
193-
const forkedViews = state.forkedViews ?? new Map<SharedTree, FuzzView[]>();
193+
const forkedViews = state.forkedViews ?? new Map<ISharedTree, FuzzView[]>();
194194
const clientForkedViews = forkedViews.get(state.client.channel) ?? [];
195195

196196
if (branchEdit.contents.branchNumber !== undefined) {
@@ -211,7 +211,7 @@ export function applyForkMergeOperation(state: FuzzTestState, branchEdit: ForkMe
211211
}
212212
case "merge": {
213213
const forkBranchIndex = branchEdit.contents.forkBranch;
214-
const forkedViews = state.forkedViews ?? new Map<SharedTree, FuzzView[]>();
214+
const forkedViews = state.forkedViews ?? new Map<ISharedTree, FuzzView[]>();
215215
const clientForkedViews = forkedViews.get(state.client.channel) ?? [];
216216

217217
const baseBranch =

packages/dds/tree/src/test/shared-tree/fuzz/fuzzUtils.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import {
2727
import type {
2828
ITreeCheckout,
2929
SchematizingSimpleTreeView,
30-
SharedTree,
3130
TreeCheckout,
3231
} from "../../../shared-tree/index.js";
3332
import { testSrcPath } from "../../testSrcPath.cjs";
@@ -45,8 +44,11 @@ import {
4544
} from "../../../simple-tree/index.js";
4645
import type { IFluidHandle } from "@fluidframework/core-interfaces";
4746

48-
// eslint-disable-next-line import/no-internal-modules
49-
import type { SharedTreeOptionsInternal } from "../../../shared-tree/sharedTree.js";
47+
import type {
48+
ISharedTree,
49+
SharedTreeOptionsInternal,
50+
// eslint-disable-next-line import/no-internal-modules
51+
} from "../../../shared-tree/sharedTree.js";
5052
import { typeboxValidator } from "../../../external-utilities/index.js";
5153
import type { FuzzView } from "./fuzzEditGenerators.js";
5254

@@ -161,8 +163,8 @@ export class SharedTreeFuzzTestFactory extends SharedTreeTestFactory {
161163
* @param onLoad - Called once for each tree that is loaded from a summary.
162164
*/
163165
public constructor(
164-
protected override readonly onCreate: (tree: SharedTree) => void,
165-
protected override readonly onLoad?: (tree: SharedTree) => void,
166+
protected override readonly onCreate: (tree: ISharedTree) => void,
167+
protected override readonly onLoad?: (tree: ISharedTree) => void,
166168
options: SharedTreeOptionsInternal = {},
167169
) {
168170
super(onCreate, onLoad, {

packages/dds/tree/src/test/shared-tree/sharedTree.spec.ts

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ import {
4848
ForestTypeOptimized,
4949
ForestTypeReference,
5050
getBranch,
51+
type ISharedTree,
5152
type ITreePrivate,
52-
type SharedTree,
5353
Tree,
5454
type TreeCheckout,
5555
} from "../../shared-tree/index.js";
@@ -92,23 +92,30 @@ import {
9292
TreeFactory,
9393
SharedTree as SharedTreeKind,
9494
} from "../../treeFactory.js";
95-
import type { ISharedObjectKind } from "@fluidframework/shared-object-base/internal";
95+
import {
96+
SharedObjectCore,
97+
type ISharedObjectKind,
98+
} from "@fluidframework/shared-object-base/internal";
9699
import { TestAnchor } from "../testAnchor.js";
97100
// eslint-disable-next-line import/no-internal-modules
98101
import { handleSchema, numberSchema, stringSchema } from "../../simple-tree/leafNodeSchema.js";
99102
import { singleJsonCursor } from "../json/index.js";
100103
import { AttachState } from "@fluidframework/container-definitions";
101104
import { JsonAsTree } from "../../jsonDomainSchema.js";
102-
// eslint-disable-next-line import/no-internal-modules
103-
import { toSimpleTreeSchema } from "../../simple-tree/api/index.js";
105+
import {
106+
asTreeViewAlpha,
107+
toSimpleTreeSchema,
108+
type ITree,
109+
// eslint-disable-next-line import/no-internal-modules
110+
} from "../../simple-tree/api/index.js";
104111
import type { IChannel } from "@fluidframework/datastore-definitions/internal";
105112

106113
const enableSchemaValidation = true;
107114

108115
const DebugSharedTree = configuredSharedTree({
109116
jsonValidator: typeboxValidator,
110117
forest: ForestTypeReference,
111-
}) as ISharedObjectKind<unknown> as ISharedObjectKind<SharedTree>;
118+
}) as ISharedObjectKind<unknown> as ISharedObjectKind<ISharedTree>;
112119

113120
class MockSharedTreeRuntime extends MockFluidDataStoreRuntime {
114121
public constructor() {
@@ -661,6 +668,7 @@ describe("SharedTree", () => {
661668
view.root.insertAtEnd("b");
662669

663670
const tree2 = sharedTreeFactory.create(runtime, "tree2");
671+
assert(tree2 instanceof SharedObjectCore);
664672
await tree2.load({
665673
deltaConnection: runtime.createDeltaConnection(),
666674
objectStorage: MockStorage.createFromSummary((await tree.summarize()).summary),
@@ -727,7 +735,7 @@ describe("SharedTree", () => {
727735
});
728736

729737
it("can summarize local edits in the attach summary", async () => {
730-
const onCreate = (tree: SharedTree) => {
738+
const onCreate = (tree: ITreePrivate) => {
731739
const view = tree.viewWith(
732740
new TreeViewConfiguration({ schema: StringArray, enableSchemaValidation }),
733741
);
@@ -764,7 +772,7 @@ describe("SharedTree", () => {
764772
});
765773

766774
it("can tolerate local edits submitted as part of a transaction in the attach summary", async () => {
767-
const onCreate = (tree: SharedTree) => {
775+
const onCreate = (tree: ITreePrivate) => {
768776
// Schematize uses a transaction as well
769777
const view = tree.viewWith(
770778
new TreeViewConfiguration({ schema: StringArray, enableSchemaValidation }),
@@ -804,7 +812,7 @@ describe("SharedTree", () => {
804812
// AB#5745: Enable this test once it passes.
805813
// TODO: above mentioned task is done, but this still fails. Fix it.
806814
it.skip("can tolerate incomplete transactions when attaching", async () => {
807-
const onCreate = (tree: SharedTree) => {
815+
const onCreate = (tree: ITreePrivate) => {
808816
const view = tree.viewWith(
809817
new TreeViewConfiguration({ schema: StringArray, enableSchemaValidation }),
810818
);
@@ -902,7 +910,7 @@ describe("SharedTree", () => {
902910
});
903911

904912
it("can process changes while detached", async () => {
905-
const onCreate = (t: SharedTree) => {
913+
const onCreate = (t: ITree) => {
906914
const viewInit = t.viewWith(
907915
new TreeViewConfiguration({ schema: StringArray, enableSchemaValidation }),
908916
);
@@ -1781,7 +1789,7 @@ describe("SharedTree", () => {
17811789
});
17821790

17831791
it("process changes while detached", async () => {
1784-
const onCreate = (parentTree: SharedTree) => {
1792+
const onCreate = (parentTree: ISharedTree) => {
17851793
const parentView = parentTree.viewWith(
17861794
new TreeViewConfiguration({
17871795
schema: StringArray,
@@ -2209,7 +2217,9 @@ describe("SharedTree", () => {
22092217
const tree = sharedTreeFactory.create(runtime, "tree");
22102218
const runtimeFactory = new MockContainerRuntimeFactory();
22112219
runtimeFactory.createContainerRuntime(runtime);
2212-
const view = tree.viewWith(new TreeViewConfiguration({ schema: StringArray }));
2220+
const view = asTreeViewAlpha(
2221+
tree.viewWith(new TreeViewConfiguration({ schema: StringArray })),
2222+
);
22132223
view.initialize([]);
22142224
assert.throws(
22152225
() => {
@@ -2230,6 +2240,7 @@ describe("SharedTree", () => {
22302240
const schema = sf.object("myObject", {});
22312241
const config = new TreeViewConfiguration({ schema, enableSchemaValidation });
22322242
const view = tree.viewWith(config);
2243+
assert(view instanceof SchematizingSimpleTreeView);
22332244

22342245
view.initialize({});
22352246
assert.equal(view.breaker, tree.kernel.breaker);

packages/dds/tree/src/test/shared-tree/undo.spec.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
chunkFromJsonTrees,
1818
createTestUndoRedoStacks,
1919
expectJsonTree,
20+
getView,
2021
moveWithin,
2122
TestTreeProviderLite,
2223
} from "../utils.js";
@@ -491,7 +492,7 @@ describe("Undo and redo", () => {
491492
const sharedTreeFactory = new TreeFactory({});
492493
const runtime = new MockFluidDataStoreRuntime({ idCompressor: createIdCompressor() });
493494
const tree = sharedTreeFactory.create(runtime, "tree");
494-
const view = tree.viewWith(new TreeViewConfiguration({ schema: Schema }));
495+
const view = asTreeViewAlpha(tree.viewWith(new TreeViewConfiguration({ schema: Schema })));
495496
view.initialize({ foo: 1 });
496497
assert.equal(tree.isAttached(), false);
497498
let revertible: Revertible | undefined;
@@ -505,6 +506,22 @@ describe("Undo and redo", () => {
505506
assert.equal(view.root.foo, 1);
506507
});
507508

509+
it("can undo while independent", () => {
510+
const sf = new SchemaFactory(undefined);
511+
class Schema extends sf.object("Object", { foo: sf.number }) {}
512+
const view = getView(new TreeViewConfiguration({ schema: Schema }));
513+
view.initialize({ foo: 1 });
514+
let revertible: Revertible | undefined;
515+
view.events.on("changed", (_, getRevertible) => {
516+
revertible = getRevertible?.();
517+
});
518+
view.root.foo = 2;
519+
assert.equal(view.root.foo, 2);
520+
assert(revertible !== undefined);
521+
revertible.revert();
522+
assert.equal(view.root.foo, 1);
523+
});
524+
508525
// TODO:#24414: Enable forkable revertibles tests to run on attached/detached mode.
509526
it("reverts original & forked revertibles after making change to the original view", () => {
510527
const originalView = createInitializedView();

0 commit comments

Comments
 (0)