Support rebasing ops for testing. Part 1, the mock. #16163

andre4i · 2023-06-27T19:10:28Z

Description

Adding a mock to support the new states added with #16176. This will be further integrated into the test pipeline via the fuzz testing harness.

vladsud · 2023-06-29T15:57:20Z

api-report/test-runtime-utils.api.md

+// @public
+export class MockContainerRuntimeFactoryForRebasing extends MockContainerRuntimeFactory {
+    // (undocumented)
+    createContainerRuntime(dataStoreRuntime: MockFluidDataStoreRuntime, overrides?: {


just create() is good :)

From MockContainerRuntimeFactory

vladsud · 2023-06-29T15:58:29Z

api-report/test-runtime-utils.api.md

+export class MockContainerRuntimeFactoryForRebasing extends MockContainerRuntimeFactory {
+    // (undocumented)
+    createContainerRuntime(dataStoreRuntime: MockFluidDataStoreRuntime, overrides?: {
+        minimumSequenceNumber?: number;


While I can see why you might want to create shape like that for last arg, I'd rather switch to this form when it's needed. Optional inside of optional is not great design pattern :)

This is all existing code and I agree it can be refactored into a better shape. However, I don't believe this change should address that.

vladsud · 2023-06-29T15:58:57Z

api-report/test-runtime-utils.api.md

+    // (undocumented)
+    processOneMessage(): void;
+    // (undocumented)
+    processSomeMessages(count: number): void;


Can you merge these 3 APIs into one? with count being optional (meaning all)?

I think that's outside the scope of this change. These methods are coming from MockContainerRuntimeFactory

packages/runtime/test-runtime-utils/src/mocksForRebasing.ts

vladsud · 2023-07-10T15:12:57Z

packages/runtime/test-runtime-utils/src/mocks.ts

@@ -430,6 +435,17 @@ export class MockFluidDataStoreRuntime
 		"fluid:MockFluidDataStoreRuntime",
 	);
 	public quorum = new MockQuorumClients();
+	public containerRuntime?: MockContainerRuntime;


I'd suggest we comment / document members as if it was public code. It's not very clear what is the purpose and semantics around this member.

These are all part of the public API and currently undocumented.

vladsud · 2023-07-10T15:13:56Z

packages/runtime/test-runtime-utils/src/mocks.ts

@@ -430,6 +435,17 @@ export class MockFluidDataStoreRuntime
 		"fluid:MockFluidDataStoreRuntime",
 	);
 	public quorum = new MockQuorumClients();
+	public containerRuntime?: MockContainerRuntime;
+	private readonly deltaConnections: MockDeltaConnection[] = [];
+	public createDeltaConnection?(): MockDeltaConnection {


Curious, why is it "?" ? I.e. it's actual implementation, it's not optional declaration.

Back-compat. This is a public API so if it's not optional it will break back-compat (we'd need to do the whole typeValidation->broken thing).

packages/runtime/test-runtime-utils/src/mocks.ts

msfluid-bot · 2023-07-18T20:11:22Z

⯅ @fluid-example/bundle-size-tests: +230 Bytes

Metric Name	Baseline Size	Compare Size	Size Diff
aqueduct.js	455.11 KB	455.11 KB	■ No change
connectionState.js	680 Bytes	680 Bytes	■ No change
containerRuntime.js	244.54 KB	244.54 KB	■ No change
loader.js	151.23 KB	151.23 KB	■ No change
map.js	47.19 KB	47.19 KB	■ No change
matrix.js	147.86 KB	147.86 KB	■ No change
odspDriver.js	93.68 KB	93.68 KB	■ No change
odspPrefetchSnapshot.js	44.51 KB	44.51 KB	■ No change
sharedString.js	164.77 KB	164.77 KB	■ No change
sharedTree2.js	242.72 KB	242.95 KB	⯅ +230 Bytes
Total Size	1.71 MB	1.71 MB	⯅ +230 Bytes

Baseline commit: dd316cf

Generated by 🚫 dangerJS against a69c719

andre4i · 2023-07-19T18:03:01Z

back to the drawing board with this, I believe it may be feasible to include this logic in the existing mock.

andre4i · 2023-07-20T00:04:36Z

packages/runtime/test-runtime-utils/src/mocksForRebasing.ts

+	public processAllMessages() {
+		// Increase the sequence number between batches
+		this.sequenceNumber++;
+		this.processSomeMessages(this.messages.length);


This is wrong, we should evacuate the whole queue at this point (even if the queue grows while it's being evacuated).

andre4i · 2023-07-20T17:01:25Z

Closing this in favor of #16460

## Description Adding a mock to support the new states added with #16176. This will be further integrated into the test pipeline via the fuzz testing harness. Another (arguably better) approach to #16163

andre4i added 2 commits June 27, 2023 09:33

Add mock for rebasing

221137f

Add rebasing

c24f3e8

github-actions bot added area: runtime Runtime related issues base: main PRs targeted against main branch labels Jun 27, 2023

andre4i added 2 commits June 27, 2023 12:11

Remove empty function

e31d309

Update docs

c62d1d4

github-actions bot added the public api change Changes to a public API label Jun 27, 2023

Small test for map

748631d

github-actions bot added the area: dds Issues related to distributed data structures label Jun 27, 2023

andre4i changed the title ~~Rebase consistency test harness. Work in progress~~ Support rebasing ops in the runtime mocks. Work in progress Jun 27, 2023

andre4i added 10 commits June 27, 2023 14:25

Update comment

309fb58

Update docs, override factory methods

8e8bf82

Better encapsulation

b3541be

Better encapsulation

52f6329

Rename test

f7ec14e

Standalone

140ce54

Merge branch 'main' into rebase-consistency-test-harness

9525c37

Track how many times the message is submitted

a349de8

Update comment

3eb38b3

Updated docs

0f6f330

andre4i changed the title ~~Support rebasing ops in the runtime mocks. Work in progress~~ Support rebasing ops for testing. Part 1, the mock. Jun 29, 2023

andre4i marked this pull request as ready for review June 29, 2023 15:31

andre4i requested review from a team as code owners June 29, 2023 15:31

andre4i added 2 commits June 29, 2023 08:39

Merge branch 'main' into rebase-consistency-test-harness

c22db7c

Add some comments

2595ef9

vladsud reviewed Jun 29, 2023

View reviewed changes

packages/runtime/test-runtime-utils/src/mocksForRebasing.ts Show resolved Hide resolved

andre4i added 13 commits June 30, 2023 16:07

Fix undefined reference

81a64a1

Fix slice

5031292

Rebase directly

5579681

Minor adjustments

8544f04

Merge branch 'main' into rebase-consistency-test-harness

0a04870

Resubmit mock - working

c6be0d9

Fix map rebasing test

35586b0

Add onchange event handler

14b3fd8

Simplify

980360d

Make the scenario more interesting

90922d9

Don't resubmit twice

7b5dde6

Add test for shareddirectory

12795b1

Fix test

155f56d

andre4i marked this pull request as ready for review July 6, 2023 18:40

andre4i requested a review from a team as a code owner July 6, 2023 18:40

vladsud reviewed Jul 10, 2023

View reviewed changes

packages/runtime/test-runtime-utils/src/mocks.ts Outdated Show resolved Hide resolved

andre4i added 2 commits July 18, 2023 12:38

Merge branch 'main' into rebase-consistency-test-harness

9dee177

Add assertions to ensure we're using the proper versions

a69c719

andre4i requested a review from agarwal-navin July 18, 2023 19:45

andre4i requested a review from Abe27342 July 18, 2023 22:39

andre4i marked this pull request as draft July 19, 2023 18:00

andre4i mentioned this pull request Jul 19, 2023

Change the runtime mocks to support batches and rebasing #16460

Merged

andre4i commented Jul 20, 2023

View reviewed changes

andre4i closed this Jul 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support rebasing ops for testing. Part 1, the mock. #16163

Support rebasing ops for testing. Part 1, the mock. #16163

andre4i commented Jun 27, 2023 •

edited

Loading

vladsud Jun 29, 2023

andre4i Jun 29, 2023

vladsud Jun 29, 2023

andre4i Jun 29, 2023

vladsud Jun 29, 2023

andre4i Jun 29, 2023

vladsud Jul 10, 2023

andre4i Jul 10, 2023

vladsud Jul 10, 2023

andre4i Jul 10, 2023 •

edited

Loading

msfluid-bot commented Jul 18, 2023

andre4i commented Jul 19, 2023

andre4i Jul 20, 2023

andre4i commented Jul 20, 2023

Support rebasing ops for testing. Part 1, the mock. #16163

Support rebasing ops for testing. Part 1, the mock. #16163

Conversation

andre4i commented Jun 27, 2023 • edited Loading

Description

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andre4i Jul 10, 2023 • edited Loading

Choose a reason for hiding this comment

msfluid-bot commented Jul 18, 2023

andre4i commented Jul 19, 2023

Choose a reason for hiding this comment

andre4i commented Jul 20, 2023

andre4i commented Jun 27, 2023 •

edited

Loading

andre4i Jul 10, 2023 •

edited

Loading