Skip to content

Conversation

@scottn12
Copy link
Contributor

@scottn12 scottn12 commented Nov 6, 2025

Description

This PR enables fuzz testing for the ConsensusOrderedCollection DDS.

applyStashedOp()

This PR also changes the applyStashedOp implementation from throwing "not implemented" to an empty implementation. This allows fuzz testing in rehydration scenarios as well as local server stress tests scenarios. This is also in-line with ConsensusRegisterCollection.

Alternatively, we can leave the original applyStashedOp implementation and disable rehydration scenarios in fuzz testing. However, this will prevent us from integrating ConsensusOrderedCollection into local server stress tests.

Misc

AB#52361

@github-actions github-actions bot added base: main PRs targeted against main branch area: dds Issues related to distributed data structures dependencies Pull requests that update a dependency file labels Nov 6, 2025
@scottn12 scottn12 marked this pull request as ready for review November 7, 2025 14:48
Copilot AI review requested due to automatic review settings November 7, 2025 14:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request adds fuzz testing infrastructure for the ConsensusOrderedCollection DDS. The implementation follows the established pattern used in other DDS packages within the repository.

Key changes:

  • Adds fuzz testing utilities and test suite for ConsensusOrderedCollection
  • Implements test infrastructure with operation generators and reducers for fuzz testing
  • Updates applyStashedOp method to support stashing operations in fuzz tests

Reviewed Changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pnpm-lock.yaml Adds dependency on @fluid-private/stochastic-test-utils for fuzz testing
package.json Adds @fluid-private/stochastic-test-utils as a dev dependency
fuzzUtils.ts New file implementing fuzz test utilities including operation generators, reducers, and validation logic
dirname.cts New file exporting __dirname for ESM compatibility
consensusOrderedCollection.fuzz.spec.ts New file defining fuzz test suites with and without rebasing
consensusOrderedCollection.ts Changes applyStashedOp from throwing error to empty implementation for stashing support
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported


protected applyStashedOp(): void {
throw new Error("not implemented");
// empty implementation
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this just for testing purposes? Or is nothing actually the right thing to do? I vaguely recall something about this, but I could be misremembering.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This helpful for local server stress tests (not in this PR yet) since we can't easily disable rehydration scenarios, so throwing an error would prevent us from integrating this DDS into those tests.

I believe throwing/empty implementation should be correct. acquire/release/complete ops depend on the distributed state (i.e. jobTracking, acquireId) that are lost/changed during rehydration. In theory, we may be able to apply stashed add ops since they do not rely on the distributed state, but dropping them should also be okay since they were not ack'd, so the item was never added and we will stay consistent with the distributed state.

The other consensus-based DDSes also don't apply stashed ops (either throw or have an empty implementation), so this would be consistent with those DDSes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: dds Issues related to distributed data structures base: main PRs targeted against main branch dependencies Pull requests that update a dependency file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants