Skip to content

Enable toggleMove in base model; fix fuzz utils #25428

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Sep 16, 2025
Merged

Enable toggleMove in base model; fix fuzz utils #25428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
dad023a
toggle and toggleMove rollback
dannimad Sep 4, 2025
b91758a
Update packages/dds/legacy-dds/src/test/array/array.rollback.spec.ts
dannimad Sep 4, 2025
e9c8337
nit
dannimad Sep 4, 2025
99a33f1
Enable toggleMove in fuzz tests; fix fuzz utils
dannimad Sep 10, 2025
b19be09
Merge branch 'main' into fuzz-toggle
dannimad Sep 10, 2025
af48b64
unreachable
dannimad Sep 10, 2025
65d5abc
bug fix; moveIds fix
dannimad Sep 11, 2025
c4b09f3
rename DEBUG_value
dannimad Sep 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions packages/dds/legacy-dds/src/array/sharedArray.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,8 @@ export class SharedArrayClass<T extends SerializableTypeForSharedArray>
}
} else if (
!this.isLocalPending(op.entryId, "isLocalPendingDelete") &&
!this.isLocalPending(op.entryId, "isLocalPendingMove")
!this.isLocalPending(op.entryId, "isLocalPendingMove") &&
this.getLiveEntry(op.entryId).isDeleted === false
) {
this.updateLiveEntry(this.getLiveEntry(op.entryId).entryId, op.entryId);
}
Expand Down Expand Up @@ -993,7 +994,7 @@ export class SharedArrayClass<T extends SerializableTypeForSharedArray>
newElement.isDeleted = isDeleted;
}
}
newElement.isLocalPendingMove += 1;
opEntry.isLocalPendingMove += 1;
break;
}
case OperationType.toggle: {
Expand Down
91 changes: 16 additions & 75 deletions packages/dds/legacy-dds/src/test/array/arrayFuzzTests.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,87 +5,28 @@

import * as path from "node:path";

import { takeAsync } from "@fluid-private/stochastic-test-utils";
import { createDDSFuzzSuite } from "@fluid-private/test-dds-utils";
import { describe } from "mocha";

import { _dirname } from "./dirname.cjs";
import {
baseSharedArrayModel,
eventEmitterForFuzzHarness,
makeSharedArrayOperationGenerator,
} from "./fuzzUtils.js";
import { baseSharedArrayModel, eventEmitterForFuzzHarness } from "./fuzzUtils.js";

describe("SharedArray fuzz", () => {
createDDSFuzzSuite(
{
...baseSharedArrayModel,
generatorFactory: () =>
takeAsync(
100,
makeSharedArrayOperationGenerator({
insert: 5,
delete: 3,
move: 3,
insertBulkAfter: 1,
toggle: 1,
toggleMove: 1,
}),
),
createDDSFuzzSuite(baseSharedArrayModel, {
validationStrategy: { type: "fixedInterval", interval: 10 },
reconnectProbability: 0.15,
numberOfClients: 3,
clientJoinOptions: {
maxNumberOfClients: 5,
clientAddProbability: 0.1,
stashableClientProbability: 0.3,
},
{
validationStrategy: { type: "fixedInterval", interval: 10 },
reconnectProbability: 0.15,
numberOfClients: 3,
clientJoinOptions: {
maxNumberOfClients: 5,
clientAddProbability: 0.1,
stashableClientProbability: 0.3,
},
detachedStartOptions: {
numOpsBeforeAttach: 5,
},
rollbackProbability: 0,
defaultTestCount: 50,
saveFailures: { directory: path.join(_dirname, "../../src/test/results") },
skip: [9, 15, 44],
emitter: eventEmitterForFuzzHarness,
detachedStartOptions: {
numOpsBeforeAttach: 5,
},
);

createDDSFuzzSuite(
{
...baseSharedArrayModel,
workloadName: "rollback",
generatorFactory: () =>
takeAsync(
100,
makeSharedArrayOperationGenerator({
insert: 5,
delete: 3,
move: 3,
insertBulkAfter: 1,
toggle: 0, // TODO: solve toggle bugs.
toggleMove: 1,
}),
),
},
{
validationStrategy: { type: "fixedInterval", interval: 10 },
reconnectProbability: 0.15,
numberOfClients: 3,
clientJoinOptions: {
maxNumberOfClients: 5,
clientAddProbability: 0.1,
},
detachedStartOptions: {
numOpsBeforeAttach: 5,
rehydrateDisabled: true,
},
rollbackProbability: 0.2,
defaultTestCount: 50,
saveFailures: { directory: path.join(_dirname, "../../src/test/results") },
emitter: eventEmitterForFuzzHarness,
},
);
rollbackProbability: 0.2,
defaultTestCount: 50,
saveFailures: { directory: path.join(_dirname, "../../src/test/results") },
emitter: eventEmitterForFuzzHarness,
});
});
119 changes: 88 additions & 31 deletions packages/dds/legacy-dds/src/test/array/fuzzUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import type {
DDSFuzzModel,
DDSFuzzTestState,
} from "@fluid-private/test-dds-utils";
import { unreachableCase } from "@fluidframework/core-utils/internal";
import type { Serializable } from "@fluidframework/datastore-definitions/internal";

import type { ISharedArray, SerializableTypeForSharedArray } from "../../index.js";
Expand All @@ -35,40 +36,48 @@ export interface SharedArrayInsert<T> {

/**
* Type for the SharedArray operation
*
* DEBUG_value used entirely for debugging purposes to back track
* operations by value in the generated json files.
*/
export interface SharedArrayDelete {
type: "delete";
index: number;
DEBUG_value: unknown;
}

/**
* Type for the SharedArray operation
*
* DEBUG_value used entirely for debugging purposes to back track
* operations by value in the generated json files.
*/
export interface SharedArrayMove {
type: "move";
oldIndex: number;
newIndex: number;
DEBUG_value: unknown;
}

/**
* Type for the SharedArray operation
*
* DEBUG_value used entirely for debugging purposes to back track
* operations by value in the generated json files.
*/
export interface SharedArrayToggle {
type: "toggle";
entryId: string;
DEBUG_value: unknown;
}

/**
* Type for the SharedArray operation
*
* DEBUG_value used entirely for debugging purposes to back track
* operations by value in the generated json files.
*/
export interface SharedArrayToggleMove {
type: "toggleMove";
oldEntryId: string;
newEntryId: string;
DEBUG_value: unknown;
}

/**
Expand Down Expand Up @@ -100,24 +109,60 @@ export const eventEmitterForFuzzHarness = new TypedEventEmitter<DDSFuzzHarnessEv

type TrackableSharedArray = ISharedArray<SerializableTypeForSharedArray> & {
// This is used to track the entry IDs for insert and move operations.
insertIds: Set<string>;
moveIds: Set<string>;
insertIds: Map<string, unknown>;
moveIds: Map<string, string>;
toggleIds: Map<string, unknown>;
};

eventEmitterForFuzzHarness.on("clientCreate", (client) => {
const channel = client.channel as TrackableSharedArray;
channel.insertIds = new Set<string>();
channel.moveIds = new Set<string>();
channel.insertIds = new Map<string, unknown>();
channel.moveIds = new Map<string, string>();
channel.toggleIds = new Map<string, unknown>();
Comment on lines +119 to +121
Copy link
Contributor

@anthony-murphy anthony-murphy Sep 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i wonder if we can simplify this, potentially in a follow up, and just keep a single set of ids, and randomly try operations, or only keep track of ids not in sharedarray.get, so basically removed ids. i suggest this, as i'm a little worried about the stress test becoming over structured, which can negatively affect coverage. Stress is especially good for catching hard to predict scenarios, and over-structuring stress can reduce its ability to find those hard to predict scenarios.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this also seems related to the todo you have in the pr description

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. And precisely this is related to the todo.


// Register listener to track insert entry IDs
channel.on("valueChanged", (op, _isLocal, _target) => {
if (op.type === OperationType.insertEntry) {
const entryId = op.entryId;
channel.insertIds.add(entryId);
}
if (op.type === OperationType.moveEntry) {
const entryId = op.entryId;
channel.moveIds.add(entryId);
switch (op.type) {
case OperationType.insertEntry: {
const entryId = op.entryId;
channel.insertIds.set(entryId, op.value);
break;
}
case OperationType.deleteEntry: {
const entryId = op.entryId;
channel.insertIds.delete(entryId);
channel.moveIds.delete(entryId);
break;
}
case OperationType.moveEntry: {
if (channel.insertIds.has(op.entryId)) {
channel.insertIds.set(op.changedToEntryId, channel.insertIds.get(op.entryId));
channel.insertIds.delete(op.entryId);
channel.moveIds.set(op.entryId, op.changedToEntryId);
}
break;
}
case OperationType.toggle: {
if (channel.insertIds.has(op.entryId)) {
channel.toggleIds.set(op.entryId, channel.insertIds.get(op.entryId));
channel.insertIds.delete(op.entryId);
channel.moveIds.delete(op.entryId);
} else {
channel.insertIds.set(op.entryId, channel.toggleIds.get(op.entryId));
channel.toggleIds.delete(op.entryId);
}
break;
}
case OperationType.toggleMove: {
channel.insertIds.set(op.entryId, channel.insertIds.get(op.changedToEntryId));
channel.insertIds.delete(op.changedToEntryId);
channel.moveIds.delete(op.changedToEntryId);
channel.moveIds.set(op.changedToEntryId, op.entryId);
break;
}
default: {
unreachableCase(op);
}
}
});
});
Expand Down Expand Up @@ -179,19 +224,28 @@ export function makeSharedArrayOperationGenerator(weights: {
const deleteOp = ({
random,
client,
}: DDSFuzzTestState<SharedArrayFactory<string>>): SharedArrayDelete => ({
type: "delete",
index: random.integer(0, Math.max(0, client.channel.get().length - 1)),
});
}: DDSFuzzTestState<SharedArrayFactory<string>>): SharedArrayDelete => {
const index = random.integer(0, Math.max(0, client.channel.get().length - 1));
return {
type: "delete",
index,
DEBUG_value: client.channel.get()[index],
};
};

const moveOp = ({
random,
client,
}: DDSFuzzTestState<SharedArrayFactory<string>>): SharedArrayMove => ({
type: "move",
oldIndex: random.integer(0, Math.max(0, client.channel.get().length - 1)),
newIndex: random.integer(0, Math.max(0, client.channel.get().length)),
});
}: DDSFuzzTestState<SharedArrayFactory<string>>): SharedArrayMove => {
const oldIndex = random.integer(0, Math.max(0, client.channel.get().length - 1));
const newIndex = random.integer(0, Math.max(0, client.channel.get().length));
return {
type: "move",
oldIndex,
newIndex,
DEBUG_value: client.channel.get()[oldIndex],
};
};

const insertBulkAfterOp = ({
random,
Expand All @@ -213,7 +267,7 @@ export function makeSharedArrayOperationGenerator(weights: {
client,
}: DDSFuzzTestState<SharedArrayFactory<string>>): SharedArrayToggle => {
const sharedArray = client.channel as TrackableSharedArray;
const entryIds = [...sharedArray.insertIds];
const entryIds = [...sharedArray.insertIds.keys()];
if (entryIds.length === 0) {
throw new Error("No entryIds found for toggle operation");
}
Expand All @@ -224,6 +278,7 @@ export function makeSharedArrayOperationGenerator(weights: {
return {
type: "toggle",
entryId,
DEBUG_value: sharedArray.insertIds.get(entryId),
};
};

Expand All @@ -232,19 +287,21 @@ export function makeSharedArrayOperationGenerator(weights: {
client,
}: DDSFuzzTestState<SharedArrayFactory<string>>): SharedArrayToggleMove => {
const sharedArray = client.channel as TrackableSharedArray;
const entryIds = [...sharedArray.moveIds];
const oldEntryId = entryIds[random.integer(0, Math.max(0, entryIds.length - 1))];
const entryIds = [...sharedArray.moveIds.keys()];
const index = random.integer(0, Math.max(0, entryIds.length - 1));
const oldEntryId = entryIds[index];
if (oldEntryId === undefined) {
throw new Error("No old entryId found for toggleMove operation");
}
const newEntryId = entryIds[random.integer(0, Math.max(0, entryIds.length - 1))];
const newEntryId = sharedArray.moveIds.get(oldEntryId);
if (newEntryId === undefined) {
throw new Error("No new entryId found for toggleMove operation");
}
return {
type: "toggleMove",
oldEntryId,
newEntryId,
DEBUG_value: sharedArray.insertIds.get(newEntryId),
};
};

Expand Down Expand Up @@ -285,7 +342,7 @@ export function makeSharedArrayOperationGenerator(weights: {
[moveOp, weights.move, hasNonzeroLength],
[insertBulkAfterOp, weights.insertBulkAfter, hasNonzeroLength],
[toggleOp, weights.toggle, hasEnoughInsertLength],
// [toggleMoveOp, weights.toggleMove, hasEnoughMoveLength],
[toggleMoveOp, weights.toggleMove, hasEnoughMoveLength],
]);

return async (state: DDSFuzzTestState<SharedArrayFactory<string>>) => {
Expand Down Expand Up @@ -315,8 +372,8 @@ export const baseSharedArrayModel: DDSFuzzModel<
delete: 3,
move: 3,
insertBulkAfter: 1,
toggle: 0,
toggleMove: 0,
toggle: 1,
toggleMove: 1,
}),
),
reducer: makeSharedArrayReducer<string>(),
Expand Down
Loading