Skip to content

improvement(test-version-utils): Automatically set minVersionForCollab in e2e test generation #24809

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 7 commits into from
Jun 20, 2025
+59 −113
Merged

improvement(test-version-utils): Automatically set minVersionForCollab in e2e test generation #24809

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
87d0cb9
use minVersionForCollab in e2e tests
scottn12 Jun 10, 2025
2c138dc
pr feedback
scottn12 Jun 11, 2025
24355ed
add comments for test changes
scottn12 Jun 16, 2025
f93ceff
Merge branch 'main' into minVFCE2E
scottn12 Jun 17, 2025
52bbadb
rename assertValidMinVersionForCollab
scottn12 Jun 20, 2025
4883825
Merge branch 'main' into minVFCE2E
scottn12 Jun 20, 2025
8921d24
change assertValidMinVersionForCollab to throw error
scottn12 Jun 20, 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
1 change: 1 addition & 0 deletions packages/test/test-end-to-end-tests/src/test/blobs.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ function makeTestContainerConfig(
createBlobPayloadPending,
},
registry,
minVersionForCollab: "2.40.0",
};
}

Expand Down
20 changes: 13 additions & 7 deletions packages/test/test-end-to-end-tests/src/test/compression.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
CompressionAlgorithms,
type IContainerRuntimeOptions,
type IContainerRuntimeOptionsInternal,
type MinimumVersionForCollab,
} from "@fluidframework/container-runtime/internal";
// TODO:AB#6558: This should be provided based on the compatibility configuration.
// eslint-disable-next-line @typescript-eslint/no-restricted-imports
Expand Down Expand Up @@ -57,11 +58,13 @@ const compressionSuite = (getProvider, apis?) => {

async function setupContainers(
runtimeOptions: IContainerRuntimeOptionsInternal = defaultRuntimeOptions,
minVersionForCollab: MinimumVersionForCollab | undefined = undefined,
) {
const containerConfig: ITestContainerConfig = {
registry: [["mapKey", SharedMap.getFactory()]],
runtimeOptions,
fluidDataObjectType: DataObjectFactoryType.Test,
minVersionForCollab,
};
const localContainer = await provider.makeTestContainer(containerConfig);
localDataObject =
Expand Down Expand Up @@ -132,14 +135,17 @@ const compressionSuite = (getProvider, apis?) => {
) {
this.skip();
}
await setupContainers({
compressionOptions: {
minimumBatchSizeInBytes: option.compression ? 10 : Number.POSITIVE_INFINITY,
compressionAlgorithm: CompressionAlgorithms.lz4,
await setupContainers(
{
compressionOptions: {
minimumBatchSizeInBytes: option.compression ? 10 : Number.POSITIVE_INFINITY,
compressionAlgorithm: CompressionAlgorithms.lz4,
},
chunkSizeInBytes: option.chunking ? 100 : Number.POSITIVE_INFINITY,
enableGroupedBatching: option.grouping,
},
chunkSizeInBytes: option.chunking ? 100 : Number.POSITIVE_INFINITY,
enableGroupedBatching: option.grouping,
});
"2.0.0", // minVersionForCollab
);
const values = [
generateRandomStringOfSize(100),
generateRandomStringOfSize(100),
Expand Down
1 change: 1 addition & 0 deletions packages/test/test-end-to-end-tests/src/test/containerRuntime.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ describeCompat(
enableGroupedBatching: compression, // Compression w/o grouping is not supported
chunkSizeInBytes: chunking ? 200 : Infinity,
},
minVersionForCollab: "2.0.0",
};
const container = await provider.makeTestContainer(options);
entry = await getEntryPoint(container);
Expand Down
1 change: 1 addition & 0 deletions packages/test/test-end-to-end-tests/src/test/offline/stashedOps.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1851,6 +1851,7 @@ describeCompat(
state: "disabled",
},
},
enableRuntimeIdCompressor: "on",
},
loaderProps: {
configProvider: configProvider({
Expand Down
159 changes: 46 additions & 113 deletions packages/test/test-version-utils/src/compatUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,7 @@
import { mixinAttributor } from "@fluid-experimental/attributor";
import { TestDriverTypes } from "@fluid-internal/test-driver-definitions";
import { FluidTestDriverConfig, createFluidTestDriver } from "@fluid-private/test-drivers";
import {
DefaultSummaryConfiguration,
CompressionAlgorithms,
disabledCompressionConfig,
ICompressionRuntimeOptions,
type IContainerRuntimeOptionsInternal,
} from "@fluidframework/container-runtime/internal";
import type { MinimumVersionForCollab } from "@fluidframework/container-runtime/internal";
import { FluidObject, IFluidLoadable, IRequest } from "@fluidframework/core-interfaces";
import { IFluidHandleContext } from "@fluidframework/core-interfaces/internal";
import { assert, unreachableCase } from "@fluidframework/core-utils/internal";
Expand Down Expand Up @@ -52,98 +46,37 @@ import { getRequestedVersion } from "./versionUtils.js";
export const TestDataObjectType = "@fluid-example/test-dataStore";

/**
* This function modifies container runtime options according to a version of runtime used.
* If a version of runtime does not support some options, they are removed.
* If a version runtime supports some options, such options are enabled to increase a chance of
* hitting feature set controlled by such options, and thus increase chances of finding product bugs.
* Determines the minimumVersionForCollab that should be used for cross-client compatibility scenarios.
* We will use the lesser of the containerRuntimeVersion and containerRuntimeForLoadingVersion
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not apparent with containerRuntimeForLoadingVersion is. I would explain the parameters more in the comment. Maybe even give an example to make it clear. These are complicated concepts so the easier we can make it to understand, the better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, updated in latest

*
* @param version - a version of container runtime to be used in test
* @param optionsArg - input runtime options (optional)
* @returns - runtime options that should be used with a given version of container runtime
* @internal
* Note: The MinimumVersionForCollab returned will only be used if a minVersionForCollab was not provided
* in the ITestContainerConfig object.
*/
function filterRuntimeOptionsForVersion(
version: string,
optionsArg: IContainerRuntimeOptionsInternal = {
summaryOptions: {
summaryConfigOverrides: {
...DefaultSummaryConfiguration,
...{
initialSummarizerDelayMs: 0,
},
},
},
},
driverType: TestDriverTypes,
) {
let options = { ...optionsArg };

// No test fails with this option, it allows us to validate properly expectations and
// implementation of services
options.loadSequenceNumberVerification = "close";

const compressorDisabled: ICompressionRuntimeOptions = {
minimumBatchSizeInBytes: Number.POSITIVE_INFINITY,
compressionAlgorithm: CompressionAlgorithms.lz4,
};

// These is the "maximum" config.
const {
compressionOptions = options.enableGroupedBatching === false
? disabledCompressionConfig
: {
minimumBatchSizeInBytes: 200,
compressionAlgorithm: CompressionAlgorithms.lz4,
},
enableGroupedBatching = true,
enableRuntimeIdCompressor = "on",
// Some t9s tests timeout with small settings. This is likely due to too many ops going through.
// Reduce chunking cut-off for such tests.
chunkSizeInBytes = driverType === "local" ? 200 : 1000,
} = options;

if (version.startsWith("1.")) {
options = {
// None of these features are supported by 1.3
compressionOptions: disabledCompressionConfig,
enableGroupedBatching: false,
enableRuntimeIdCompressor: undefined,
// Enable chunking.
// We need to ensure that 1.x documents (that use chunking) can still be opened by 2.x.
// This options does nothing for 2.x builds as chunking is only enabled if compression is enabled.
chunkSizeInBytes,
...options,
};
} else if (version.startsWith("2.0.0-rc.1.")) {
options = {
compressionOptions: compressorDisabled, // Can't use compression, need https://github.com/microsoft/FluidFramework/pull/20111 fix
enableGroupedBatching,
enableRuntimeIdCompressor: undefined, // it was boolean in RC1, switched to enum in RC2
chunkSizeInBytes: Number.POSITIVE_INFINITY, // disabled, need https://github.com/microsoft/FluidFramework/pull/20115 fix
...options,
};
} else if (version.startsWith("2.0.0-rc.2.")) {
options = {
compressionOptions: compressorDisabled, // Can't use compression, need https://github.com/microsoft/FluidFramework/pull/20111 fix
enableGroupedBatching,
// control over schema was generalized in RC3 - see https://github.com/microsoft/FluidFramework/pull/20174
// IdCompressor settings moved around - can't enable them across versions without tripping on asserts
enableRuntimeIdCompressor: undefined,
chunkSizeInBytes: Number.POSITIVE_INFINITY, // disabled, need https://github.com/microsoft/FluidFramework/pull/20115 fix
...options,
};
} else {
// "2.0.0-rc.3." ++
options = {
compressionOptions,
enableGroupedBatching,
chunkSizeInBytes,
enableRuntimeIdCompressor,
...options,
};
function getMinVersionForCollab(
containerRuntimeVersion: string,
containerRuntimeForLoadingVersion: string | undefined,
): MinimumVersionForCollab {
isMinimumVersionForCollab(containerRuntimeVersion);
if (containerRuntimeForLoadingVersion === undefined) {
// If `containerRuntimeForLoading` is not defined, then this is not a cross-client compat scenario.
// In this case, we can use the `containerRuntimeVersion` as the default minVersionForCollab.
return containerRuntimeVersion;
}
isMinimumVersionForCollab(containerRuntimeForLoadingVersion);
// If `containerRuntimeForLoading` is defined, we will use the lower of the two versions to ensure
// compatibility between the two runtimes.
return semver.compare(containerRuntimeVersion, containerRuntimeForLoadingVersion) <= 0
? containerRuntimeVersion
: containerRuntimeForLoadingVersion;
}

return options;
/**
* Asserts the given version is valid semver and is type MinimumVersionForCollab.
*/
function isMinimumVersionForCollab(
version: string,
): asserts version is MinimumVersionForCollab {
assert(semver.valid(version) !== null, "version must be valid semver");
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be enough to use node's assert here instead of our custom one? I think this code doesn't end up in any production bundle so assert tagging doesn't matter.

Also, I think maybe the function wants to be called assertValidVersionForCollab or something along those lines? For an is<Something> function I think it's preferrable to always return true or false when we know that's actually the case (instead of only ever being able to return true or throwing). And the way we use this function seems to be more a "this should be true or bust", not really a "handle each scenario differently".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, I think maybe the function wants to be called assertValidVersionForCollab or something along those lines?

Good call, will switch the name

Would it be enough to use node's assert here instead of our custom one? I think this code doesn't end up in any production bundle so assert tagging doesn't matter.

The reason I added a function was to avoid casting as MinimumVersionForCollab. The check I added in assertValidMinVersionForCollab doesn't really promise that the version inputted is type MinimumVersionForCollab (just that it's valid semver). IMO it will be fine since all the versions coming in are controlled from our version generation tools, but I thought it would be nice to have assertValidMinVersionForCollab setup so if we wanted to add more validation/guardrails in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, having the typeguard seems good to me as well, my point is about using assert from core-utils vs the native one from node (or simply an if (<is null>) { throw new Error("version must be valid semver")}. With the current assert, the error message will be replaced with a hex code during assert tagging for the next release, and in the scenarios where this function gets called (our pipelines, and I believe that's it?) I think outputting a hex code instead of the string directly is a con with no benefit.

Copy link
Contributor Author

@scottn12 scottn12 Jun 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Ah I see what you meant now and I agree. Updated in latest

}

/**
Expand Down Expand Up @@ -246,12 +179,12 @@ export async function getVersionedTestObjectProviderFromApis(
return new factoryCtor(
TestDataObjectType,
dataStoreFactory,
filterRuntimeOptionsForVersion(
apis.containerRuntime.version,
containerOptions?.runtimeOptions,
type,
),
containerOptions?.minVersionForCollab,
containerOptions?.runtimeOptions,
containerOptions?.minVersionForCollab ??
getMinVersionForCollab(
apis.containerRuntime.version,
apis.containerRuntimeForLoading?.version,
),
);
};

Expand Down Expand Up @@ -359,12 +292,12 @@ export async function getCompatVersionedTestObjectProviderFromApis(
return new factoryCtor(
TestDataObjectType,
dataStoreFactory,
filterRuntimeOptionsForVersion(
minVersion,
containerOptions?.runtimeOptions,
driverConfig.type,
),
containerOptions?.minVersionForCollab,
containerOptions?.runtimeOptions,
containerOptions?.minVersionForCollab ??
getMinVersionForCollab(
apis.containerRuntime.version,
apis.containerRuntimeForLoading?.version,
),
[innerRequestHandler],
);
};
Expand All @@ -384,12 +317,12 @@ export async function getCompatVersionedTestObjectProviderFromApis(
return new factoryCtor(
TestDataObjectType,
dataStoreFactory,
filterRuntimeOptionsForVersion(
minVersion,
containerOptions?.runtimeOptions,
driverConfig.type,
),
containerOptions?.minVersionForCollab,
containerOptions?.runtimeOptions,
containerOptions?.minVersionForCollab ??
getMinVersionForCollab(
apis.containerRuntime.version,
apis.containerRuntimeForLoading.version,
),
[innerRequestHandler],
);
};
Expand Down
Loading