Skip to content

Fix broken TinyliciousClient test and ensure tinylicious tests run in CI #25344

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+23 −26
Open

Fix broken TinyliciousClient test and ensure tinylicious tests run in CI #25344

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
038c532
Fix broken TinyliciousClient test and ensure it runs in CI
ChumpChief Aug 28, 2025
62a4fe5
Also add test:realsvc:tinylicious:report to other packages that have …
ChumpChief Aug 28, 2025
801577f
Match azure-client scripts to tinylicious-client
ChumpChief Aug 29, 2025
dae75da
Alphabetize
ChumpChief Aug 29, 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/framework/client-logger/fluid-telemetry/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@
"test:mocha:esm:unit": "mocha --recursive \"lib/test/**/*.spec.js\"",
"test:realsvc": "npm run test:realsvc:tinylicious",
"test:realsvc:tinylicious": "start-server-and-test start:tinylicious:test 7070 test:realsvc:tinylicious:run",
"test:realsvc:tinylicious:report": "npm run test:realsvc:tinylicious",
"test:realsvc:tinylicious:run": "npm run test:mocha:end-to-end",
"tsc": "fluid-tsc commonjs --project ./tsconfig.cjs.json && copyfiles -f ../../../../common/build/build-common/src/cjs/package.json ./dist",
"typetests:gen": "flub generate typetests --dir . -v",
Expand Down
5 changes: 3 additions & 2 deletions packages/service-clients/azure-client/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,9 @@
"start:tinylicious:test": "npx @fluidframework/azure-local-service > tinylicious.log 2>&1",
"test": "npm run test:realsvc",
"test:realsvc": "npm run test:realsvc:tinylicious",
"test:realsvc:local:run": "mocha --recursive \"lib/test/**/*.spec.*js\" --timeout 10000",
"test:realsvc:tinylicious": "start-server-and-test start:tinylicious:test 7070 test:realsvc:local:run",
"test:realsvc:tinylicious:report": "npm run test:realsvc:tinylicious",
"test:realsvc:tinylicious": "start-server-and-test start:tinylicious:test 7070 test:realsvc:tinylicious:run",
"test:realsvc:tinylicious:run": "mocha --recursive \"lib/test/**/*.spec.*js\" --exit",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without --exit, the tests hang at completion. This implies something isn't getting cleaned up so it's not great, but this at least gets the tests running again.

"tsc": "fluid-tsc commonjs --project ./tsconfig.cjs.json && copyfiles -f ../../../common/build/build-common/src/cjs/package.json ./dist",
"typetests:gen": "flub generate typetests --dir . -v",
"typetests:prepare": "flub typetests --dir . --reset --previous --normalize"
Expand Down
1 change: 1 addition & 0 deletions packages/service-clients/tinylicious-client/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
"test": "npm run test:realsvc",
"test:realsvc": "npm run test:realsvc:tinylicious",
"test:realsvc:tinylicious": "start-server-and-test start:tinylicious:test 7070 test:realsvc:tinylicious:run",
"test:realsvc:tinylicious:report": "npm run test:realsvc:tinylicious",
"test:realsvc:tinylicious:run": "mocha --recursive \"lib/test/**/*.spec.*js\" --exit",
"tsc": "fluid-tsc commonjs --project ./tsconfig.cjs.json && copyfiles -f ../../../common/build/build-common/src/cjs/package.json ./dist",
"typetests:gen": "flub generate typetests --dir . -v",
Expand Down
42 changes: 18 additions & 24 deletions packages/service-clients/tinylicious-client/src/test/TinyliciousClient.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@

/* eslint-disable @typescript-eslint/no-explicit-any */

/* eslint-disable @typescript-eslint/no-unsafe-argument */

/* eslint-disable @typescript-eslint/no-unsafe-assignment */

/* eslint-disable @typescript-eslint/no-unsafe-member-access */
Expand All @@ -30,32 +28,26 @@ import { TinyliciousClient } from "../index.js";

import { TestDataObject } from "./TestDataObject.js";

const corruptedAliasOp = async (
runtime: IContainerRuntime,
alias: string,
): Promise<boolean | Error> =>
new Promise<boolean>((resolve, reject) => {
runtime.once("dispose", () => reject(new Error("Runtime disposed")));
(runtime as any).submit(ContainerMessageType.Alias, { id: alias }, resolve);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

processAliasMessages internally invokes the localOpMetadata to resolve its own promise, which I assume is the only way this would have worked in the past? Though I don't see how it could have gotten to that point before erroring out as a malformed op. In any case, I don't think the test needs to await that resolution to remain valid.

}).catch((error) => new Error(error.message));
const corruptedAliasOp = (runtime: IContainerRuntime, alias: string): void => {
(runtime as any).submit({ type: ContainerMessageType.Alias, contents: { id: alias } });
Copy link
Preview

Copilot AI Aug 28, 2025

Choose a reason for hiding this comment

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

The submit method call appears to be using an incorrect API signature. The original code used separate parameters for message type and contents, but this change combines them into a single object. Verify this matches the expected IContainerRuntime.submit API signature.

Suggested change
(runtime as any).submit({ type: ContainerMessageType.Alias, contents: { id: alias } });
(runtime as any).submit(ContainerMessageType.Alias, { id: alias });

Copilot uses AI. Check for mistakes.

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 is one of the ways in which the test was broken, which changed in #16276

};

const runtimeOf = (dataObject: TestDataObject): IContainerRuntime =>
(dataObject as any).context.containerRuntime as IContainerRuntime;

const connectionModeOf = (container: IFluidContainer): ConnectionMode =>
(container as any).container.connectionMode as ConnectionMode;

const allDataCorruption = async (containers: IFluidContainer[]): Promise<boolean> =>
Promise.all(
containers.map(
async (c) =>
new Promise<boolean>((resolve) =>
c.once("disposed", (error) => {
resolve(error?.errorType === ContainerErrorTypes.dataCorruptionError);
}),
),
),
).then((all) => !all.includes(false));
const waitForDataCorruption = async (container: IFluidContainer): Promise<void> =>
new Promise<void>((resolve, reject) =>
container.once("disposed", (error) => {
if (error?.errorType === ContainerErrorTypes.dataCorruptionError) {
resolve();
} else {
reject(error);
}
}),
);

for (const compatibilityMode of ["1", "2"] as const) {
describe(`TinyliciousClient (compatibilityMode: ${compatibilityMode})`, function () {
Expand Down Expand Up @@ -362,9 +354,11 @@ for (const compatibilityMode of ["1", "2"] as const) {
});

const do1 = createFluidContainer.initialObjects.do1;
const dataCorruption = allDataCorruption([createFluidContainer]);
await corruptedAliasOp(runtimeOf(do1), "alias");
assert(await dataCorruption);
const dataCorruptionP = waitForDataCorruption(createFluidContainer);
corruptedAliasOp(runtimeOf(do1), "alias");
// dataCorruptionP resolves if the container disposes with the expected error, rejects
// if it disposes with any other error.
await dataCorruptionP;
});

/**
Expand Down
Loading