Skip to content

Commit

Permalink
Revert "eslint(test-end-to-end-tests): Prefix test-end-to-end-tests b…
Browse files Browse the repository at this point in the history 
…efore enabling no-unchecked-record-access" (#23498)

Reverts #23436

1. Where `?` was used to address linter defect, TypeScript appears to
mostly ignore that these cases may lead to `undefined` result which is
not accepted per type specifications. Use of `?` is thus a behavior
change that will shift point of failure away from where it could first
be detected - revert those behavior changes. (Many of the test uses of
`?` in original change are permissible as there is a follow-up assertion
that will fail. But those were not separated during revert.)

2. Where `T | undefined` was used to address linter defect, TypeScript
will narrow without `undefined` without `noUncheckedIndexAccess`
enabled. Thus, the code appears more confusing as there is a
non-respected type annotation.
  • Loading branch information
@jason-ha
jason-ha authored Jan 14, 2025
1 parent 6f083c3 commit 26cb010
Show file tree
Hide file tree
Showing 21 changed files with 114 additions and 181 deletions.
4 changes: 2 additions & 2 deletions packages/test/test-end-to-end-tests/src/mocking.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export function wrapObjectAndOverride<T extends Record<string, any>>(
): T {
return new Proxy(obj, {
get: (target: T, property: string, r) => {
const override = overrides?.[property as keyof T];
const override = overrides[property as keyof T];
// check if the current property has an override
if (override) {
// check if the override is a function, which means it is factory
Expand All @@ -35,7 +35,7 @@ export function wrapObjectAndOverride<T extends Record<string, any>>(
// it is an object which nests more overrides, so
// get the property from the passed in object,
// so we can proxy nested overrides to it
const real = target?.[property as keyof T];
const real = target[property as keyof T];
// if the real property is a function, we'll
// call it, so whatever it returns can have
// the nested overrides applied to it
Expand Down
20 changes: 7 additions & 13 deletions packages/test/test-end-to-end-tests/src/test/benchmark/summarization.memory.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,7 @@ import {
ContainerRuntime,
DefaultSummaryConfiguration,
} from "@fluidframework/container-runtime/internal";
import {
ISummaryBlob,
SummaryType,
type SummaryObject,
} from "@fluidframework/driver-definitions";
import { ISummaryBlob, SummaryType } from "@fluidframework/driver-definitions";
import { channelsTreeName } from "@fluidframework/runtime-definitions/internal";
import {
ITestContainerConfig,
Expand Down Expand Up @@ -87,7 +83,7 @@ describeCompat("Summarization - runtime benchmarks", "NoCompat", (getTestObjectP
summary.tree[".metadata"]?.type === SummaryType.Blob,
"Expected .metadata blob in summary root.",
);
const metadata = readBlobContent(summary.tree[".metadata"]?.content) as Record<
const metadata = readBlobContent(summary.tree[".metadata"].content) as Record<
string,
unknown
>;
Expand All @@ -100,14 +96,13 @@ describeCompat("Summarization - runtime benchmarks", "NoCompat", (getTestObjectP
"Unexpected metadata blob disableIsolatedChannels",
);

const channelsTree: SummaryObject | undefined = summary.tree[channelsTreeName];
const channelsTree = summary.tree[channelsTreeName];
assert(
channelsTree?.type === SummaryType.Tree,
"Expected .channels tree in summary root.",
);

const defaultDataStoreNode: SummaryObject | undefined =
channelsTree.tree[defaultDataStore._context.id];
const defaultDataStoreNode = channelsTree.tree[defaultDataStore._context.id];
assert(
defaultDataStoreNode?.type === SummaryType.Tree,
"Expected default data store tree in summary.",
Expand All @@ -117,10 +112,9 @@ describeCompat("Summarization - runtime benchmarks", "NoCompat", (getTestObjectP
defaultDataStoreNode.tree[".component"]?.type === SummaryType.Blob,
"Expected .component blob in default data store summary tree.",
);
const dataStoreChannelsTree: SummaryObject | undefined =
defaultDataStoreNode.tree[channelsTreeName];
const dataStoreChannelsTree = defaultDataStoreNode.tree[channelsTreeName];
const attributes = readBlobContent(
defaultDataStoreNode.tree[".component"]?.content,
defaultDataStoreNode.tree[".component"].content,
) as Record<string, unknown>;
assert(
attributes.snapshotFormatVersion === undefined,
Expand All @@ -139,7 +133,7 @@ describeCompat("Summarization - runtime benchmarks", "NoCompat", (getTestObjectP
"Expected .channels tree in default data store.",
);

const defaultDdsNode: SummaryObject | undefined = dataStoreChannelsTree.tree.root;
const defaultDdsNode = dataStoreChannelsTree.tree.root;
assert(
defaultDdsNode?.type === SummaryType.Tree,
"Expected default root DDS in summary.",
Expand Down
20 changes: 7 additions & 13 deletions packages/test/test-end-to-end-tests/src/test/benchmark/summarization.time.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,7 @@ import {
ContainerRuntime,
DefaultSummaryConfiguration,
} from "@fluidframework/container-runtime/internal";
import {
ISummaryBlob,
SummaryType,
type SummaryObject,
} from "@fluidframework/driver-definitions";
import { ISummaryBlob, SummaryType } from "@fluidframework/driver-definitions";
import { channelsTreeName } from "@fluidframework/runtime-definitions/internal";
import {
ITestContainerConfig,
Expand Down Expand Up @@ -86,7 +82,7 @@ describeCompat("Summarization - runtime benchmarks", "NoCompat", (getTestObjectP
summary.tree[".metadata"]?.type === SummaryType.Blob,
"Expected .metadata blob in summary root.",
);
const metadata = readBlobContent(summary.tree[".metadata"]?.content) as Record<
const metadata = readBlobContent(summary.tree[".metadata"].content) as Record<
string,
unknown
>;
Expand All @@ -99,14 +95,13 @@ describeCompat("Summarization - runtime benchmarks", "NoCompat", (getTestObjectP
"Unexpected metadata blob disableIsolatedChannels",
);

const channelsTree: SummaryObject | undefined = summary.tree[channelsTreeName];
const channelsTree = summary.tree[channelsTreeName];
assert(
channelsTree?.type === SummaryType.Tree,
"Expected .channels tree in summary root.",
);

const defaultDataStoreNode: SummaryObject | undefined =
channelsTree.tree[defaultDataStore._context.id];
const defaultDataStoreNode = channelsTree.tree[defaultDataStore._context.id];
assert(
defaultDataStoreNode?.type === SummaryType.Tree,
"Expected default data store tree in summary.",
Expand All @@ -116,10 +111,9 @@ describeCompat("Summarization - runtime benchmarks", "NoCompat", (getTestObjectP
defaultDataStoreNode.tree[".component"]?.type === SummaryType.Blob,
"Expected .component blob in default data store summary tree.",
);
const dataStoreChannelsTree: SummaryObject | undefined =
defaultDataStoreNode.tree[channelsTreeName];
const dataStoreChannelsTree = defaultDataStoreNode.tree[channelsTreeName];
const attributes = readBlobContent(
defaultDataStoreNode.tree[".component"]?.content,
defaultDataStoreNode.tree[".component"].content,
) as Record<string, unknown>;
assert(
attributes.snapshotFormatVersion === undefined,
Expand All @@ -138,7 +132,7 @@ describeCompat("Summarization - runtime benchmarks", "NoCompat", (getTestObjectP
"Expected .channels tree in default data store.",
);

const defaultDdsNode: SummaryObject | undefined = dataStoreChannelsTree.tree.root;
const defaultDdsNode = dataStoreChannelsTree.tree.root;
assert(
defaultDdsNode?.type === SummaryType.Tree,
"Expected default root DDS in summary.",
Expand Down
66 changes: 25 additions & 41 deletions packages/test/test-end-to-end-tests/src/test/data-virtualization/groupIdInSummary.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,7 @@ import type {
IFluidHandle,
} from "@fluidframework/core-interfaces";
import { SummaryType } from "@fluidframework/driver-definitions";
import type {
ISnapshot,
ISnapshotTree,
SummaryObject,
} from "@fluidframework/driver-definitions/internal";
import type { ISnapshot, ISnapshotTree } from "@fluidframework/driver-definitions/internal";
import { getSnapshotTree } from "@fluidframework/driver-utils/internal";
import type { IFluidDataStoreContext } from "@fluidframework/runtime-definitions/internal";
import {
Expand Down Expand Up @@ -267,10 +263,10 @@ describeCompat(
);
await provider.ensureSynchronized();
const { summaryVersion, summaryTree } = await summarizeNow(summarizer);
const channelsTree: SummaryObject | undefined = summaryTree.tree[".channels"];
const channelsTree = summaryTree.tree[".channels"];
assert(channelsTree.type === SummaryType.Tree, "channels should be a tree");
const dataObjectTreeA: SummaryObject | undefined = channelsTree.tree[dataObjectA.id];
const dataObjectTreeB: SummaryObject | undefined = channelsTree.tree[dataObjectB.id];
const dataObjectTreeA = channelsTree.tree[dataObjectA.id];
const dataObjectTreeB = channelsTree.tree[dataObjectB.id];
assert(dataObjectTreeA !== undefined, "dataObjectTree should exist");
assert(dataObjectTreeA.type === SummaryType.Tree, "dataObjectTree should be a tree");
assert(
Expand Down Expand Up @@ -349,10 +345,10 @@ describeCompat(
);
await provider.ensureSynchronized();
const { summaryVersion, summaryTree } = await summarizeNow(summarizer);
const channelsTree: SummaryObject | undefined = summaryTree.tree[".channels"];
const channelsTree = summaryTree.tree[".channels"];
assert(channelsTree.type === SummaryType.Tree, "channels should be a tree");
const dataObjectTreeA: SummaryObject | undefined = channelsTree.tree[dataObjectA.id];
const dataObjectTreeB: SummaryObject | undefined = channelsTree.tree[dataObjectB.id];
const dataObjectTreeA = channelsTree.tree[dataObjectA.id];
const dataObjectTreeB = channelsTree.tree[dataObjectB.id];
assert(dataObjectTreeA !== undefined, "dataObjectTree should exist");
assert(dataObjectTreeA.type === SummaryType.Tree, "dataObjectTree should be a tree");
assert(
Expand Down Expand Up @@ -518,13 +514,12 @@ describeCompat(

const blobContents = loadingSnapshot.blobContents;
// Snapshot validation (a snapshot call with NO loadingGroupIds)
const channelsTree: ISnapshotTree | undefined =
loadingSnapshot.snapshotTree.trees[".channels"];
const mainObjectTree: ISnapshotTree | undefined = channelsTree.trees[mainObject.id];
const dataObjectATree: ISnapshotTree | undefined = channelsTree.trees[dataObjectA.id];
const dataObjectBTree: ISnapshotTree | undefined = channelsTree.trees[dataObjectB.id];
const dataObjectCTree: ISnapshotTree | undefined = channelsTree.trees[dataObjectC.id];
const dataObjectDTree: ISnapshotTree | undefined = channelsTree.trees[dataObjectD.id];
const channelsTree = loadingSnapshot.snapshotTree.trees[".channels"];
const mainObjectTree = channelsTree.trees[mainObject.id];
const dataObjectATree = channelsTree.trees[dataObjectA.id];
const dataObjectBTree = channelsTree.trees[dataObjectB.id];
const dataObjectCTree = channelsTree.trees[dataObjectC.id];
const dataObjectDTree = channelsTree.trees[dataObjectD.id];

assertPopulatedTree(mainObjectTree, noId, blobContents, "mainObject tree not right");
assertOmittedBlobContents(
Expand Down Expand Up @@ -646,17 +641,12 @@ describeCompat(
);

// Snapshot validation (a snapshot call for loadingGroupIds = [loadingGroupId])
const channelsTree2: ISnapshotTree | undefined =
groupSnapshot.snapshotTree.trees[".channels"];
const mainObjectTree2: ISnapshotTree | undefined = channelsTree2.trees[mainObject.id];
const dataObjectATree2: ISnapshotTree | undefined =
channelsTree2.trees[dataObjectA.id];
const dataObjectBTree2: ISnapshotTree | undefined =
channelsTree2.trees[dataObjectB.id];
const dataObjectCTree2: ISnapshotTree | undefined =
channelsTree2.trees[dataObjectC.id];
const dataObjectDTree2: ISnapshotTree | undefined =
channelsTree2.trees[dataObjectD.id];
const channelsTree2 = groupSnapshot.snapshotTree.trees[".channels"];
const mainObjectTree2 = channelsTree2.trees[mainObject.id];
const dataObjectATree2 = channelsTree2.trees[dataObjectA.id];
const dataObjectBTree2 = channelsTree2.trees[dataObjectB.id];
const dataObjectCTree2 = channelsTree2.trees[dataObjectC.id];
const dataObjectDTree2 = channelsTree2.trees[dataObjectD.id];

assertOmittedTree(mainObjectTree2, noId, blobContents, "mainObject tree incorrect");
assertPopulatedTree(
Expand Down Expand Up @@ -700,18 +690,12 @@ describeCompat(
assert(group2Snapshot !== undefined, "should have captured group2 snapshot!");
const blobContents = group2Snapshot.blobContents;
assert.deepEqual(group2Snapshot.sequenceNumber, summaryRefSeq, "Unexpected snapshot");
const channels2Tree2: ISnapshotTree | undefined =
group2Snapshot.snapshotTree.trees[".channels"];
const mainObject2Tree2: ISnapshotTree | undefined =
channels2Tree2.trees[mainObject.id];
const dataObjectA2Tree2: ISnapshotTree | undefined =
channels2Tree2.trees[dataObjectA.id];
const dataObjectB2Tree2: ISnapshotTree | undefined =
channels2Tree2.trees[dataObjectB.id];
const dataObjectC2Tree2: ISnapshotTree | undefined =
channels2Tree2.trees[dataObjectC.id];
const dataObjectD2Tree2: ISnapshotTree | undefined =
channels2Tree2.trees[dataObjectD.id];
const channels2Tree2 = group2Snapshot.snapshotTree.trees[".channels"];
const mainObject2Tree2 = channels2Tree2.trees[mainObject.id];
const dataObjectA2Tree2 = channels2Tree2.trees[dataObjectA.id];
const dataObjectB2Tree2 = channels2Tree2.trees[dataObjectB.id];
const dataObjectC2Tree2 = channels2Tree2.trees[dataObjectC.id];
const dataObjectD2Tree2 = channels2Tree2.trees[dataObjectD.id];

assertOmittedTree(
mainObject2Tree2,
Expand Down
2 changes: 1 addition & 1 deletion .../test/test-end-to-end-tests/src/test/data-virtualization/loadNewerGroupIdSnapshot.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ describeCompat(
if (isGroupIdLoaderVersion(apis.loader.version)) {
const groupSnapshot = await snapshotADeferred.promise;
const snapshotTreeA =
groupSnapshot.snapshotTree.trees[".channels"]?.trees[dataObjectA2.id];
groupSnapshot.snapshotTree.trees[".channels"].trees[dataObjectA2.id];
assertPopulatedGroupIdTree(
snapshotTreeA,
groupSnapshot.blobContents,
Expand Down
8 changes: 4 additions & 4 deletions packages/test/test-end-to-end-tests/src/test/deRehydrateContainerTests.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ describeCompat(
SparseMatrix,
} = apis.dds;
function assertSubtree(tree: ISnapshotTree, key: string, msg?: string): ISnapshotTree {
const subTree: ISnapshotTree | undefined = tree.trees[key];
const subTree = tree.trees[key];
assert(subTree, msg ?? `${key} subtree not present`);
return subTree;
}
Expand All @@ -162,9 +162,9 @@ describeCompat(
blobs: ISerializableBlobContents,
key: string,
): T {
const id: string | undefined = subtree.blobs[key];
const id = subtree.blobs[key];
assert(id, `blob id for ${key} missing`);
const contents: string | undefined = blobs[id];
const contents = blobs[id];

assert(contents, `blob contents for ${key} missing`);
return JSON.parse(contents) as T;
Expand Down Expand Up @@ -318,7 +318,7 @@ describeCompat(
);

// Check blobs contents for protocolAttributes
const protocolAttributesBlobId = baseSnapshot.trees[".protocol"]?.blobs.attributes;
const protocolAttributesBlobId = baseSnapshot.trees[".protocol"].blobs.attributes;
assert(
snapshotBlobs[protocolAttributesBlobId] !== undefined,
"Blobs should contain attributes blob",
Expand Down
18 changes: 7 additions & 11 deletions packages/test/test-end-to-end-tests/src/test/gc/gcDataVirtualization.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,7 @@ import {
import { IContainer, LoaderHeader } from "@fluidframework/container-definitions/internal";
import type { ConfigTypes, IConfigProviderBase } from "@fluidframework/core-interfaces";
import { ISummaryTree, SummaryType } from "@fluidframework/driver-definitions";
import type {
ISnapshot,
ISnapshotTree,
SummaryObject,
} from "@fluidframework/driver-definitions/internal";
import type { ISnapshot } from "@fluidframework/driver-definitions/internal";
import {
createSummarizer,
ITestContainerConfig,
Expand Down Expand Up @@ -87,10 +83,10 @@ describeCompat("GC & Data Virtualization", "NoCompat", (getTestObjectProvider) =
};

function getDataStoreInSummaryTree(summaryTree: ISummaryTree, dataStoreId: string) {
const channelsTree: SummaryObject | undefined = summaryTree.tree[".channels"];
const channelsTree = summaryTree.tree[".channels"];
assert(channelsTree !== undefined, "Expected a .channels tree");
assert(channelsTree.type === SummaryType.Tree, "Expected a tree");
return channelsTree.tree?.[dataStoreId];
return channelsTree.tree[dataStoreId];
}

async function isDataStoreInSummaryTree(summaryTree: ISummaryTree, dataStoreId: string) {
Expand Down Expand Up @@ -168,7 +164,7 @@ describeCompat("GC & Data Virtualization", "NoCompat", (getTestObjectProvider) =
assert(callCount === 0, "Expected no snapshot call");
const gcState = getGCStateFromSummary(summaryTree);
assert(gcState !== undefined, "Expected GC state to be generated");
const gcNodeA = gcState.gcNodes?.[handleA.absolutePath];
const gcNodeA = gcState.gcNodes[handleA.absolutePath];
assert(gcNodeA !== undefined, "Data Store should exist on gc graph");
const unreferencedTimestampMs = gcNodeA.unreferencedTimestampMs;
assert(unreferencedTimestampMs !== undefined, "Data Store should be unreferenced");
Expand Down Expand Up @@ -201,8 +197,8 @@ describeCompat("GC & Data Virtualization", "NoCompat", (getTestObjectProvider) =
assert(snapshotCaptured !== undefined, "Expected snapshot to be captured");

// Validate that we loaded the snapshot without datastoreA on the snapshot
const tree = (snapshotCaptured as ISnapshot).snapshotTree.trees[".channels"]?.trees;
const datastoreATree: ISnapshotTree | undefined = tree[dataStoreId];
const tree = (snapshotCaptured as ISnapshot).snapshotTree.trees[".channels"].trees;
const datastoreATree = tree[dataStoreId];
assert(datastoreATree !== undefined, "DataStoreA should be in the snapshot");

// Summarize and verify datastoreA is still unreferenced
Expand All @@ -214,7 +210,7 @@ describeCompat("GC & Data Virtualization", "NoCompat", (getTestObjectProvider) =
assert(callCount === 0, "Expected no snapshot call");
const gcState2 = getGCStateFromSummary(summaryTree2);
assert(gcState2 !== undefined, "Expected GC state to be generated");
const gcNodeA2 = gcState2.gcNodes?.[handleA.absolutePath];
const gcNodeA2 = gcState2.gcNodes[handleA.absolutePath];
assert(gcNodeA2 !== undefined, "DataStoreA should exist on gc graph");
assert(
gcNodeA2.unreferencedTimestampMs === unreferencedTimestampMs,
Expand Down
4 changes: 2 additions & 2 deletions packages/test/test-end-to-end-tests/src/test/gc/gcDatastoreAliased.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ describeCompat("GC Data Store Aliased Full Compat", "FullCompat", (getTestObject
const gcStatePreAlias = getGCStateFromSummary(summaryWithStats.summary);
assert(gcStatePreAlias !== undefined, "Should get gc pre state from summary!");
assert(
gcStatePreAlias.gcNodes?.[toFluidHandleInternal(dataObject2.handle).absolutePath]
gcStatePreAlias.gcNodes[toFluidHandleInternal(dataObject2.handle).absolutePath]
.unreferencedTimestampMs !== undefined,
"dataStore2 should be unreferenced as it is not aliased and not root!",
);
Expand All @@ -95,7 +95,7 @@ describeCompat("GC Data Store Aliased Full Compat", "FullCompat", (getTestObject
const gcStatePostAlias = getGCStateFromSummary(summaryWithStats.summary);
assert(gcStatePostAlias !== undefined, "Should get gc post state from summary!");
assert(
gcStatePostAlias.gcNodes?.[toFluidHandleInternal(dataObject2.handle).absolutePath]
gcStatePostAlias.gcNodes[toFluidHandleInternal(dataObject2.handle).absolutePath]
.unreferencedTimestampMs === undefined,
"dataStore2 should be referenced as it is aliased and thus a root datastore!",
);
Expand Down
10 changes: 3 additions & 7 deletions packages/test/test-end-to-end-tests/src/test/gc/gcDatastoreDuplicateRoutes.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,7 @@ import { IContainer } from "@fluidframework/container-definitions/internal";
import { ISummarizer } from "@fluidframework/container-runtime/internal";
// eslint-disable-next-line import/no-internal-modules
import { IGarbageCollectionState } from "@fluidframework/container-runtime/internal/test/gc";
import {
ISummaryBlob,
SummaryType,
type SummaryObject,
} from "@fluidframework/driver-definitions";
import { ISummaryBlob, SummaryType } from "@fluidframework/driver-definitions";
import { gcBlobPrefix, gcTreeKey } from "@fluidframework/runtime-definitions/internal";
import {
ITestObjectProvider,
Expand Down Expand Up @@ -69,7 +65,7 @@ describeCompat("GC Data Store Duplicates", "NoCompat", (getTestObjectProvider, a
summaryResult.summaryVersion,
);
summaryResult = await waitForSummary(summarizer2);
const gcObject: SummaryObject | undefined = summaryResult.summaryTree.tree[gcTreeKey];
const gcObject = summaryResult.summaryTree.tree[gcTreeKey];
assert(gcObject !== undefined, "Expected a gc blob!");
assert(gcObject.type === SummaryType.Handle, "Expected a handle!");
assert(gcObject.handleType === SummaryType.Tree, "Expected a gc tree handle!");
Expand Down Expand Up @@ -100,7 +96,7 @@ describeCompat("GC Data Store Duplicates", "NoCompat", (getTestObjectProvider, a

// Get GC State
summaryResult = await waitForSummary(summarizer2);
const gcTree: SummaryObject | undefined = summaryResult.summaryTree.tree[gcTreeKey];
const gcTree = summaryResult.summaryTree.tree[gcTreeKey];
assert(gcTree?.type === SummaryType.Tree, "Expected a Tree!");
const gcBlob = gcTree.tree[`${gcBlobPrefix}_root`] as ISummaryBlob;
const gcState = JSON.parse(gcBlob.content as string) as IGarbageCollectionState;
Expand Down
Loading

0 comments on commit 26cb010

Please sign in to comment.