Skip to content

Commit

Permalink
Fix dehydrate tests (attempt #2): Use proper versions of DDS, DataSto…
Browse files Browse the repository at this point in the history
…re, runtime, Loader in versioned tests (#20650)

An extraction from #20621

Reason for changes
These tests explode when I change interface between DDS & DataStore

Description
These tests are not written correctly. They mix arbitrary version of DDS with the latest version of FluidDataStoreRuntime (through TestFluidObjectFactory). We never supported such mixes - version of DDS has to match version or data store!

Using matching TestFluidObjectFactory is easy, but then we start running into somewhat similar problem of mixing FluidDataStoreRuntime & ContainerRuntime. We support only N/N-1 mixes here, and nothing more. So we have to use the version provided by test framework, thus you see some dance above CodeLoader to get right factory.

And while we are at it, it would be great for the test to use Loader version provided by test framework, not latest version.
This breaks for old versions as the way test is written it works only for latest structure of the document. Previously test framework was running a ton of combos, but most of them were actually the same, as even when it was supplied different versions of loader, we were using only latest version of loader. So, the right fix here - use loader version provided by framework but skip all the tests that use old version of the loader, thus getting same result, but with fewer iterations (and faster tests).

As part of fixing it, use proper back-compat compatible ways to get to entry points at loader & runtime layers.
  • Loading branch information
vladsud authored Apr 16, 2024
1 parent caae7f1 commit 442cb76
Showing 1 changed file with 67 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,13 @@ import {
LoaderContainerTracker,
LocalCodeLoader,
TestFluidObject,
TestFluidObjectFactory,
createDocumentId,
getContainerEntryPointBackCompat,
getDataStoreEntryPointBackCompat,
} from "@fluidframework/test-utils/internal";
import { createDataStoreFactory } from "@fluidframework/runtime-utils/internal";
import * as semver from "semver";
import { pkgVersion } from "../packageVersion.js";

// eslint-disable-next-line import/no-internal-modules
import type { SnapshotWithBlobs } from "../../../../loader/container-loader/lib/serializedStateManager.js";
Expand Down Expand Up @@ -197,15 +200,17 @@ describeCompat(
async function createDetachedContainerAndGetEntryPoint() {
const container: IContainer = await loader.createDetachedContainer(codeDetails);
// Get the root dataStore from the detached container.
const defaultDataStore = (await container.getEntryPoint()) as TestFluidObject;
const defaultDataStore =
await getContainerEntryPointBackCompat<TestFluidObject>(container);
return {
container,
defaultDataStore,
};
}

function createTestLoader(): Loader {
const factory: TestFluidObjectFactory = new TestFluidObjectFactory([
// It's important to use data store runtime of the same version as DDSs!
const factory = new apis.dataRuntime.TestFluidObjectFactory([
[sharedStringId, SharedString.getFactory()],
[sharedMapId, SharedMap.getFactory()],
[crcId, ConsensusRegisterCollection.getFactory()],
Expand All @@ -216,8 +221,22 @@ describeCompat(
[sparseMatrixId, SparseMatrix.getFactory()],
[sharedCounterId, SharedCounter.getFactory()],
]);
const codeLoader = new LocalCodeLoader([[codeDetails, factory]], {});
const testLoader = new Loader({

// This dance is to ensure that we get reasonable version of ContainerRuntime.
// If we do not set IRuntimeFactory property, LocalCodeLoader will use ContainerRuntime from current version
// We only support limited (N/N-1) compatibility for container runtime and data stores, so that will not work.
// Use version supplied by test framework
const defaultFactory = createDataStoreFactory("default", factory);
(defaultFactory as any).IRuntimeFactory =
new apis.containerRuntime.ContainerRuntimeFactoryWithDefaultDataStore({
defaultFactory,
registryEntries: [[defaultFactory.type, Promise.resolve(defaultFactory)]],
runtimeOptions: {},
});
const codeLoader = new LocalCodeLoader([[codeDetails, defaultFactory]], {});

// Use Loader supplied by test framework.
const testLoader = new apis.loader.Loader({
urlResolver: provider.urlResolver,
documentServiceFactory: provider.documentServiceFactory,
codeLoader,
Expand All @@ -229,15 +248,16 @@ describeCompat(

const createPeerDataStore = async (containerRuntime: IContainerRuntimeBase) => {
const dataStore = await containerRuntime.createDataStore(["default"]);
const peerDataStore = (await dataStore.entryPoint.get()) as ITestFluidObject;
const peerDataStore =
await getDataStoreEntryPointBackCompat<ITestFluidObject>(dataStore);
return {
peerDataStore,
peerDataStoreRuntimeChannel: peerDataStore.channel,
};
};

async function getDataObjectFromContainer(container: IContainer, key: string) {
const entryPoint = (await container.getEntryPoint()) as TestFluidObject;
const entryPoint = await getContainerEntryPointBackCompat<TestFluidObject>(container);
const handle: IFluidHandle<TestFluidObject> | undefined = entryPoint.root.get(key);
assert(handle !== undefined, `handle for [${key}] must exist`);
return handle.get();
Expand All @@ -255,8 +275,16 @@ describeCompat(
beforeEach("createLoader", async function () {
provider = getTestObjectProvider();
if (
semver.compare(provider.driver.version, "0.46.0") === -1 &&
(provider.driver.type === "routerlicious" || provider.driver.type === "tinylicious")
// These tests use dedicated (same) version loader, container runtime, DDSs.
// Thus there is no value in running more pairs that are essentially exactly the same as other tests.
provider.type === "TestObjectProviderWithVersionedLoad" ||
// These tests only work with the latest version of loader -
// they do make certain assumptions that are not valid for older loaders. This check could be relaxed in
// the future.
apis.loader.version !== pkgVersion ||
(semver.compare(provider.driver.version, "0.46.0") === -1 &&
(provider.driver.type === "routerlicious" ||
provider.driver.type === "tinylicious"))
) {
this.skip();
}
Expand Down Expand Up @@ -399,9 +427,9 @@ describeCompat(
await loader.rehydrateDetachedContainerFromSnapshot(snapshotTree);

// Check for default data store
const entryPoint = await container2.getEntryPoint();
assert.notStrictEqual(entryPoint, undefined, "Component should exist!!");
const defaultDataStore = entryPoint as TestFluidObject;
const defaultDataStore =
await getContainerEntryPointBackCompat<TestFluidObject>(container2);
assert.notStrictEqual(defaultDataStore, undefined, "Component should exist!!");

// Check for dds
const sharedMap = await defaultDataStore.getSharedObject<ISharedMap>(sharedMapId);
Expand Down Expand Up @@ -451,9 +479,9 @@ describeCompat(
await container2.attach(request);

// Check for default data store
const entryPoint = await container2.getEntryPoint();
assert.notStrictEqual(entryPoint, undefined, "Component should exist!!");
const defaultDataStore = entryPoint as TestFluidObject;
const defaultDataStore =
await getContainerEntryPointBackCompat<TestFluidObject>(container2);
assert.notStrictEqual(defaultDataStore, undefined, "Component should exist!!");

// Check for dds
const sharedMap = await defaultDataStore.getSharedObject<ISharedMap>(sharedMapId);
Expand Down Expand Up @@ -502,9 +530,9 @@ describeCompat(
}

// Check for default data store
const entryPoint = await container1.getEntryPoint();
assert.notStrictEqual(entryPoint, undefined, "Component should exist!!");
const defaultDataStore = entryPoint as TestFluidObject;
const defaultDataStore =
await getContainerEntryPointBackCompat<TestFluidObject>(container1);
assert.notStrictEqual(defaultDataStore, undefined, "Component should exist!!");

// Check for dds
const sharedMap = await defaultDataStore.getSharedObject<ISharedMap>(sharedMapId);
Expand Down Expand Up @@ -548,7 +576,8 @@ describeCompat(
const { container } = await createDetachedContainerAndGetEntryPoint();

const snapshotTree = container.serialize();
const defaultDataStore = (await container.getEntryPoint()) as TestFluidObject;
const defaultDataStore =
await getContainerEntryPointBackCompat<TestFluidObject>(container);
assert(
defaultDataStore.context.storage !== undefined,
"Storage should be present in detached data store",
Expand All @@ -564,7 +593,8 @@ describeCompat(

const container2: IContainer =
await loader.rehydrateDetachedContainerFromSnapshot(snapshotTree);
const defaultDataStore2 = (await container2.getEntryPoint()) as TestFluidObject;
const defaultDataStore2 =
await getContainerEntryPointBackCompat<TestFluidObject>(container2);
assert(
defaultDataStore2.context.storage !== undefined,
"Storage should be present in rehydrated data store",
Expand All @@ -582,7 +612,8 @@ describeCompat(
it("Change contents of dds, then rehydrate and then check summary", async () => {
const { container } = await createDetachedContainerAndGetEntryPoint();

const defaultDataStoreBefore = (await container.getEntryPoint()) as TestFluidObject;
const defaultDataStoreBefore =
await getContainerEntryPointBackCompat<TestFluidObject>(container);
const sharedStringBefore =
await defaultDataStoreBefore.getSharedObject<SharedString>(sharedStringId);
const intervalsBefore = sharedStringBefore.getIntervalCollection("intervals");
Expand Down Expand Up @@ -651,7 +682,8 @@ describeCompat(
const container2 =
await loader.rehydrateDetachedContainerFromSnapshot(snapshotTree);

const defaultComponentAfter = (await container2.getEntryPoint()) as TestFluidObject;
const defaultComponentAfter =
await getContainerEntryPointBackCompat<TestFluidObject>(container2);
const sharedStringAfter =
await defaultComponentAfter.getSharedObject<SharedString>(sharedStringId);
const intervalsAfter = sharedStringAfter.getIntervalCollection("intervals");
Expand Down Expand Up @@ -703,7 +735,8 @@ describeCompat(
it("Rehydrate container from summary, change contents of dds and then check summary", async () => {
const { container } = await createDetachedContainerAndGetEntryPoint();
let str = "AA";
const defaultComponent1 = (await container.getEntryPoint()) as TestFluidObject;
const defaultComponent1 =
await getContainerEntryPointBackCompat<TestFluidObject>(container);
const sharedString1 =
await defaultComponent1.getSharedObject<SharedString>(sharedStringId);
sharedString1.insertText(0, str);
Expand All @@ -712,7 +745,7 @@ describeCompat(
const container2 =
await loader.rehydrateDetachedContainerFromSnapshot(snapshotTree);
const defaultDataStoreBefore =
(await container2.getEntryPoint()) as TestFluidObject;
await getContainerEntryPointBackCompat<TestFluidObject>(container2);
const sharedStringBefore =
await defaultDataStoreBefore.getSharedObject<SharedString>(sharedStringId);
const sharedMapBefore =
Expand All @@ -722,7 +755,8 @@ describeCompat(
sharedMapBefore.set("0", str);

await container2.attach(request);
const defaultComponentAfter = (await container.getEntryPoint()) as TestFluidObject;
const defaultComponentAfter =
await getContainerEntryPointBackCompat<TestFluidObject>(container);
const sharedStringAfter =
await defaultComponentAfter.getSharedObject<SharedString>(sharedStringId);
const sharedMapAfter =
Expand Down Expand Up @@ -879,7 +913,7 @@ describeCompat(
await loader.rehydrateDetachedContainerFromSnapshot(snapshotTree);

const rehydratedEntryPoint =
(await rehydratedContainer.getEntryPoint()) as TestFluidObject;
await getContainerEntryPointBackCompat<TestFluidObject>(rehydratedContainer);
const rehydratedRootOfDataStore =
await rehydratedEntryPoint.getSharedObject<ISharedMap>(sharedMapId);
const dataStore2Handle: IFluidHandle<TestFluidObject> | undefined =
Expand Down Expand Up @@ -915,7 +949,7 @@ describeCompat(
await loader.rehydrateDetachedContainerFromSnapshot(snapshotTree);

const rehydratedEntryPoint =
(await rehydratedContainer.getEntryPoint()) as TestFluidObject;
await getContainerEntryPointBackCompat<TestFluidObject>(rehydratedContainer);
const rootOfDds2 =
await rehydratedEntryPoint.getSharedObject<ISharedMap>(sharedMapId);
const dds2Handle: IFluidHandle<ISharedMap> | undefined = rootOfDds2.get(dds2Key);
Expand Down Expand Up @@ -958,7 +992,9 @@ describeCompat(
await loader.rehydrateDetachedContainerFromSnapshot(snapshotTree);

const rehydratedEntryPoint =
(await rehydratedContainer.getEntryPoint()) as TestFluidObject;
await getContainerEntryPointBackCompat<TestFluidObject>(
rehydratedContainer,
);
const rootOfDds2 =
await rehydratedEntryPoint.getSharedObject<ISharedMap>(sharedMapId);
const dds2Handle: IFluidHandle<ISharedMap> | undefined =
Expand Down Expand Up @@ -1020,7 +1056,9 @@ describeCompat(
await loader.rehydrateDetachedContainerFromSnapshot(snapshotTree);

const rehydratedEntryPoint =
(await rehydratedContainer.getEntryPoint()) as TestFluidObject;
await getContainerEntryPointBackCompat<TestFluidObject>(
rehydratedContainer,
);
const rehydratedRootOfDataStore2 =
await rehydratedEntryPoint.getSharedObject<ISharedMap>(sharedMapId);
const dataStore2Handle: IFluidHandle<TestFluidObject> | undefined =
Expand Down

0 comments on commit 442cb76

Please sign in to comment.