refactor(tree): Enable compressed encoding in pending tests #21069
Conversation
github-actions
bot
added
area: dds
| // Can't use assertSchema() because it sets an event listener that fails when we're rebasing | ||
| // the changes that move from one schema to another, because we try to go from an optional rootField | ||
| // to a forbidden one. | ||
| // assertSchema(peer, schemaGeneralized); |
There was a problem hiding this comment.
Can we make it so the schema change is outside of the range of changes/commits that get rebased, so we can call assertSchema() like the other tests?
| scope: "test", | ||
| libraries: [leaf.library], | ||
| }); | ||
| const schemaGeneralized = builder.intoSchema(Any); |
There was a problem hiding this comment.
So before this change, was there no schema available for sharedtree( including a generalized one)? Was this generalized schema (as opposed to flexfield/rootField) recently introduced?
There was a problem hiding this comment.
To be honest I'm not entirely clear how it works :) just followed an existing pattern in other tests that used compressed encoding. I'm hoping the SharedTree team will point out if there's anything weird with the usage pattern.
| tree: SharedTree, | ||
| schema: FlexTreeSchema<TRoot>, | ||
| onDispose: () => void = () => assert.fail(), | ||
| ): FlexTreeView<TRoot> { | ||
| const viewSchema = new ViewSchema(defaultSchemaPolicy, {}, schema); | ||
| return requireSchema(tree.checkout, viewSchema, onDispose, createMockNodeKeyManager()); | ||
| } | ||
|
|
||
| export function updateSchema(tree: SharedTree, schema: FlexTreeSchema): void { |
| @@ -66,7 +70,7 @@ const nodesCountDeep = [ | |||
| // TODO: ADO#7111 Schema should be fixed to enable schema based encoding. | |||
There was a problem hiding this comment.
Can this comment be removed now?
| @@ -66,7 +70,7 @@ const nodesCountDeep = [ | |||
| // TODO: ADO#7111 Schema should be fixed to enable schema based encoding. | |||
| const factory = new SharedTreeFactory({ | |||
| jsonValidator: typeboxValidator, | |||
| treeEncodeType: TreeCompressionStrategy.Uncompressed, | |||
| treeEncodeType: TreeCompressionStrategy.Compressed, | |||
There was a problem hiding this comment.
Is there a need to keep benchmarks for a tree without compressed encoding? Wondering if we should maintain tests for both.
| updateSchema(tree, schemaGeneralized); | ||
| assertSchema(tree, schemaGeneralized); |
There was a problem hiding this comment.
I am not familiar with how these tests work exactly but I'm wondering if schema updates and assertions should occur after the inserts on line 373?
Description
Enables compressed encoding for some tests that were missing a schema.
Reviewer Guidance
The review process is outlined on this wiki page.
The task calls for
But I think that's not really what the tests AB#7111 was about needed, since they're operating at a level where the schema validator doesn't apply (they're creating a
SharedTreeobject and directly using itseditor(non-exposed API) to create changes, not schematizing it and using public APIs ofITree.