diff --git a/TODO/investigation.md b/TODO/investigation.md new file mode 100644 index 000000000000..c139d893f1b8 --- /dev/null +++ b/TODO/investigation.md @@ -0,0 +1,391 @@ +# Investigation: Why mocha tests need --exit flag + +## Summary of Root Causes Found + +### Root Cause 1: SnapshotRefresher 24-hour timer +**Affected packages:** packages/loader/container-loader + +`SnapshotRefresher` creates a 24-hour (86400000ms) `setTimeout` via `Timer.setLongTimeout`. +Tests created `SerializedStateManager` instances without calling `.dispose()` after tests. + +**Fix:** Added `afterEach` cleanup to dispose each `SerializedStateManager` instance. + +### Root Cause 2: DeliLambda.readClientIdleTimer (60s setInterval) +**Affected packages:** All packages using LocalDeltaConnectionServer directly or via TestTreeProvider/LocalServerTestDriver + +`DeliLambda` creates a 60-second `setInterval` (readClientIdleTimer) when the local orderer +is set up. Only cleared when `DeliLambda.close()` is called via `LocalDeltaConnectionServer.close()`. + +**Fixes applied:** +- `packages/test/test-drivers/src/localServerTestDriver.ts`: Made `dispose()` async, awaiting `server.close()` +- `packages/test/test-driver-definitions/src/interfaces.ts`: Updated `dispose()` to `void | Promise` +- `packages/test/test-version-utils/src/describeCompat.ts` + `describeWithVersions.ts`: Await `driver.dispose()` in `after` hook +- `packages/test/local-server-tests/src/test/*.spec.ts`: All files changed to `deltaConnectionServer.close()` (instead of just `webSocketServer.close()`) +- `packages/test/test-end-to-end-tests/src/test/dataStoresNested.spec.ts`: Hoisted `driver` to outer scope, added `await driver.dispose()` in `afterEach` + +### Root Cause 3: GC timers in ContainerRuntime +**Affected packages:** packages/runtime/container-runtime + +`GarbageCollector` creates a MAX_INT32 timer on construction. Tests creating `ContainerRuntime` +instances without disposing them left this timer alive. + +**Fix:** Dispose all `ContainerRuntime` instances in test cleanup. + +### Root Cause 4: ApplicationInsights diagnosticLogInterval +**Affected packages:** packages/framework/client-logger/fluid-telemetry + +ApplicationInsights SDK creates a `setInterval` (10s) for internal log polling, even without +calling `initialize()`. Stop it with `appInsightsClient.stopPollingInternalLogs?.()`. + +### Root Cause 5: Quill requires document at ESM module-load time +**Affected packages:** examples/data-objects/inventory-app + +`@fluidframework/react` exports Quill-dependent code, which needs `document` at import time. +The fix is a `globalSetup.ts` file that calls `globalJsdom()` at module load time (before test +files are loaded). Named with 'g' so it sorts before 'i' (inventoryApp.test.js) alphabetically. + +## All Fixed Packages + +| Package | Config.exit removed | Root Cause | +|---------|---------------------|-----------| +| packages/loader/container-loader | ✅ | 1 - SnapshotRefresher | +| packages/runtime/container-runtime | ✅ | 3 - GC timer | +| packages/dds/tree | ✅ | 8, 9 - same container-runtime fix applies | +| packages/test/local-server-tests | ✅ | 2 - DeliLambda | +| examples/data-objects/table-document | ✅ | 2 - describeCompat driver await | +| packages/framework/client-logger/fluid-telemetry | ✅ | 4 - ApplicationInsights | +| packages/framework/react | ✅ | 7 - JSDOM timers + IFluidContainer.dispose() | +| packages/test/snapshots | ✅ | 10 - worker threads + container dispose leaks | +| examples/data-objects/webflow | ✅ | 2 - describeCompat/WithVersions missing provider.reset() + esm-loader-css removed | +| packages/service-clients/odsp-client | ✅ | Already clean | +| examples/data-objects/inventory-app | ✅ | 5 - Quill/JSDOM | +| experimental/PropertyDDS/packages/property-dds | ✅ | 6 - Container GC timer not disposed | +| experimental/dds/tree | ✅ | 8, 9 - SummaryManager + Summarizer timer leaks | +| experimental/PropertyDDS/packages/property-common | ✅ | Already clean | +| experimental/PropertyDDS/packages/property-properties | ✅ | Already clean | +| packages/dds/matrix/src/test/memory | ✅ | Already clean (benchmark) | +| packages/dds/sequence/src/test/memory | ✅ | Already clean (benchmark) | +| packages/dds/map/src/test/memory | ✅ | Already clean (benchmark) | +| packages/test/test-end-to-end-tests | ✅ | 2, 11a-d, 12, 13, 14, 15, 16, 17 - multiple fixes | +| packages/test/mocha-test-setup | ✅ | 16 - global.setTimeout patch for N-1 timer unref | +| packages/test/test-end-to-end-tests/benchmark | ✅ | Already clean (benchmark) | +| packages/test/local-server-stress-tests | ✅ | Already clean - harness disposes properly | + +## Root Cause 11d: describeInstallVersions after hook throws when before hook fails + +**Affected packages:** packages/test/test-end-to-end-tests (legacy chunking tests) + +When the `before("Create TestObjectProvider")` hook in `describeInstallVersions` fails (e.g. due +to a package installation error for old versions like 0.56.0 that are incompatible with modern +Node.js), `provider` remains `undefined`. The `after("Cleanup TestObjectProvider")` hook was +throwing `"Expected provider to be set up by before hook"` in that case, producing a spurious +test failure. + +**Fix:** Changed the `after` hook to `return` early instead of `throw` when `provider === undefined`. + +**File changed:** `packages/test/test-version-utils/src/describeWithVersions.ts` + +## Root Cause 11a: N-1 SummaryManager.dispose() not closing summarizer container +**Affected packages:** packages/test/test-end-to-end-tests (compat tests using old ContainerRuntime) + +When compat tests run with an N-1 `@fluidframework/container-runtime`, the older `SummaryManager.dispose()` +does NOT call `this.summarizer?.close()` (that fix was added in the current version). So when the parent +interactive container is disposed via `provider.reset()`, the summarizer container it had spawned remains +alive with its own `ContainerRuntime` instance holding a `GarbageCollector.sessionExpiryTimer` (MAX_INT32 +`setTimeout`), preventing mocha from exiting. + +**Fix:** In `LoaderContainerTracker.addContainer()`, non-interactive (summarizer) containers are now +tracked in a separate `trackedSummarizerContainers: Set` instead of being silently discarded. +In `reset()`, after disposing all interactive containers, the summarizer containers are explicitly disposed +too. This is idempotent for the current runtime version (where `SummaryManager.dispose()` already closes +the summarizer), but critical for N-1 compat scenarios. + +**File changed:** +`packages/test/test-utils/src/loaderContainerTracker.ts` + +## Root Cause 11b: TestObjectProviderWithVersionedLoad has two drivers; only one was disposed +**Affected packages:** packages/test/test-end-to-end-tests (compat tests) + +`TestObjectProviderWithVersionedLoad` (used for cross-client compat tests) maintains two separate +`LocalServerTestDriver` instances: `driverForCreating` and `driverForLoading`. Each driver has its own +`LocalDeltaConnectionServer` with a `DeliLambda.readClientIdleTimer` (60-second `setInterval`). + +The `describeCompat` and `describeWithVersions` `after` hooks previously called +`await provider.driver.dispose?.()`, which invokes the `driver` getter. That getter returns either +`driverForCreating` or `driverForLoading` based on the `useCreateApi` flag (set to `true` by `reset()`). +So only `driverForCreating` was ever disposed, leaving `driverForLoading`'s server and its 60-second +`setInterval` alive. + +**Fix 1:** Added `dispose(): Promise` to the `ITestObjectProvider` interface and implemented it in: +- `TestObjectProvider.dispose()`: delegates to `await this.driver.dispose?.()` (single driver) +- `TestObjectProviderWithVersionedLoad.dispose()`: explicitly disposes BOTH `driverForCreating` AND + `driverForLoading`, ensuring both `LocalDeltaConnectionServer` instances are closed. + +**Fix 2:** Changed `describeCompat.ts` and `describeWithVersions.ts` `after("Cleanup TestObjectProvider")` +hooks to call `await provider.dispose()` instead of `await provider.driver.dispose?.()`. + +**Files changed:** +- `packages/test/test-utils/src/testObjectProvider.ts` +- `packages/test/test-version-utils/src/describeCompat.ts` +- `packages/test/test-version-utils/src/describeWithVersions.ts` + +## Root Cause 11c: Local TestObjectProvider instances in container.spec.ts never reset +**Affected packages:** packages/test/test-end-to-end-tests + +Three tests in `container.spec.ts` created their own `TestObjectProvider` instances (with `new +TestObjectProvider(Loader, provider.driver, runtimeFactory)`) locally inside `it()` blocks, without +calling `.reset()` at the end. The containers tracked by these local providers were never disposed, +leaving their `GarbageCollector.sessionExpiryTimer` (MAX_INT32 `setTimeout`) running after the tests. + +**Affected tests:** +- "Delta manager receives readonly event when calling container.forceReadonly()" +- "getPendingLocalState() called on container" +- "can control op processing with connect() and disconnect()" + +**Fix:** Wrapped each test body in a `try/finally` block, calling `localTestObjectProvider.reset()` in +the `finally` clause to ensure cleanup even if the test throws. + +**File changed:** +`packages/test/test-end-to-end-tests/src/test/container.spec.ts` + +## Root Cause 16: N-1 compat Summarizer.runCore() timers (global.setTimeout patch) + +**Affected packages:** packages/test/test-end-to-end-tests (N-1 compat tests) + +N-1 packages (e.g. `@fluidframework/container-runtime@2.83.0`) create 2-minute `setTimeout` +timers inside `Summarizer.runCore()` `Promise.race()` calls. Root cause 9 fixed this in the +current version, but N-1 packages cannot be patched. These timers have no external cancellation +mechanism, so they keep the process alive for up to 2 minutes after tests complete. + +**Fix:** Patch `globalThis.setTimeout` in `packages/test/mocha-test-setup/src/mochaHooks.ts` +at module-load time to automatically call `unref()` on any timer with delay > 10,000ms. +`unref()` marks a Node.js timer as non-blocking for the event loop, so the process can exit +naturally after tests finish without waiting for those 2-minute timers to fire. + +**File changed:** +`packages/test/mocha-test-setup/src/mochaHooks.ts` + +## Root Cause 17: compression.spec.ts describeInstallVersions socket leak + +**Affected packages:** packages/test/test-end-to-end-tests + +The `describeInstallVersions` block called `compressionSuite` with a factory that created a +**new** `TestObjectProvider` (and a new `LocalDeltaConnectionServer` with OS socket connections) +on every `beforeEach` call. The `afterEach` in `compressionSuite` only calls `provider.reset()`, +never `provider.dispose()`, so the socket connections were never closed. + +After all 6 "Op Compression self-healing with old loader" tests completed, 2 OS socket handles +remained open indefinitely, hanging the process. This was NOT a timer issue (hence the +`global.setTimeout` patch alone was insufficient). + +**Fix:** Restructured the `describeInstallVersions` block to create the versioned provider +ONCE in a `before` hook and dispose it in an `after` hook. The provider is reused across all +tests in the suite (each test still gets clean state via `provider.reset()` in `afterEach`). + +**File changed:** +`packages/test/test-end-to-end-tests/src/test/compression.spec.ts` + +## Remaining (not fixed) + +- `tools/test-tools/.mocharc.cjs`: Standalone package (own pnpm workspace, not part of main monorepo). Tests appear to be trivially clean (just spawnSync) but can't verify without separate install. + +## Root Cause 6: Container not disposed after test (GC sessionExpiryTimer) +**Affected packages:** experimental/PropertyDDS/packages/property-dds + +When tests create containers via `TestObjectProvider.makeTestContainer()`/`loadTestContainer()` +or directly via `LocalDeltaConnectionServer`, calling `deltaConnectionServer.close()` alone +is NOT sufficient. The containers themselves have active `ContainerRuntime` instances with a +`GarbageCollector.sessionExpiryTimer` (MAX_INT32 timeout) that must be explicitly cleared +by disposing the containers. + +**Pattern:** `opProcessingController.reset()` calls `container.close()` + `container.dispose()` +on all tracked containers, which cascades to `ContainerRuntime.dispose()` → `GarbageCollector.dispose()` +→ `sessionExpiryTimer.clear()`. + +**Fix:** In `afterEach`, call `opProcessingController.reset()` BEFORE `deltaConnectionServer.close()`, +or call `objProvider.reset()` if using `TestObjectProvider`. + +## Key implementation notes + +### describeCompat/describeWithVersions after hook +The `after` hook now awaits `provider.dispose()` (NOT `provider.driver.dispose?.()`) so all +LocalDeltaConnectionServer instances are fully closed. `TestObjectProviderWithVersionedLoad` +has TWO drivers and overrides `dispose()` to close both. + +### localServerTestDriver.dispose() +Changed from fire-and-forget `void this._server.close()` to `await this._server.close()`. + +### Quill/JSDOM setup pattern +For packages that import @fluidframework/react (which transitively imports Quill): +- Create a `globalSetup.ts` in the test directory +- Name it to sort alphabetically BEFORE the test files (e.g., 'g' before 'i') +- Call `globalJsdom()` at module-load time (top level, not inside before()) +- Clean up in `before()` hook; individual tests can call `globalJsdom()` themselves + +### Memory/benchmark test mocharcs +The `@fluid-tools/benchmark` library has no `setInterval`/`setTimeout` timers. +Memory tests using this library exit cleanly without `--exit`. + +## Root Cause 7: JSDOM timers + IFluidContainer.dispose() not calling ContainerRuntime.dispose() +**Affected packages:** packages/framework/react + +Two issues: +1. **JSDOM requestAnimationFrame timers**: When tests call `globalJsdom()` and render React + components, JSDOM's `requestAnimationFrame` (implemented as recursive `setTimeout`) keeps the + process alive. Fix: call `jsdom.window.close()` (which calls `stopAllTimers()`) before + `cleanup()` in `after()` hooks. Note: `cleanup()` alone does NOT stop timers. + +2. **IFluidContainer.dispose() called container.close() instead of container.dispose()**: + `FluidContainer.dispose()` was calling `this.container.close()` which goes through `closeCore()` + and emits "closed" but does NOT call `disposeCore()`. Only `disposeCore()` calls + `this._runtime?.dispose()` which triggers `ContainerRuntime.dispose()` → + `GarbageCollector.dispose()` → `sessionExpiryTimer.clear()`. Fix: changed to + `this.container.dispose()` and added "disposed" event listener (in addition to "closed") + in `FluidContainer` constructor. + +**Files changed:** +- `packages/framework/fluid-static/src/fluidContainer.ts`: Call `container.dispose()` instead + of `container.close()` in `FluidContainer.dispose()`; also subscribe to "disposed" event +- `packages/framework/react/src/test/mochaHooks.ts`: Add `window.close()` before `cleanup()` +- `packages/framework/react/src/test/reactSharedTreeView.spec.tsx`: Wrap test in try/finally + with `container.dispose()`, add `window.close()` to DOM tests `after()` hook +- `packages/framework/react/src/test/{useObservation,useTree}.spec.tsx`: Add `window.close()` +- `packages/framework/react/src/test/text/textEditor.test.tsx`: Add `window.close()` + +## Notes for pending packages + +- There are still packages whose mocha tests hang. Tackle them one by one. +- For each: `pnpm build`, then `pnpm test:mocha`. Rebuild and re-test after every fix attempt. +- Use `.only` on specific test suites to identify exactly which ones are hanging. + +## Root Cause 8: Summarizer container GC timer not cleared after parent container disposed +**Affected packages:** experimental/dds/tree, packages/dds/tree + +When a parent (interactive) container is disposed, the summarizer container it had spawned via +`startSummarization()` was left alive. The summarizer container holds its own `ContainerRuntime` +with a `GarbageCollector.sessionExpiryTimer` (MAX_INT32 timeout) that kept the process alive. + +**Initial fix attempt (reverted):** Added `this.summarizer?.close()` in `SummaryManager.dispose()`. +This caused a regression: `close()` triggers an async chain that calls back into the (now-disposed) +interactive container's runtime, producing spurious "Runtime is closed" `ContainerClose` events +that failed the "Verify container telemetry" afterEach hook in the mixinSummaryHandler test. + +**Actual fix:** In `LoaderContainerTracker.addContainer()`, non-interactive (summarizer) containers +are tracked in `trackedSummarizerContainers`. In `reset()`, they are explicitly disposed via +`container.dispose()` (NOT `container.close()`, to avoid `ContainerClose` telemetry events). +This is the root cause 11a fix. It covers both N-1 compat and current version. + +**Files changed:** +`packages/test/test-utils/src/loaderContainerTracker.ts` + +## Root Cause 9: Summarizer.runCore() leaving dangling Promise.race() timers +**Affected packages:** experimental/dds/tree, packages/dds/tree + +`Summarizer.runCore()` uses two `Promise.race()` patterns with `setTimeout` for timeouts +(both 2 minutes). When the non-timeout promise wins the race, the losing `setTimeout` is +never cancelled, leaving it alive for 2 minutes and preventing mocha from exiting. + +- **Timer 1**: Race between `runCoordinatorCreateFn()` and a 2-minute coordinator creation + timeout. If the coordinator is created first, the 2-minute timer was never cleared. +- **Timer 2**: Race between `runningSummarizer.waitStop()` and a 2-minute stop timeout. + If `waitStop` completes first, the 2-minute timer was never cleared. + +**Fix:** Store the timeout IDs in variables and call `clearTimeout()` on them when +`Promise.race()` resolves (for Timer 1, clear it inside the non-timeout `.then()` callback; +for Timer 2, clear it unconditionally after `await Promise.race()`). + +**File changed:** +`packages/runtime/container-runtime/src/summary/summaryDelayLoadedModule/summarizer.ts` + +## Root Cause 10: Worker thread MessagePort + container dispose leaks (packages/test/snapshots) +**Affected packages:** packages/test/snapshots + +Three leaks: + +1. **Worker threads staying alive**: `processNode()` creates Worker threads to run `processOneNode()`. + After work completes, the worker posts "true" and the parent resolves the promise. BUT the worker + thread itself stays alive because its containers were closed via `container.close()` (not `dispose()`), + leaving GC sessionExpiryTimers (MAX_INT32) running in the worker. The live worker keeps its + `MessagePort` open in the parent thread, which holds a reference in the parent's event loop. + **Fix:** Call `worker.terminate()` immediately after the worker sends "true", proactively releasing + all worker resources. + +2. **validateSnapshots.ts containers not disposed**: In `Mode.BackCompat`, `validateSnapshots()` runs + in the main thread (not a worker) and calls `loadContainer()` + `uploadSummary()` but never disposes + the container. The container's GC sessionExpiryTimer (MAX_INT32) keeps the process alive. + **Fix:** Added `finally { container?.dispose(); }`. + +3. **LocalDeltaConnectionServer not closed in serialized.spec.ts**: Each test created its own + `LocalDeltaConnectionServer` (even though containers are detached and never connect). Also, + `LoaderContainerTracker` was never reset. + **Fix:** Collect servers in an array, add `after()` hook to close them and reset the tracker. + Note: detached containers don't create DeliLambda timers (orderer is only created on connection), + so these servers didn't actually cause the hang in practice, but cleaning them up is correct. + +**Files changed:** +- `packages/test/snapshots/src/replayMultipleFiles.ts`: `worker.terminate()` on success +- `packages/test/snapshots/src/validateSnapshots.ts`: `finally { container?.dispose() }` +- `packages/test/snapshots/src/test/serialized.spec.ts`: track servers + `after()` cleanup + +## Root Cause 12: NoopHeuristic timer not cleared on container close/dispose + +**Affected packages:** packages/loader/container-loader (and all packages with containers) + +`NoopHeuristic` holds a `Timer` (2000ms `setTimeout`) that fires after an op is processed if no +ops were sent by the client. Neither `container.ts`'s `closeCore()` nor `disposeCore()` cleared +this timer, so after tests, 400+ active `Timeout` objects were keeping the event loop alive. + +**Fix:** +- Added `dispose()` method to `NoopHeuristic` that calls `this.timer?.clear()` +- Added `this.timer?.clear()` to `notifyDisconnect()` (timer shouldn't keep the event loop alive after disconnect; will be restarted on reconnect when the next op arrives) +- In `container.ts` `closeCore()`: call `this.noopHeuristic?.dispose(); this.noopHeuristic = undefined` +- In `container.ts` `disposeCore()`: same cleanup (handles dispose-without-close-first) + +**Files changed:** +- `packages/loader/container-loader/src/noopHeuristic.ts` +- `packages/loader/container-loader/src/container.ts` + +## Root Cause 13: SummaryManager.delayBeforeCreatingSummarizer() timer not cancellable + +**Affected packages:** packages/runtime/container-runtime + +`SummaryManager.delayBeforeCreatingSummarizer()` creates a 5-second `setTimeout` to delay +summarizer creation after election. The timer was stored in a local variable inside the function +and was not accessible from `dispose()`. If the container was disposed while the delay was +running, the timer kept the event loop alive for up to 5 seconds. + +**Fix:** Promoted the timer to a class field `delayBeforeCreatingSummarizerTimer`. In `dispose()`, +cancel it with `clearTimeout()`. Also clear it after `Promise.race()` resolves (in case the +op-count branch resolved the promise without going through the timer's `clearTimeout` path). + +**File changed:** `packages/runtime/container-runtime/src/summary/summaryManager.ts` + +## Root Cause 14: ContainerRuntime.fetchLatestSnapshotAndMaybeClose() non-cancellable delay + +**Affected packages:** packages/runtime/container-runtime + +`ContainerRuntime.fetchLatestSnapshotAndMaybeClose()` used a `delay()` utility (5-second +`setTimeout`) to wait before closing the summarizer after fetching a snapshot. The `delay()` +call was not cancellable, so if the container was disposed mid-delay, the timer kept the event +loop alive. + +**Fix:** Replaced `await delay(ms)` with a cancellable promise pattern: stored the timer and +resolve function in `this.closeSummarizerDelayHandle`. In `dispose()`, cancel the timer and +resolve the promise early. Added a `_disposed` guard after the delay to skip the close logic +if the container was disposed during the wait. + +**File changed:** `packages/runtime/container-runtime/src/containerRuntime.ts` + +## Root Cause 15: noDeltaStream.spec.ts containers bypass LoaderContainerTracker + +**Affected packages:** packages/test/test-end-to-end-tests + +Several containers in `noDeltaStream.spec.ts` were created via `createLoader()` / direct loader +APIs outside of the `TestObjectProvider` factory methods. These containers bypassed the +`LoaderContainerTracker`, so they were never disposed by `provider.reset()`. Their +`GarbageCollector.sessionExpiryTimer` (MAX_INT32 `setTimeout`) kept the process alive. + +**Fix:** Wrapped container usage in `try/finally` blocks to call `container.close()` + +`container.dispose()` after each test. + +**File changed:** `packages/test/test-end-to-end-tests/src/test/noDeltaStream.spec.ts` diff --git a/TODO/tasks.md b/TODO/tasks.md new file mode 100644 index 000000000000..b30345dc449e --- /dev/null +++ b/TODO/tasks.md @@ -0,0 +1,33 @@ +## Completed + +[x] Fix property-dds tests hanging: added opProcessingController.reset() and objProvider.reset() + in afterEach hooks to dispose containers (clears GC sessionExpiryTimer). + Root cause: deltaConnectionServer.close() alone doesn't dispose containers. + +[x] Remove --exit from local-server-stress-tests: harness already properly disposes + containers and closes server in finally block. + +[x] Fix examples/data-objects/webflow - removed esm-loader-css from mocharc, changed layout.spec.ts + to import htmlFormatter directly (avoids CSS import chain), and fixed describeCompat/describeWithVersions + to call provider.reset() before driver.dispose() in cleanup hook. + +[x] Fix experimental/dds/tree - SummaryManager.dispose() not closing summarizer container, + and Summarizer.runCore() leaving dangling Promise.race() 2-minute timeout timers (root causes 8, 9) + +[x] Fix packages/framework/client-logger/fluid-telemetry - ApplicationInsights stopPollingInternalLogs + +[x] Fix packages/framework/react - already clean + +[x] Fix packages/service-clients/odsp-client - already clean + +[x] Fix packages/test/test-end-to-end-tests: two-part fix: + 1. patch global.setTimeout in mocha-test-setup/mochaHooks.ts to unref() timers >10s + (handles N-1 compat timer hangs from Summarizer.runCore() Promise.race() timers) + 2. fix compression.spec.ts describeInstallVersions block: create versionedProvider ONCE + in before/after hooks instead of per-test in beforeEach/afterEach (fixes socket leak) + +## Remaining + +- tools/test-tools/.mocharc.cjs: Standalone package (own pnpm-workspace.yaml, not part of main + monorepo). Tests appear to be trivially clean (just spawnSync) but can't verify without + separate install. Low priority. diff --git a/examples/data-objects/inventory-app/.mocharc.cjs b/examples/data-objects/inventory-app/.mocharc.cjs index a19ed21e9cd7..fcb7c58983b8 100644 --- a/examples/data-objects/inventory-app/.mocharc.cjs +++ b/examples/data-objects/inventory-app/.mocharc.cjs @@ -8,9 +8,6 @@ const getFluidTestMochaConfig = require("@fluid-internal/mocha-test-setup/mocharc-common"); const config = getFluidTestMochaConfig(__dirname); -// TODO: figure out why this package needs the --exit flag, tests might not be cleaning up correctly after themselves. -// AB#7856 -config.exit = true; // Set up JSDOM before Quill is imported (Quill requires document at import time) config["node-option"] ??= []; diff --git a/examples/data-objects/table-document/.mocharc.cjs b/examples/data-objects/table-document/.mocharc.cjs index 6345db3514ce..45580c775c72 100644 --- a/examples/data-objects/table-document/.mocharc.cjs +++ b/examples/data-objects/table-document/.mocharc.cjs @@ -8,7 +8,4 @@ const getFluidTestMochaConfig = require("@fluid-internal/mocha-test-setup/mocharc-common"); const config = getFluidTestMochaConfig(__dirname); -// TODO: figure out why this package needs the --exit flag, tests might not be cleaning up correctly after themselves -// AB#7856 -config.exit = true; module.exports = config; diff --git a/examples/data-objects/webflow/.mocharc.cjs b/examples/data-objects/webflow/.mocharc.cjs index 077b3508b0d8..7d9504f93f4b 100644 --- a/examples/data-objects/webflow/.mocharc.cjs +++ b/examples/data-objects/webflow/.mocharc.cjs @@ -9,8 +9,4 @@ const packageDir = __dirname; const getFluidTestMochaConfig = require("@fluid-private/test-version-utils/mocharc-common"); const config = getFluidTestMochaConfig(packageDir); -config["node-option"].push("experimental-loader=esm-loader-css"); -// TODO: figure out why this package needs the --exit flag, tests might not be cleaning up correctly after themselves -// AB#7856 -config.exit = true; module.exports = config; diff --git a/examples/data-objects/webflow/src/test/layout.spec.ts b/examples/data-objects/webflow/src/test/layout.spec.ts index 0f4cc1bf3519..95a0422a365c 100644 --- a/examples/data-objects/webflow/src/test/layout.spec.ts +++ b/examples/data-objects/webflow/src/test/layout.spec.ts @@ -19,7 +19,10 @@ import { } from "@fluidframework/test-utils/internal"; import { FlowDocument } from "../document/index.js"; -import { htmlFormatter } from "../index.js"; +// Import htmlFormatter directly to avoid loading webflowView.tsx which imports CSS files. +// Using the main index.js would transitively import CSS via host/webflowView.tsx, +// requiring --experimental-loader=esm-loader-css which prevents clean process exit in Node.js 20. +import { htmlFormatter } from "../html/formatters.js"; import { Layout } from "../view/layout.js"; interface ISnapshotNode { diff --git a/experimental/PropertyDDS/packages/property-common/.mocharc.json b/experimental/PropertyDDS/packages/property-common/.mocharc.json index cc7645a66cc7..f9819e9966d3 100644 --- a/experimental/PropertyDDS/packages/property-common/.mocharc.json +++ b/experimental/PropertyDDS/packages/property-common/.mocharc.json @@ -2,7 +2,6 @@ "R": "spec", "recursive": true, "timeout": "180s", - "exit": true, "full-trace": true, "spec": ["test/setup.js", "test/**/*.spec.js"] } diff --git a/experimental/PropertyDDS/packages/property-dds/.mocharc.cjs b/experimental/PropertyDDS/packages/property-dds/.mocharc.cjs index adf4067e4f06..45580c775c72 100644 --- a/experimental/PropertyDDS/packages/property-dds/.mocharc.cjs +++ b/experimental/PropertyDDS/packages/property-dds/.mocharc.cjs @@ -8,5 +8,4 @@ const getFluidTestMochaConfig = require("@fluid-internal/mocha-test-setup/mocharc-common"); const config = getFluidTestMochaConfig(__dirname); -config.exit = true; module.exports = config; diff --git a/experimental/PropertyDDS/packages/property-dds/src/test/propertyTree.spec.ts b/experimental/PropertyDDS/packages/property-dds/src/test/propertyTree.spec.ts index 6ceea0a4bb2c..56774b862336 100644 --- a/experimental/PropertyDDS/packages/property-dds/src/test/propertyTree.spec.ts +++ b/experimental/PropertyDDS/packages/property-dds/src/test/propertyTree.spec.ts @@ -65,6 +65,7 @@ interface withSummarizer extends Result { describe("PropertyDDS summarizer", () => { let objProvider: ITestObjectProvider; + let driver: LocalServerTestDriver; const propertyDdsId = "PropertyTree"; const USERS = "users"; const getClient = async (withSummarizer = false, load = false) => { @@ -97,7 +98,7 @@ describe("PropertyDDS summarizer", () => { }; beforeEach(async () => { - const driver = new LocalServerTestDriver(); + driver = new LocalServerTestDriver(); const registry = [[propertyDdsId, new PropertyTreeFactory()]] as ChannelFactoryRegistry; objProvider = new TestObjectProvider( @@ -111,6 +112,11 @@ describe("PropertyDDS summarizer", () => { ); }); + afterEach(async () => { + objProvider.reset(); + await driver.dispose(); + }); + it("Scenario 1", async function () { /** * This test produces a scenario where a summarizer creates a summary with an empty remoteChanges array. @@ -401,6 +407,11 @@ function executePerPropertyTreeType( sharedPropertyTree2 = await dataObject2.getSharedObject(propertyDdsId); }); + afterEach(async () => { + opProcessingController.reset(); + await deltaConnectionServer.close(); + }); + describe("APIs", () => { it("Can create a PropertyTree", () => { expect(sharedPropertyTree1).to.not.be.equal(undefined); diff --git a/experimental/PropertyDDS/packages/property-dds/src/test/rebasing.spec.ts b/experimental/PropertyDDS/packages/property-dds/src/test/rebasing.spec.ts index 34c2d2623564..19e1e2f5832d 100644 --- a/experimental/PropertyDDS/packages/property-dds/src/test/rebasing.spec.ts +++ b/experimental/PropertyDDS/packages/property-dds/src/test/rebasing.spec.ts @@ -1135,6 +1135,11 @@ describe("PropertyDDS", () => { await setupContainers(); }); + afterEach(async () => { + opProcessingController.reset(); + await deltaConnectionServer.close(); + }); + rebaseTests(); }); @@ -1143,6 +1148,11 @@ describe("PropertyDDS", () => { await setupContainers(false); }); + afterEach(async () => { + opProcessingController.reset(); + await deltaConnectionServer.close(); + }); + rebaseTests(); }); }); diff --git a/experimental/PropertyDDS/packages/property-properties/.mocharc.json b/experimental/PropertyDDS/packages/property-properties/.mocharc.json index cc7645a66cc7..f9819e9966d3 100644 --- a/experimental/PropertyDDS/packages/property-properties/.mocharc.json +++ b/experimental/PropertyDDS/packages/property-properties/.mocharc.json @@ -2,7 +2,6 @@ "R": "spec", "recursive": true, "timeout": "180s", - "exit": true, "full-trace": true, "spec": ["test/setup.js", "test/**/*.spec.js"] } diff --git a/experimental/dds/tree/.mocharc.cjs b/experimental/dds/tree/.mocharc.cjs index c64cc0f9f8b5..c57552c7375a 100644 --- a/experimental/dds/tree/.mocharc.cjs +++ b/experimental/dds/tree/.mocharc.cjs @@ -8,5 +8,4 @@ const getFluidTestMochaConfig = require('@fluid-internal/mocha-test-setup/mocharc-common'); const config = getFluidTestMochaConfig(__dirname); -config.exit = true; module.exports = config; diff --git a/experimental/dds/tree/src/test/utilities/TestUtilities.ts b/experimental/dds/tree/src/test/utilities/TestUtilities.ts index ca311c85ee20..bf429d766fab 100644 --- a/experimental/dds/tree/src/test/utilities/TestUtilities.ts +++ b/experimental/dds/tree/src/test/utilities/TestUtilities.ts @@ -286,9 +286,13 @@ export interface LocalServerSharedTreeTestingOptions { } const testObjectProviders: TestObjectProvider[] = []; -afterEach(() => { +afterEach(async () => { for (const provider of testObjectProviders) { - provider.reset(); + try { + provider.reset(); + } finally { + await provider.driver.dispose?.(); + } } testObjectProviders.length = 0; }); @@ -369,10 +373,14 @@ export async function setUpLocalServerTestSharedTree( if (testObjectProvider !== undefined) { provider = testObjectProvider; - const driver = new LocalServerTestDriver(); const loader = makeTestLoader(provider); // Once ILoaderOptions is specificable, this should use `provider.loadTestContainer` instead. - container = await loader.resolve({ url: await driver.createContainerUrl(treeId), headers }, pendingLocalState); + // Use the provider's existing driver to get the URL rather than creating a new LocalServerTestDriver + // (which would create an unused LocalDeltaConnectionServer with a running DeliLambda timer). + container = await loader.resolve( + { url: await provider.driver.createContainerUrl(treeId), headers }, + pendingLocalState + ); await waitContainerToCatchUp(container); } else { const driver = new LocalServerTestDriver(); diff --git a/packages/dds/map/src/test/memory/.mocharc.cjs b/packages/dds/map/src/test/memory/.mocharc.cjs index bc28b0f6da23..e7fd36b49a5d 100644 --- a/packages/dds/map/src/test/memory/.mocharc.cjs +++ b/packages/dds/map/src/test/memory/.mocharc.cjs @@ -8,7 +8,6 @@ */ module.exports = { - "exit": true, "fgrep": ["@Benchmark", "@MemoryUsage"], "node-option": ["expose-gc", "gc-global", "unhandled-rejections=strict"], // without leading "--" "recursive": true, diff --git a/packages/dds/matrix/src/test/memory/.mocharc.cjs b/packages/dds/matrix/src/test/memory/.mocharc.cjs index 6f6e7ad33954..559cd3c4045a 100644 --- a/packages/dds/matrix/src/test/memory/.mocharc.cjs +++ b/packages/dds/matrix/src/test/memory/.mocharc.cjs @@ -8,7 +8,6 @@ */ module.exports = { - "exit": true, "fgrep": ["@Benchmark", "@MemoryUsage"], "node-option": ["expose-gc", "gc-global", "unhandled-rejections=strict"], // without leading "--" "recursive": true, diff --git a/packages/dds/sequence/src/test/memory/.mocharc.cjs b/packages/dds/sequence/src/test/memory/.mocharc.cjs index bc28b0f6da23..e7fd36b49a5d 100644 --- a/packages/dds/sequence/src/test/memory/.mocharc.cjs +++ b/packages/dds/sequence/src/test/memory/.mocharc.cjs @@ -8,7 +8,6 @@ */ module.exports = { - "exit": true, "fgrep": ["@Benchmark", "@MemoryUsage"], "node-option": ["expose-gc", "gc-global", "unhandled-rejections=strict"], // without leading "--" "recursive": true, diff --git a/packages/dds/tree/.mocharc.cjs b/packages/dds/tree/.mocharc.cjs index ca2f52a3fd68..705cb9ec9eb6 100644 --- a/packages/dds/tree/.mocharc.cjs +++ b/packages/dds/tree/.mocharc.cjs @@ -16,8 +16,4 @@ const config = getFluidTestMochaConfig( [], process.argv.includes("--emulateProduction") ? "PROD" : undefined, ); -// TODO: figure out why this package needs the --exit flag, tests might not be cleaning up correctly after themselves -// In this package, tests which use `TestTreeProvider.create` cause this issue, but there might be other cases as well. -// AB#7856 -config.exit = true; module.exports = config; diff --git a/packages/dds/tree/src/test/feature-libraries/node-identifier/nodeIdentifier.spec.ts b/packages/dds/tree/src/test/feature-libraries/node-identifier/nodeIdentifier.spec.ts index 08a272b9a47f..1f4244a89aee 100644 --- a/packages/dds/tree/src/test/feature-libraries/node-identifier/nodeIdentifier.spec.ts +++ b/packages/dds/tree/src/test/feature-libraries/node-identifier/nodeIdentifier.spec.ts @@ -30,7 +30,10 @@ async function getIIDCompressor(): Promise { runtime: IFluidDataStoreRuntime; } ).runtime; - return runtime.idCompressor ?? fail("Expected IIdCompressor to be present in runtime"); + const compressor = + runtime.idCompressor ?? fail("Expected IIdCompressor to be present in runtime"); + await provider.dispose(); + return compressor; } describe("Node Identifier", () => { diff --git a/packages/dds/tree/src/test/shared-tree/sharedTree.spec.ts b/packages/dds/tree/src/test/shared-tree/sharedTree.spec.ts index 326ddd95a260..8a904c6a432d 100644 --- a/packages/dds/tree/src/test/shared-tree/sharedTree.spec.ts +++ b/packages/dds/tree/src/test/shared-tree/sharedTree.spec.ts @@ -157,6 +157,23 @@ function treeTestFactory(): ISharedTree { } describe("SharedTree", () => { + const providersToDispose: ITestTreeProvider[] = []; + + afterEach(async () => { + for (const provider of providersToDispose) { + await provider.dispose(); + } + providersToDispose.length = 0; + }); + + async function makeProvider( + ...args: Parameters + ): Promise { + const provider = await TestTreeProvider.create(...args); + providersToDispose.push(provider); + return provider; + } + describe("viewWith", () => { it("@Smoke initialize tree", () => { const tree = treeTestFactory(); @@ -319,7 +336,7 @@ describe("SharedTree", () => { it("handle in op", async () => { // TODO: ADO#7111 schema should be specified to enable compressed encoding. - const provider = await TestTreeProvider.create( + const provider = await makeProvider( 2, SummarizeType.disabled, configuredSharedTree({ @@ -389,7 +406,7 @@ describe("SharedTree", () => { }); it("can be connected to another tree", async () => { - const provider = await TestTreeProvider.create(2); + const provider = await makeProvider(2); assert(provider.trees[0].isAttached()); assert(provider.trees[1].isAttached()); @@ -416,7 +433,7 @@ describe("SharedTree", () => { }); it("can summarize and load", async () => { - const provider = await TestTreeProvider.create(1, SummarizeType.onDemand); + const provider = await makeProvider(1, SummarizeType.onDemand); const value = 42; provider.trees[0] .viewWith( @@ -553,7 +570,7 @@ describe("SharedTree", () => { }); it("on a client which uploaded a blob", async () => { - const provider = await TestTreeProvider.create(1, SummarizeType.onDemand); + const provider = await makeProvider(1, SummarizeType.onDemand); await provider.ensureSynchronized(); const tree = provider.trees[0]; const view = tree.viewWith( @@ -570,7 +587,7 @@ describe("SharedTree", () => { describe("uploads new schema data", () => { it("without incremental summary context", async () => { - const provider = await TestTreeProvider.create(1, SummarizeType.onDemand); + const provider = await makeProvider(1, SummarizeType.onDemand); await provider.ensureSynchronized(); const summaryTree = await provider.trees[0].summarize(); const indexes = summaryTree.summary.tree.indexes; @@ -585,7 +602,7 @@ describe("SharedTree", () => { }); it("when it has changed since the last summary", async () => { - const provider = await TestTreeProvider.create(1, SummarizeType.onDemand); + const provider = await makeProvider(1, SummarizeType.onDemand); await provider.ensureSynchronized(); const tree = provider.trees[0]; const view = tree.viewWith( @@ -609,7 +626,7 @@ describe("SharedTree", () => { }); it("can load from summary", async () => { - const provider = await TestTreeProvider.create(1, SummarizeType.onDemand); + const provider = await makeProvider(1, SummarizeType.onDemand); const [tree1] = provider.trees; const view1 = tree1.viewWith( @@ -640,7 +657,7 @@ describe("SharedTree", () => { }); it("can process ops after loading from summary", async () => { - const provider = await TestTreeProvider.create(3, SummarizeType.onDemand); + const provider = await makeProvider(3, SummarizeType.onDemand); const [container1, container2, container3] = provider.containers; const [tree1, tree2, tree3] = provider.trees; @@ -714,7 +731,7 @@ describe("SharedTree", () => { }); it("can load a summary from a tree and receive edits of the new state", async () => { - const provider = await TestTreeProvider.create(1, SummarizeType.onDemand); + const provider = await makeProvider(1, SummarizeType.onDemand); const [summarizingTree] = provider.trees; const view = summarizingTree.viewWith( new TreeViewConfiguration({ @@ -787,7 +804,7 @@ describe("SharedTree", () => { }); it("can load a summary from a tree and receive edits that require detached tree refreshers", async () => { - const provider = await TestTreeProvider.create(1, SummarizeType.onDemand); + const provider = await makeProvider(1, SummarizeType.onDemand); const [summarizingTree] = provider.trees; const view = summarizingTree.viewWith( new TreeViewConfiguration({ @@ -842,7 +859,7 @@ describe("SharedTree", () => { assert.deepEqual([...view.root], ["A", "C"]); view.dispose(); }; - const provider = await TestTreeProvider.create( + const provider = await makeProvider( 1, SummarizeType.onDemand, new SharedTreeTestFactory(onCreate), @@ -882,7 +899,7 @@ describe("SharedTree", () => { assert.deepEqual([...view.root], ["A", "C"]); view.dispose(); }; - const provider = await TestTreeProvider.create( + const provider = await makeProvider( 1, SummarizeType.onDemand, new SharedTreeTestFactory(onCreate), @@ -924,7 +941,7 @@ describe("SharedTree", () => { assert.deepEqual([...viewUpgrade.root], ["A", "C"]); viewUpgrade.dispose(); }; - const provider = await TestTreeProvider.create( + const provider = await makeProvider( 1, SummarizeType.onDemand, new SharedTreeTestFactory(onCreate), @@ -1017,11 +1034,7 @@ describe("SharedTree", () => { assert.deepEqual([...viewInit.root], ["A", "B"]); viewInit.dispose(); }; - const provider = await TestTreeProvider.create( - 1, - undefined, - new SharedTreeTestFactory(onCreate), - ); + const provider = await makeProvider(1, undefined, new SharedTreeTestFactory(onCreate)); const view = provider.trees[0].viewWith( new TreeViewConfiguration({ schema: StringArray, @@ -1957,7 +1970,7 @@ describe("SharedTree", () => { describe("Rebasing", () => { it("rebases stashed ops with prior state present", async () => { - const provider = await TestTreeProvider.create(2); + const provider = await makeProvider(2); const tree1 = provider.trees[0]; const view1 = tree1.viewWith( new TreeViewConfiguration({ schema: StringArray, enableSchemaValidation }), @@ -2004,7 +2017,7 @@ describe("SharedTree", () => { describe("tolerates open async transactions in the face of inbound commits", () => { it("committed transaction", async () => { - const provider = await TestTreeProvider.create(2); + const provider = await makeProvider(2); const tree1 = provider.trees[0]; const tree2 = provider.trees[1]; const config = new TreeViewConfiguration({ @@ -2036,7 +2049,7 @@ describe("SharedTree", () => { }); it("aborted transaction", async () => { - const provider = await TestTreeProvider.create(2); + const provider = await makeProvider(2); const tree1 = provider.trees[0]; const tree2 = provider.trees[1]; const config = new TreeViewConfiguration({ @@ -2235,11 +2248,7 @@ describe("SharedTree", () => { assert.deepEqual([...parentView.root], ["C", "B", "A"]); parentView.dispose(); }; - const provider = await TestTreeProvider.create( - 1, - undefined, - new SharedTreeTestFactory(onCreate), - ); + const provider = await makeProvider(1, undefined, new SharedTreeTestFactory(onCreate)); const [tree] = provider.trees; assert.deepEqual( [ @@ -2272,7 +2281,7 @@ describe("SharedTree", () => { describe("Schema changes", () => { it("handles two trees schematizing identically at the same time", async () => { - const provider = await TestTreeProvider.create(2, SummarizeType.disabled); + const provider = await makeProvider(2, SummarizeType.disabled); const [tree1, tree2] = provider.trees; const value1 = "42"; const value2 = "42"; @@ -2332,7 +2341,7 @@ describe("SharedTree", () => { // Undoing schema changes is not supported because it may render some of the forest contents invalid. // This may be revisited in the future. it.skip("can be undone at the tip", async () => { - const provider = await TestTreeProvider.create(2, SummarizeType.disabled); + const provider = await makeProvider(2, SummarizeType.disabled); const tree = provider.trees[0]; const { undoStack } = createTestUndoRedoStacks(tree.kernel.checkout.events); @@ -2362,7 +2371,7 @@ describe("SharedTree", () => { // This doesn't bubble up b/c of issues using TestTreeProvider without proper listening to errors coming // from containers. it("can apply and resubmit stashed schema ops", async () => { - const provider = await TestTreeProvider.create(2); + const provider = await makeProvider(2); const pausedContainer: ContainerAlpha = asLegacyAlpha(provider.containers[0]); const url = (await pausedContainer.getAbsoluteUrl("")) ?? assert.fail("didn't get url"); @@ -2501,7 +2510,7 @@ describe("SharedTree", () => { jsonValidator: FormatValidatorBasic, treeEncodeType: TreeCompressionStrategy.Uncompressed, }).getFactory(); - const provider = await TestTreeProvider.create(1, SummarizeType.onDemand, factory); + const provider = await makeProvider(1, SummarizeType.onDemand, factory); provider.trees[0] .viewWith(new TreeViewConfiguration({ schema: StringArray, enableSchemaValidation })) .initialize(["A", "B", "C"]); @@ -2547,7 +2556,7 @@ describe("SharedTree", () => { jsonValidator: FormatValidatorBasic, treeEncodeType: TreeCompressionStrategy.Compressed, }).getFactory(); - const provider2 = await TestTreeProvider.create(1, SummarizeType.onDemand, factory2); + const provider2 = await makeProvider(1, SummarizeType.onDemand, factory2); provider2.trees[0] .viewWith( @@ -2837,7 +2846,7 @@ describe("SharedTree", () => { ] as const) { it(subCase, async () => { const internalOption = resolveOptions({ enableSharedBranches: true }); - const provider = await TestTreeProvider.create( + const provider = await makeProvider( 1, SummarizeType.onDemand, new SharedTreeTestFactory(() => {}, undefined, internalOption), diff --git a/packages/dds/tree/src/test/util/testTreeProvider.spec.ts b/packages/dds/tree/src/test/util/testTreeProvider.spec.ts index 82cfd3b00005..646488836dfa 100644 --- a/packages/dds/tree/src/test/util/testTreeProvider.spec.ts +++ b/packages/dds/tree/src/test/util/testTreeProvider.spec.ts @@ -6,14 +6,35 @@ import { strict as assert } from "node:assert"; import { SharedTreeCore } from "../../shared-tree-core/index.js"; -import { SummarizeType, TestTreeProvider, spyOnMethod } from "../utils.js"; +import { + type ITestTreeProvider, + SummarizeType, + TestTreeProvider, + spyOnMethod, +} from "../utils.js"; describe("TestTreeProvider", () => { + const providersToDispose: ITestTreeProvider[] = []; + + afterEach(async () => { + for (const provider of providersToDispose) { + await provider.dispose(); + } + providersToDispose.length = 0; + }); + + async function makeProvider( + ...args: Parameters + ): Promise { + const provider = await TestTreeProvider.create(...args); + providersToDispose.push(provider); + return provider; + } it("can create 1", async () => { - const provider = await TestTreeProvider.create(1); + const provider = await makeProvider(1); }); it("can create 2", async () => { - const provider = await TestTreeProvider.create(2); + const provider = await makeProvider(2); }); it("can manually trigger summaries with summarizeOnDemand", async () => { @@ -22,7 +43,7 @@ describe("TestTreeProvider", () => { summaryCount += 1; }); - const provider = await TestTreeProvider.create(1, SummarizeType.onDemand); + const provider = await makeProvider(1, SummarizeType.onDemand); const summaries = summaryCount; await provider.summarize(); @@ -34,7 +55,7 @@ describe("TestTreeProvider", () => { it("cannot manually trigger summaries without setting summarizeOnDemand", async () => { let summarizerError; try { - const provider = await TestTreeProvider.create(1); + const provider = await makeProvider(1); await provider.summarize(); } catch (error) { summarizerError = error; @@ -45,7 +66,7 @@ describe("TestTreeProvider", () => { it("cannot manually trigger summaries with 0 trees", async () => { let summarizerError; try { - const provider = await TestTreeProvider.create(0, SummarizeType.onDemand); + const provider = await makeProvider(0, SummarizeType.onDemand); await provider.summarize(); } catch (error) { summarizerError = error; @@ -59,7 +80,7 @@ describe("TestTreeProvider", () => { summaryCount += 1; }); - const provider = await TestTreeProvider.create(2, SummarizeType.onDemand); + const provider = await makeProvider(2, SummarizeType.onDemand); const summaries = summaryCount; await provider.summarize(); diff --git a/packages/dds/tree/src/test/utils.ts b/packages/dds/tree/src/test/utils.ts index 8a1af3060c20..2f7d50db5dcc 100644 --- a/packages/dds/tree/src/test/utils.ts +++ b/packages/dds/tree/src/test/utils.ts @@ -273,6 +273,7 @@ export class TestTreeProvider { private readonly _trees: ISharedTree[] = []; private readonly _containers: IContainer[] = []; private readonly summarizer?: ISummarizer; + private readonly summarizerContainer?: IContainer; public get trees(): readonly ISharedTree[] { return this._trees; @@ -321,8 +322,23 @@ export class TestTreeProvider { ), { summaryOptions: { + // For onDemand, use the manually-created summarizer (via createSummarizer below). + // Using "summaryOnRequest" prevents SummaryManager from creating an auto-summarizer + // on interactive containers asynchronously — which could escape tracking and leave + // GC and SummarizeHeuristicRunner timers alive after the test completes. + // Unlike "disabled", "summaryOnRequest" still allows summarizer containers to + // create their Summarizer instance (needed by createSummarizer()). summaryConfigOverrides: - summarizeType === SummarizeType.disabled ? { state: "disabled" } : undefined, + summarizeType === SummarizeType.disabled + ? { state: "disabled" } + : summarizeType === SummarizeType.onDemand + ? { + state: "summaryOnRequest", + initialSummarizerDelayMs: 0, + maxAckWaitTime: 20000, + maxOpsSinceLastSummary: 7000, + } + : undefined, }, enableRuntimeIdCompressor: "on", }, @@ -333,11 +349,15 @@ export class TestTreeProvider { if (summarizeType === SummarizeType.onDemand) { const container = await objProvider.makeTestContainer(); const firstTree = await this.getTree(container); - const { summarizer } = await createSummarizer(objProvider, container); + const { summarizer, container: summarizerContainer } = await createSummarizer( + objProvider, + container, + ); const provider = new TestTreeProvider(objProvider, [ container, firstTree, summarizer, + summarizerContainer, ]) as ITestTreeProvider; for (let i = 1; i < trees; i++) { await provider.createTree(); @@ -400,16 +420,40 @@ export class TestTreeProvider { return this.trees[Symbol.iterator](); } + /** + * Dispose the underlying test driver to free resources (e.g. local server timers). + * Call this in afterEach/after hooks for tests that use this provider. + */ + public async dispose(): Promise { + // Close all tracked containers and reset the tracker. + this.provider.resetLoaderContainerTracker(); + // Dispose each container to trigger ContainerRuntime.dispose(), which clears the + // GarbageCollector session expiry timer that would otherwise keep the process alive. + for (const container of this._containers) { + if (container.disposed !== true) { + container.dispose(); + } + } + // The summarizer container is not tracked by LoaderContainerTracker (it skips non-interactive + // clients), so we must dispose it explicitly to clear its GC session expiry timer. + if (this.summarizerContainer !== undefined && this.summarizerContainer.disposed !== true) { + this.summarizerContainer.dispose(); + } + // Dispose the driver to release local server resources (e.g. DeliLambda timers). + await this.provider.driver.dispose?.(); + } + private constructor( provider: ITestObjectProvider, - firstTreeParams?: [IContainer, ISharedTree, ISummarizer], + firstTreeParams?: [IContainer, ISharedTree, ISummarizer, IContainer], ) { this.provider = provider; if (firstTreeParams !== undefined) { - const [container, firstTree, summarizer] = firstTreeParams; + const [container, firstTree, summarizer, summarizerContainer] = firstTreeParams; this._containers.push(container); this._trees.push(firstTree); this.summarizer = summarizer; + this.summarizerContainer = summarizerContainer; } return new Proxy(this, { get: (target, prop, receiver) => { diff --git a/packages/framework/client-logger/fluid-telemetry/.mocharc.cjs b/packages/framework/client-logger/fluid-telemetry/.mocharc.cjs index 49382c7b39ce..4f8c7ce7e5e7 100644 --- a/packages/framework/client-logger/fluid-telemetry/.mocharc.cjs +++ b/packages/framework/client-logger/fluid-telemetry/.mocharc.cjs @@ -11,7 +11,4 @@ const config = getFluidTestMochaConfig(__dirname); // These tests need to be run with multiple different test file filters for different cases. // As this is a rather different setup, it's simplest to just let the individual scripts specify what they need and disable the default. delete config.spec; -// TODO: figure out why this package needs the --exit flag, tests might not be cleaning up correctly after themselves -// AB#7856 -config.exit = true; module.exports = config; diff --git a/packages/framework/client-logger/fluid-telemetry/src/container/telemetryManager.ts b/packages/framework/client-logger/fluid-telemetry/src/container/telemetryManager.ts index 76b51ce384a6..7440ced4207e 100644 --- a/packages/framework/client-logger/fluid-telemetry/src/container/telemetryManager.ts +++ b/packages/framework/client-logger/fluid-telemetry/src/container/telemetryManager.ts @@ -24,6 +24,8 @@ import type { ContainerEventTelemetryProducer } from "./telemetryProducer.js"; export class ContainerTelemetryManager { private static readonly HEARTBEAT_EMISSION_INTERNAL_MS = 60000; + private heartbeatInterval: ReturnType | undefined; + public constructor( private readonly container: IFluidContainer, private readonly telemetryProducer: ContainerEventTelemetryProducer, @@ -45,10 +47,12 @@ export class ContainerTelemetryManager { ); this.container.on( IFluidContainerSystemEventNames.DISPOSED, - (error?: ICriticalContainerError) => + (error?: ICriticalContainerError) => { this.handleContainerSystemEvent(IFluidContainerSystemEventNames.DISPOSED, { error, - }), + }); + clearInterval(this.heartbeatInterval); + }, ); } @@ -57,7 +61,7 @@ export class ContainerTelemetryManager { * if and only if the container is in a "connected" state. It is used to keep a pulse check on a live container */ private setupHeartbeatTelemetryEmission(): void { - setInterval(() => { + this.heartbeatInterval = setInterval(() => { // eslint-disable-next-line @typescript-eslint/no-unsafe-enum-comparison -- container.connectionState may not be typed precisely if (this.container.connectionState === ConnectionState.Connected) { const telemetry = this.telemetryProducer.produceHeartbeatTelemetry(); @@ -66,6 +70,10 @@ export class ContainerTelemetryManager { } } }, ContainerTelemetryManager.HEARTBEAT_EMISSION_INTERNAL_MS); + // In Node.js, unref() allows the process to exit even if the interval is still pending. + // The optional chaining handles browser environments where setInterval returns a number. + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access + (this.heartbeatInterval as any).unref?.(); } /** diff --git a/packages/framework/client-logger/fluid-telemetry/src/test/containerTelemetry.spec.ts b/packages/framework/client-logger/fluid-telemetry/src/test/containerTelemetry.spec.ts index d0d08ec1783b..2436f8824518 100644 --- a/packages/framework/client-logger/fluid-telemetry/src/test/containerTelemetry.spec.ts +++ b/packages/framework/client-logger/fluid-telemetry/src/test/containerTelemetry.spec.ts @@ -68,6 +68,11 @@ describe("container telemetry via", () => { }; }); + afterEach(() => { + // Dispose the container to trigger cleanup of the heartbeat interval in ContainerTelemetryManager. + mockFluidContainer.dispose(); + }); + it("Emitting 'connected' container system event produces expected ContainerConnectedTelemetry using Azure App Insights", () => { startTelemetry(telemetryConfig); diff --git a/packages/framework/fluid-static/src/fluidContainer.ts b/packages/framework/fluid-static/src/fluidContainer.ts index 0f57df727a36..eade1d6940a0 100644 --- a/packages/framework/fluid-static/src/fluidContainer.ts +++ b/packages/framework/fluid-static/src/fluidContainer.ts @@ -331,6 +331,7 @@ class FluidContainer this.acquireExtension = extensionStore.acquireExtension.bind(extensionStore); container.on("connected", this.connectedHandler); container.on("closed", this.disposedHandler); + container.on("disposed", this.disposedHandler); container.on("disconnected", this.disconnectedHandler); container.on("saved", this.savedHandler); container.on("dirty", this.dirtyHandler); @@ -390,9 +391,10 @@ class FluidContainer } public dispose(): void { - this.container.close(); + this.container.dispose(); this.container.off("connected", this.connectedHandler); this.container.off("closed", this.disposedHandler); + this.container.off("disposed", this.disposedHandler); this.container.off("disconnected", this.disconnectedHandler); this.container.off("saved", this.savedHandler); this.container.off("dirty", this.dirtyHandler); diff --git a/packages/framework/react/.mocharc.cjs b/packages/framework/react/.mocharc.cjs index abdc987d1f19..e23fa5951008 100644 --- a/packages/framework/react/.mocharc.cjs +++ b/packages/framework/react/.mocharc.cjs @@ -8,8 +8,5 @@ const getFluidTestMochaConfig = require("@fluid-internal/mocha-test-setup/mocharc-common"); const config = getFluidTestMochaConfig(__dirname); -// TODO: figure out why this package needs the --exit flag, tests might not be cleaning up correctly after themselves. -// AB#7856 -config.exit = true; module.exports = config; diff --git a/packages/framework/react/src/test/mochaHooks.ts b/packages/framework/react/src/test/mochaHooks.ts index e15d1fa7eb81..e8a3f20a84dc 100644 --- a/packages/framework/react/src/test/mochaHooks.ts +++ b/packages/framework/react/src/test/mochaHooks.ts @@ -11,5 +11,10 @@ const cleanup = globalJsdom(); // Remove JSDOM after imports are done, but before we run any tests. // Tests which require JSDOM can call globalJsdom() to setup their own clean dom. before(() => { + // Close the JSDOM window before removing globals. This clears all timers created during + // JSDOM setup (e.g. requestAnimationFrame intervals) to allow mocha to exit cleanly. + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access + const jsdom = (globalThis as any).$jsdom as { window: { close(): void } } | undefined; + jsdom?.window.close(); cleanup(); }); diff --git a/packages/framework/react/src/test/reactSharedTreeView.spec.tsx b/packages/framework/react/src/test/reactSharedTreeView.spec.tsx index d6d658611fc5..43d6e57a4a9e 100644 --- a/packages/framework/react/src/test/reactSharedTreeView.spec.tsx +++ b/packages/framework/react/src/test/reactSharedTreeView.spec.tsx @@ -41,10 +41,14 @@ describe("reactSharedTreeView", () => { const tinyliciousClient = new TinyliciousClient(); const { container } = await tinyliciousClient.createContainer(containerSchema, "2"); - const dataObject = container.initialObjects.tree; - assert.equal(dataObject.treeView.root.nuts, 5); - dataObject.treeView.root.nuts += 1; - assert.equal(dataObject.treeView.root.bolts, 6); + try { + const dataObject = container.initialObjects.tree; + assert.equal(dataObject.treeView.root.nuts, 5); + dataObject.treeView.root.nuts += 1; + assert.equal(dataObject.treeView.root.bolts, 6); + } finally { + container.dispose(); + } }); describe("dom tests", () => { @@ -55,6 +59,10 @@ describe("reactSharedTreeView", () => { }); after(() => { + // Close JSDOM window to clear timers (e.g. requestAnimationFrame intervals) before cleanup. + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access + const jsdom = (globalThis as any).$jsdom as { window: { close(): void } } | undefined; + jsdom?.window.close(); cleanup(); }); diff --git a/packages/framework/react/src/test/text/textEditor.test.tsx b/packages/framework/react/src/test/text/textEditor.test.tsx index ff1b7ff1358a..e6d4fdd42ec9 100644 --- a/packages/framework/react/src/test/text/textEditor.test.tsx +++ b/packages/framework/react/src/test/text/textEditor.test.tsx @@ -83,6 +83,10 @@ describe("textEditor", () => { }); after(() => { + // Close JSDOM window to clear timers (e.g. requestAnimationFrame intervals) before cleanup. + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access + const jsdom = (globalThis as any).$jsdom as { window: { close(): void } } | undefined; + jsdom?.window.close(); cleanup(); }); diff --git a/packages/framework/react/src/test/useObservation.spec.tsx b/packages/framework/react/src/test/useObservation.spec.tsx index 599814426d7f..6cfcb4bce3ee 100644 --- a/packages/framework/react/src/test/useObservation.spec.tsx +++ b/packages/framework/react/src/test/useObservation.spec.tsx @@ -25,6 +25,10 @@ describe("useObservation", () => { }); after(() => { + // Close JSDOM window to clear timers (e.g. requestAnimationFrame intervals) before cleanup. + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access + const jsdom = (globalThis as any).$jsdom as { window: { close(): void } } | undefined; + jsdom?.window.close(); cleanup(); }); diff --git a/packages/framework/react/src/test/useTree.spec.tsx b/packages/framework/react/src/test/useTree.spec.tsx index 3f8c8d6100f7..c74525c72f8b 100644 --- a/packages/framework/react/src/test/useTree.spec.tsx +++ b/packages/framework/react/src/test/useTree.spec.tsx @@ -26,6 +26,10 @@ describe("useTree", () => { }); after(() => { + // Close JSDOM window to clear timers (e.g. requestAnimationFrame intervals) before cleanup. + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access + const jsdom = (globalThis as any).$jsdom as { window: { close(): void } } | undefined; + jsdom?.window.close(); cleanup(); }); diff --git a/packages/loader/container-loader/.mocharc.cjs b/packages/loader/container-loader/.mocharc.cjs index 6345db3514ce..45580c775c72 100644 --- a/packages/loader/container-loader/.mocharc.cjs +++ b/packages/loader/container-loader/.mocharc.cjs @@ -8,7 +8,4 @@ const getFluidTestMochaConfig = require("@fluid-internal/mocha-test-setup/mocharc-common"); const config = getFluidTestMochaConfig(__dirname); -// TODO: figure out why this package needs the --exit flag, tests might not be cleaning up correctly after themselves -// AB#7856 -config.exit = true; module.exports = config; diff --git a/packages/loader/container-loader/src/container.ts b/packages/loader/container-loader/src/container.ts index f6035607f005..29afc3be26c7 100644 --- a/packages/loader/container-loader/src/container.ts +++ b/packages/loader/container-loader/src/container.ts @@ -1043,6 +1043,10 @@ export class Container private closeCore(error?: ICriticalContainerError): void { assert(!this.closed, 0x315 /* re-entrancy */); + // Clear the NoopHeuristic timer so it doesn't keep the event loop alive after the container is closed. + this.noopHeuristic?.dispose(); + this.noopHeuristic = undefined; + try { // Ensure that we raise all key events even if one of these throws try { @@ -1094,6 +1098,11 @@ export class Container assert(!this._disposed, 0x54c /* Container already disposed */); this._disposed = true; + // Clear the NoopHeuristic timer so it doesn't keep the event loop alive after the container is disposed. + // This handles the case where dispose() is called without close() first (e.g. container.dispose() in tests). + this.noopHeuristic?.dispose(); + this.noopHeuristic = undefined; + try { // Ensure that we raise all key events even if one of these throws try { diff --git a/packages/loader/container-loader/src/noopHeuristic.ts b/packages/loader/container-loader/src/noopHeuristic.ts index 8327707f9844..b027e000f653 100644 --- a/packages/loader/container-loader/src/noopHeuristic.ts +++ b/packages/loader/container-loader/src/noopHeuristic.ts @@ -98,6 +98,13 @@ export class NoopHeuristic extends TypedEventEmitter { public notifyDisconnect(): void { // No need to noop for any ops processed prior to disconnect - we are already removed from MSN calculation. this.opsProcessedSinceOpSent = 0; + // Clear the timer so it doesn't keep the event loop alive after disconnect. + // If the container reconnects, the timer will be restarted by notifyMessageProcessed when the next op arrives. + this.timer?.clear(); + } + + public dispose(): void { + this.timer?.clear(); } public notifyMessageSent(): void { diff --git a/packages/loader/container-loader/src/test/serializedStateManager.spec.ts b/packages/loader/container-loader/src/test/serializedStateManager.spec.ts index 7f7bf2951786..04dfded2fc8c 100644 --- a/packages/loader/container-loader/src/test/serializedStateManager.spec.ts +++ b/packages/loader/container-loader/src/test/serializedStateManager.spec.ts @@ -201,6 +201,22 @@ function enableOfflineSnapshotRefresh(logger: ITelemetryBaseLogger): ITelemetryB describe("serializedStateManager", () => { let logger: MockLogger; + const instancesToDispose: SerializedStateManager[] = []; + + afterEach(() => { + for (const sm of instancesToDispose) { + sm.dispose(); + } + instancesToDispose.length = 0; + }); + + function makeSsm( + ...args: ConstructorParameters + ): SerializedStateManager { + const sm = new SerializedStateManager(...args); + instancesToDispose.push(sm); + return sm; + } beforeEach("setup", () => { logger = new MockLogger(); @@ -209,7 +225,7 @@ describe("serializedStateManager", () => { describe("before refreshing the snapshot", () => { it("can't get pending local state when offline load disabled", async () => { const storageAdapter = new MockStorageAdapter(); - const serializedStateManager = new SerializedStateManager( + const serializedStateManager = makeSsm( enableOfflineSnapshotRefresh(logger), storageAdapter, false, @@ -231,7 +247,7 @@ describe("serializedStateManager", () => { ); }); it("can get pending local state after attach", async () => { - const serializedStateManager = new SerializedStateManager( + const serializedStateManager = makeSsm( enableOfflineSnapshotRefresh(logger), failSometimeProxy({ loadedGroupIdSnapshots: {}, @@ -256,7 +272,7 @@ describe("serializedStateManager", () => { baseSnapshot: { ...snapshotTree, id: "fromPending" }, }; const storageAdapter = new MockStorageAdapter(); - const serializedStateManager = new SerializedStateManager( + const serializedStateManager = makeSsm( enableOfflineSnapshotRefresh(logger), storageAdapter, true, @@ -281,7 +297,7 @@ describe("serializedStateManager", () => { it("can fetch snapshot and get state from it", async () => { const storageAdapter = new MockStorageAdapter(); - const serializedStateManager = new SerializedStateManager( + const serializedStateManager = makeSsm( enableOfflineSnapshotRefresh(logger), storageAdapter, true, @@ -318,7 +334,7 @@ describe("serializedStateManager", () => { }; const storageAdapter = new MockStorageAdapter(); const getLatestSnapshotInfoP = new Deferred(); - const serializedStateManager = new SerializedStateManager( + const serializedStateManager = makeSsm( enableOfflineSnapshotRefresh(logger), storageAdapter, true, @@ -368,7 +384,7 @@ describe("serializedStateManager", () => { baseSnapshot: { ...snapshotTree, id: "fromPending" }, }; const storageAdapter = new MockStorageAdapter(); - const serializedStateManager = new SerializedStateManager( + const serializedStateManager = makeSsm( enableOfflineSnapshotRefresh(logger), storageAdapter, true, @@ -433,7 +449,7 @@ describe("serializedStateManager", () => { }, snapshotBlobs: pending.snapshotBlobs, }); - const serializedStateManager = new SerializedStateManager( + const serializedStateManager = makeSsm( enableOfflineSnapshotRefresh(logger), storageAdapter, true, @@ -473,7 +489,7 @@ describe("serializedStateManager", () => { const storageAdapter = new MockStorageAdapter(); let saved = false; const isDirtyF = (): boolean => (saved ? false : isDirty); - const serializedStateManager = new SerializedStateManager( + const serializedStateManager = makeSsm( enableOfflineSnapshotRefresh(logger), storageAdapter, true, @@ -534,7 +550,7 @@ describe("serializedStateManager", () => { let saved = false; const isDirtyF = (): boolean => (saved ? false : isDirty); const storageAdapter = new MockStorageAdapter(); - const serializedStateManager = new SerializedStateManager( + const serializedStateManager = makeSsm( enableOfflineSnapshotRefresh(logger), storageAdapter, true, @@ -585,7 +601,7 @@ describe("serializedStateManager", () => { let saved = false; const isDirtyF = (): boolean => (saved ? false : isDirty); const storageAdapter = new MockStorageAdapter(); - const serializedStateManager = new SerializedStateManager( + const serializedStateManager = makeSsm( enableOfflineSnapshotRefresh(logger), storageAdapter, true, @@ -647,7 +663,7 @@ describe("serializedStateManager", () => { savedOps: [], }; const storageAdapter = new MockStorageAdapter(); - const serializedStateManager = new SerializedStateManager( + const serializedStateManager = makeSsm( enableOfflineSnapshotRefresh(logger), storageAdapter, true, @@ -678,7 +694,7 @@ describe("serializedStateManager", () => { let saved = false; const isDirtyF = (): boolean => (saved ? false : isDirty); const storageAdapter = new MockStorageAdapter(); - const serializedStateManager = new SerializedStateManager( + const serializedStateManager = makeSsm( enableOfflineSnapshotRefresh(logger), storageAdapter, true, @@ -748,7 +764,7 @@ describe("serializedStateManager", () => { let saved = false; const isDirtyF = (): boolean => (saved ? false : isDirty); const storageAdapter = new MockStorageAdapter(); - const serializedStateManager = new SerializedStateManager( + const serializedStateManager = makeSsm( enableOfflineSnapshotRefresh(logger), storageAdapter, true, @@ -797,7 +813,7 @@ describe("serializedStateManager", () => { baseSnapshot: { ...snapshotTree, id: "fromPending" }, }; const storageAdapter = new MockStorageAdapter(); - const serializedStateManager = new SerializedStateManager( + const serializedStateManager = makeSsm( enableOfflineSnapshotRefresh(logger), storageAdapter, true, @@ -831,7 +847,7 @@ describe("serializedStateManager", () => { baseSnapshot: { ...snapshotTree, id: "fromPending" }, }; const storageAdapter = new MockStorageAdapter(); - const serializedStateManager = new SerializedStateManager( + const serializedStateManager = makeSsm( enableOfflineSnapshotRefresh(logger), storageAdapter, true, @@ -881,7 +897,7 @@ describe("serializedStateManager", () => { let saved = false; const isDirtyF = (): boolean => (saved ? false : isDirty); const storageAdapter = new MockStorageAdapter(); - const serializedStateManager = new SerializedStateManager( + const serializedStateManager = makeSsm( enableOfflineSnapshotRefresh(logger), storageAdapter, true, @@ -981,7 +997,7 @@ describe("serializedStateManager", () => { const storageAdapter = new MockStorageAdapter(); let saved = false; const isDirtyF = (): boolean => (saved ? false : isDirty); - const serializedStateManager = new SerializedStateManager( + const serializedStateManager = makeSsm( enableOfflineSnapshotRefresh(logger), storageAdapter, true, @@ -1044,7 +1060,7 @@ describe("serializedStateManager", () => { const storageAdapter = new MockStorageAdapter(); let saved = false; const isDirtyF = (): boolean => (saved ? false : isDirty); - const serializedStateManager = new SerializedStateManager( + const serializedStateManager = makeSsm( enableOfflineSnapshotRefresh(logger), storageAdapter, true, @@ -1107,7 +1123,7 @@ describe("serializedStateManager", () => { const storageAdapter = new MockStorageAdapter(); let saved = false; const isDirtyF = (): boolean => (saved ? false : isDirty); - const serializedStateManager = new SerializedStateManager( + const serializedStateManager = makeSsm( enableOfflineSnapshotRefresh(logger), storageAdapter, true, @@ -1174,7 +1190,7 @@ describe("serializedStateManager", () => { const storageAdapter = new MockStorageAdapter(); let saved = false; const isDirtyF = (): boolean => (saved ? false : isDirty); - const serializedStateManager = new SerializedStateManager( + const serializedStateManager = makeSsm( enableOfflineSnapshotRefresh(logger), storageAdapter, true, @@ -1247,7 +1263,7 @@ describe("serializedStateManager", () => { let saved = false; const isDirtyF = (): boolean => (saved ? false : isDirty); const storageAdapter = new MockStorageAdapter(); - const serializedStateManager = new SerializedStateManager( + const serializedStateManager = makeSsm( enableOfflineSnapshotRefresh(logger), storageAdapter, true, @@ -1313,7 +1329,7 @@ describe("serializedStateManager", () => { let saved = false; const isDirtyF = (): boolean => (saved ? false : isDirty); const storageAdapter = new MockStorageAdapter(); - const serializedStateManager = new SerializedStateManager( + const serializedStateManager = makeSsm( enableOfflineSnapshotRefresh(logger), storageAdapter, true, diff --git a/packages/runtime/container-runtime/.mocharc.cjs b/packages/runtime/container-runtime/.mocharc.cjs index 6345db3514ce..45580c775c72 100644 --- a/packages/runtime/container-runtime/.mocharc.cjs +++ b/packages/runtime/container-runtime/.mocharc.cjs @@ -8,7 +8,4 @@ const getFluidTestMochaConfig = require("@fluid-internal/mocha-test-setup/mocharc-common"); const config = getFluidTestMochaConfig(__dirname); -// TODO: figure out why this package needs the --exit flag, tests might not be cleaning up correctly after themselves -// AB#7856 -config.exit = true; module.exports = config; diff --git a/packages/runtime/container-runtime/src/containerRuntime.ts b/packages/runtime/container-runtime/src/containerRuntime.ts index 259d772552e7..dc3fb7859a3d 100644 --- a/packages/runtime/container-runtime/src/containerRuntime.ts +++ b/packages/runtime/container-runtime/src/containerRuntime.ts @@ -71,7 +71,6 @@ import { Lazy, LazyPromise, PromiseCache, - delay, fail, unreachableCase, } from "@fluidframework/core-utils/internal"; @@ -1445,6 +1444,15 @@ export class ContainerRuntime private emitDirtyDocumentEvent = true; private readonly useDeltaManagerOpsProxy: boolean; private readonly closeSummarizerDelayMs: number; + /** + * Timer and resolve function for the delay in `fetchLatestSnapshotAndMaybeClose`. + * Tracked at class level so {@link ContainerRuntime.dispose} can cancel the timer and resolve the promise early, + * preventing the timer from keeping the event loop alive after the container is disposed. + */ + private closeSummarizerDelayHandle?: { + timer: ReturnType; + resolve: () => void; + }; private readonly signalTelemetryManager = new SignalTelemetryManager(); @@ -2346,6 +2354,14 @@ export class ContainerRuntime } this._disposed = true; + // Cancel the closeSummarizerDelay timer if it's still running, so it doesn't keep the + // event loop alive after the container is disposed. + if (this.closeSummarizerDelayHandle !== undefined) { + clearTimeout(this.closeSummarizerDelayHandle.timer); + this.closeSummarizerDelayHandle.resolve(); + this.closeSummarizerDelayHandle = undefined; + } + this.mc.logger.sendTelemetryEvent( { eventName: "ContainerRuntimeDisposed", @@ -5164,7 +5180,24 @@ export class ContainerRuntime return; } - await delay(this.closeSummarizerDelayMs); + // Use a cancellable delay: track the timer and a resolve function at class level so + // dispose() can cancel it and resolve the promise early, preventing the timer from + // keeping the event loop alive after the container is disposed. + await new Promise((resolve) => { + this.closeSummarizerDelayHandle = { + timer: setTimeout(() => { + this.closeSummarizerDelayHandle = undefined; + resolve(); + }, this.closeSummarizerDelayMs), + resolve, + }; + }); + // Clear any lingering handle (e.g. if the promise was resolved early via dispose()). + this.closeSummarizerDelayHandle = undefined; + // If the container was disposed while we were waiting, skip the close logic. + if (this._disposed) { + return; + } this._summarizer?.stop("latestSummaryStateStale"); this.disposeFn(); } diff --git a/packages/runtime/container-runtime/src/summary/summaryDelayLoadedModule/summarizer.ts b/packages/runtime/container-runtime/src/summary/summaryDelayLoadedModule/summarizer.ts index d83ca01746f6..0eae480e0719 100644 --- a/packages/runtime/container-runtime/src/summary/summaryDelayLoadedModule/summarizer.ts +++ b/packages/runtime/container-runtime/src/summary/summaryDelayLoadedModule/summarizer.ts @@ -169,19 +169,22 @@ export class Summarizer extends TypedEventEmitter implements // if the summarizer container fails to connect. let coordinatorTimedOut = false; const coordinatorTimeoutMs = 2 * 60 * 1000; // 2 minutes + let coordinatorTimeoutId: ReturnType | undefined; const coordinatorResult = await Promise.race([ this.runCoordinatorCreateFn(this.runtime).then((coordinator) => { + // Clear the timeout when coordinator is created to avoid leaving a dangling timer. + clearTimeout(coordinatorTimeoutId); if (coordinatorTimedOut) { coordinator.stop("summarizerClientDisconnected"); } return coordinator; }), - new Promise((resolve) => - setTimeout(() => { + new Promise((resolve) => { + coordinatorTimeoutId = setTimeout(() => { coordinatorTimedOut = true; resolve(undefined); - }, coordinatorTimeoutMs), - ), + }, coordinatorTimeoutMs); + }), ]); // If we timed out before coordinator was created, exit early @@ -252,10 +255,13 @@ export class Summarizer extends TypedEventEmitter implements !runCoordinator.cancelled && Summarizer.stopReasonCanRunLastSummary(stopReason), ); const summarizerStopTimeoutMs = 2 * 60 * 1000; // 2 minutes - const timeoutPromise = new Promise<"timeout">((resolve) => - setTimeout(() => resolve("timeout"), summarizerStopTimeoutMs), - ); + let stopTimeoutId: ReturnType | undefined; + const timeoutPromise = new Promise<"timeout">((resolve) => { + stopTimeoutId = setTimeout(() => resolve("timeout"), summarizerStopTimeoutMs); + }); const waitStopResult = await Promise.race([waitStopPromise, timeoutPromise]); + // Clear the timeout regardless of which promise won to avoid leaving a dangling timer. + clearTimeout(stopTimeoutId); if (waitStopResult === "timeout") { this.logger.sendTelemetryEvent({ eventName: "SummarizerStopTimeout", diff --git a/packages/runtime/container-runtime/src/summary/summaryManager.ts b/packages/runtime/container-runtime/src/summary/summaryManager.ts index 601a9a808ec9..19b0d39aef1c 100644 --- a/packages/runtime/container-runtime/src/summary/summaryManager.ts +++ b/packages/runtime/container-runtime/src/summary/summaryManager.ts @@ -104,6 +104,11 @@ export class SummaryManager private summarizer?: ISummarizer; private _disposed = false; private summarizerStopTimeout?: ReturnType; + /** + * Timer for the delay in {@link delayBeforeCreatingSummarizer}. + * Tracked at the class level so {@link dispose} can cancel it if the container is disposed mid-delay. + */ + private delayBeforeCreatingSummarizerTimer?: ReturnType; /** * Monotonically increasing counter that tracks summarizer lifecycle generations. * Incremented each time {@link cleanupAfterSummarizerStop} runs. Used by the @@ -457,18 +462,21 @@ export class SummaryManager } if (delayMs > 0) { - let timer: number | undefined; let resolveOpPromiseFn: (value: void | PromiseLike) => void; // Create a listener that will break the delay if we've exceeded the initial delay ops count. const opsListenerFn = (): void => { if (this.summaryCollection.opsSinceLastAck >= this.opsToBypassInitialDelay) { - clearTimeout(timer); + clearTimeout(this.delayBeforeCreatingSummarizerTimer); + this.delayBeforeCreatingSummarizerTimer = undefined; resolveOpPromiseFn(); } }; // Create a Promise that will resolve when the delay expires. const delayPromise = new Promise((resolve) => { - timer = setTimeout(() => resolve(), delayMs); + this.delayBeforeCreatingSummarizerTimer = setTimeout(() => { + this.delayBeforeCreatingSummarizerTimer = undefined; + resolve(); + }, delayMs); }); // Create a Promise that will resolve if the ops count passes the threshold. const opPromise = new Promise((resolve) => { @@ -477,6 +485,10 @@ export class SummaryManager this.summaryCollection.addOpListener(opsListenerFn); await Promise.race([delayPromise, opPromise]); this.summaryCollection.removeOpListener(opsListenerFn); + // Clear the timer in case Promise.race resolved via opPromise without opsListenerFn clearing it + // (e.g., if resolveOpPromiseFn was called from dispose() or some other path). + clearTimeout(this.delayBeforeCreatingSummarizerTimer); + this.delayBeforeCreatingSummarizerTimer = undefined; } return startWithInitialDelay; } @@ -505,6 +517,19 @@ export class SummaryManager if (this.summarizerStopTimeout !== undefined) { clearTimeout(this.summarizerStopTimeout); } + // Cancel the delayBeforeCreatingSummarizer timer if it's still running. + // This prevents the timer from keeping the event loop alive after the container is disposed. + if (this.delayBeforeCreatingSummarizerTimer !== undefined) { + clearTimeout(this.delayBeforeCreatingSummarizerTimer); + this.delayBeforeCreatingSummarizerTimer = undefined; + } + // Clear the summarizer reference so it can be GC'd. The summarizer container itself + // is tracked by LoaderContainerTracker.trackedSummarizerContainers and will be + // disposed via that path (in tests) or when its own container is GC'd (in production). + // We deliberately do NOT call this.summarizer?.close() here, because doing so + // can trigger an async chain that calls back into the (already-disposed) interactive + // container's runtime, producing spurious "Runtime is closed" ContainerClose events. + this.summarizer = undefined; this._disposed = true; } diff --git a/packages/runtime/container-runtime/src/test/containerRuntime.extensions.spec.ts b/packages/runtime/container-runtime/src/test/containerRuntime.extensions.spec.ts index 6dad232f716b..af8d66a60ed1 100644 --- a/packages/runtime/container-runtime/src/test/containerRuntime.extensions.spec.ts +++ b/packages/runtime/container-runtime/src/test/containerRuntime.extensions.spec.ts @@ -323,6 +323,10 @@ describe("Runtime", () => { extension = runtime.acquireExtension(testExtensionId, TestExtensionFactory); }); + afterEach(() => { + runtime.dispose(); + }); + describe("connection status", () => { it("should return true when context is `Connected`", async () => { updateConnectionState(runtime, context, ConnectionState.Connected, "mockClientId"); @@ -383,6 +387,10 @@ describe("Runtime", () => { ); }); + afterEach(() => { + fallbackRuntime.dispose(); + }); + it("should fallback to canSendOps when context is `Connected`", async () => { updateConnectionState( fallbackRuntime, @@ -585,6 +593,7 @@ describe("Runtime", () => { false, "Extension should be disconnected after transition back to Disconnected", ); + readOnlyRuntime.dispose(); }); it("should handle connection type changes", async () => { @@ -682,6 +691,7 @@ describe("Runtime", () => { { type: "disconnected" }, "Second event should be disconnected", ); + fallbackRuntime.dispose(); }); }); }); diff --git a/packages/runtime/container-runtime/src/test/hardwareStats.spec.ts b/packages/runtime/container-runtime/src/test/hardwareStats.spec.ts index 547a30d897d5..4cd817ed8037 100644 --- a/packages/runtime/container-runtime/src/test/hardwareStats.spec.ts +++ b/packages/runtime/container-runtime/src/test/hardwareStats.spec.ts @@ -35,11 +35,12 @@ describe("Hardware Stats", () => { updateDirtyContainerState: (dirty: boolean) => {}, getLoadedFromVersion: () => undefined, }; + let containerRuntime: ContainerRuntime | undefined; const getDeviceSpecEvents = (): ITelemetryBaseEvent[] => mockLogger.events.filter((event) => event.eventName === "DeviceSpec"); - const loadContainer = async () => + const loadContainer = async (): Promise => ContainerRuntime.loadRuntime2({ context: mockContext as IContainerContext, registry: new FluidDataStoreRegistry([]), @@ -55,6 +56,7 @@ describe("Hardware Stats", () => { }); beforeEach(async () => { + containerRuntime = undefined; mockLogger = new MockLogger(); mockContext = { deltaManager: new MockDeltaManager(), @@ -67,6 +69,11 @@ describe("Hardware Stats", () => { }; }); + afterEach(() => { + containerRuntime?.dispose(); + containerRuntime = undefined; + }); + it("should generate correct hardware stats with regular navigator", async () => { const navigator = { deviceMemory: 10, @@ -78,7 +85,7 @@ describe("Hardware Stats", () => { assert.strictEqual(deviceMemory, 10, "incorrect deviceMemory value"); assert.strictEqual(hardwareConcurrency, 8, "incorrect hardwareConcurrency value"); - await loadContainer(); + containerRuntime = await loadContainer(); // checking telemetry const events = getDeviceSpecEvents(); @@ -100,7 +107,7 @@ describe("Hardware Stats", () => { assert.strictEqual(deviceMemory, undefined, "incorrect deviceMemory value"); assert.strictEqual(hardwareConcurrency, undefined, "incorrect hardwareConcurrency value"); - await loadContainer(); + containerRuntime = await loadContainer(); // checking telemetry const events = getDeviceSpecEvents(); @@ -121,7 +128,7 @@ describe("Hardware Stats", () => { assert.strictEqual(deviceMemory, undefined, "incorrect deviceMemory value"); assert.strictEqual(hardwareConcurrency, undefined, "incorrect hardwareConcurrency value"); - await loadContainer(); + containerRuntime = await loadContainer(); // checking telemetry const events = getDeviceSpecEvents(); diff --git a/packages/runtime/container-runtime/src/test/runtimeLayerCompatValidation.spec.ts b/packages/runtime/container-runtime/src/test/runtimeLayerCompatValidation.spec.ts index a101a6e7a2c7..1aedd2d4ad48 100644 --- a/packages/runtime/container-runtime/src/test/runtimeLayerCompatValidation.spec.ts +++ b/packages/runtime/container-runtime/src/test/runtimeLayerCompatValidation.spec.ts @@ -119,7 +119,7 @@ async function createAndLoadRuntime( ILayerCompatDetails: compatibilityDetails, }; - await ContainerRuntime.loadRuntime({ + const runtime = await ContainerRuntime.loadRuntime({ context: mockContext as IContainerContext, registryEntries: [], existing: false, @@ -127,6 +127,7 @@ async function createAndLoadRuntime( myProp: "myValue", }), }); + runtime.dispose(); } async function createAndLoadDataStore( diff --git a/packages/service-clients/odsp-client/.mocharc.cjs b/packages/service-clients/odsp-client/.mocharc.cjs index 6345db3514ce..45580c775c72 100644 --- a/packages/service-clients/odsp-client/.mocharc.cjs +++ b/packages/service-clients/odsp-client/.mocharc.cjs @@ -8,7 +8,4 @@ const getFluidTestMochaConfig = require("@fluid-internal/mocha-test-setup/mocharc-common"); const config = getFluidTestMochaConfig(__dirname); -// TODO: figure out why this package needs the --exit flag, tests might not be cleaning up correctly after themselves -// AB#7856 -config.exit = true; module.exports = config; diff --git a/packages/service-clients/odsp-client/src/test/odspClient.spec.ts b/packages/service-clients/odsp-client/src/test/odspClient.spec.ts index 2d613b44ea1b..6008f1e4022b 100644 --- a/packages/service-clients/odsp-client/src/test/odspClient.spec.ts +++ b/packages/service-clients/odsp-client/src/test/odspClient.spec.ts @@ -57,6 +57,7 @@ describe("OdspClient", () => { // const connectTimeoutMs = 5000; let client: OdspClient; let schema: ContainerSchema; + let createdContainers: { dispose(): void }[] = []; beforeEach(() => { client = createOdspClient(); @@ -67,6 +68,13 @@ describe("OdspClient", () => { }; }); + afterEach(() => { + for (const container of createdContainers) { + container.dispose(); + } + createdContainers = []; + }); + /** * Scenario: test when ODSP Client is instantiated correctly, it can create * a container successfully. @@ -75,9 +83,8 @@ describe("OdspClient", () => { * be returned. */ it("can create new ODSP container successfully", async () => { - const resourcesP = client.createContainer(schema); - - await assert.doesNotReject(resourcesP, () => true, "container cannot be created in ODSP"); + const { container } = await client.createContainer(schema); + createdContainers.push(container); }); /** @@ -89,6 +96,7 @@ describe("OdspClient", () => { */ it("created container is detached", async () => { const { container } = await client.createContainer(schema); + createdContainers.push(container); assert.strictEqual( container.attachState, AttachState.Detached, @@ -98,6 +106,7 @@ describe("OdspClient", () => { it("GC is disabled by default", async () => { const { container: container_defaultConfig } = await client.createContainer(schema); + createdContainers.push(container_defaultConfig); // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment const { sweepEnabled, throwOnTombstoneLoad } = // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access diff --git a/packages/test/local-server-stress-tests/package.json b/packages/test/local-server-stress-tests/package.json index 4b1216793a1c..d82e084cc273 100644 --- a/packages/test/local-server-stress-tests/package.json +++ b/packages/test/local-server-stress-tests/package.json @@ -28,7 +28,7 @@ "lint:fix": "fluid-build . --task eslint:fix --task format", "test": "npm run test:mocha", "test:coverage": "c8 npm test", - "test:mocha": "mocha \"lib/test/**/*.spec.*js\" --exit", + "test:mocha": "mocha \"lib/test/**/*.spec.*js\"", "test:mocha:verbose": "cross-env FLUID_TEST_VERBOSE=1 npm run test:mocha" }, "c8": { diff --git a/packages/test/local-server-tests/.mocharc.cjs b/packages/test/local-server-tests/.mocharc.cjs index 6345db3514ce..45580c775c72 100644 --- a/packages/test/local-server-tests/.mocharc.cjs +++ b/packages/test/local-server-tests/.mocharc.cjs @@ -8,7 +8,4 @@ const getFluidTestMochaConfig = require("@fluid-internal/mocha-test-setup/mocharc-common"); const config = getFluidTestMochaConfig(__dirname); -// TODO: figure out why this package needs the --exit flag, tests might not be cleaning up correctly after themselves -// AB#7856 -config.exit = true; module.exports = config; diff --git a/packages/test/local-server-tests/src/test/audience.spec.ts b/packages/test/local-server-tests/src/test/audience.spec.ts index fe631a49a6dc..5d09b23dd1c4 100644 --- a/packages/test/local-server-tests/src/test/audience.spec.ts +++ b/packages/test/local-server-tests/src/test/audience.spec.ts @@ -141,5 +141,9 @@ describe("Audience correctness", () => { container1.clientId, "client2's audience doesn't have client1", ); + + container1.dispose(); + container2.dispose(); + await deltaConnectionServer.close(); }); }); diff --git a/packages/test/local-server-tests/src/test/connectionMode.spec.ts b/packages/test/local-server-tests/src/test/connectionMode.spec.ts index e2ffa65497cd..82c3ee44fc71 100644 --- a/packages/test/local-server-tests/src/test/connectionMode.spec.ts +++ b/packages/test/local-server-tests/src/test/connectionMode.spec.ts @@ -112,8 +112,9 @@ describe("Logging Last Connection Mode ", () => { await loaderContainerTracker.ensureSynchronized(); }); - afterEach(() => { + afterEach(async () => { loaderContainerTracker.reset(); + await deltaConnectionServer.close(); }); it(`Logs the correct connection mode at disconnect`, async () => { diff --git a/packages/test/local-server-tests/src/test/decoupledCreate.spec.ts b/packages/test/local-server-tests/src/test/decoupledCreate.spec.ts index a6c493d63796..e5852aa788c4 100644 --- a/packages/test/local-server-tests/src/test/decoupledCreate.spec.ts +++ b/packages/test/local-server-tests/src/test/decoupledCreate.spec.ts @@ -127,6 +127,10 @@ describe("Scenario Test", () => { // ensure the newly loaded container has the data we expect. const entryPoint: FluidObject = await container2.getEntryPoint(); assert.strictEqual(entryPoint.ITestFluidObject?.root.get("someKey"), "someValue"); + container2.dispose(); } + + container.dispose(); + await deltaConnectionServer.close(); }); }); diff --git a/packages/test/local-server-tests/src/test/documentDirty.spec.ts b/packages/test/local-server-tests/src/test/documentDirty.spec.ts index 3308e5ec1cd4..0ee3a82b7ee6 100644 --- a/packages/test/local-server-tests/src/test/documentDirty.spec.ts +++ b/packages/test/local-server-tests/src/test/documentDirty.spec.ts @@ -517,7 +517,7 @@ describe("Document Dirty", () => { }); afterEach(async () => { - await deltaConnectionServer.webSocketServer.close(); + await deltaConnectionServer.close(); }); }); @@ -666,8 +666,9 @@ describe("Document Dirty", () => { return; }); - afterEach(() => { + afterEach(async () => { loaderContainerTracker.reset(); + await deltaConnectionServer.close(); }); it("clears the dirty flag after container is attached", async () => { diff --git a/packages/test/local-server-tests/src/test/handleResolution.spec.ts b/packages/test/local-server-tests/src/test/handleResolution.spec.ts index c9236551ed58..7d549c5515d5 100644 --- a/packages/test/local-server-tests/src/test/handleResolution.spec.ts +++ b/packages/test/local-server-tests/src/test/handleResolution.spec.ts @@ -139,6 +139,9 @@ describe("Multi-level handle access", () => { assert.strictEqual(dataObject2C2.root.get("key1"), "value1"); assert.strictEqual(dataObject3.root.get("key2"), "value2"); assert.strictEqual(dataObject3C2.root.get("key1"), "value1"); + + loaderContainerTracker.reset(); + await deltaConnectionServer.close(); }); it("validates that handles in a non-root data store and its dependencies resolve correctly when initial container is attached", async () => { @@ -214,6 +217,9 @@ describe("Multi-level handle access", () => { assert.strictEqual(dataObject2C2.root.get("key1"), "value1"); assert.strictEqual(dataObject3.root.get("key2"), "value2"); assert.strictEqual(dataObject3C2.root.get("key1"), "value1"); + + loaderContainerTracker.reset(); + await deltaConnectionServer.close(); }); it("validates that handles in a non-root data store and its dependencies resolve correctly after container close and rehydrate", async () => { @@ -275,5 +281,8 @@ describe("Multi-level handle access", () => { dataObject3C2.handle.get(), "Must be able to access data object 3 handle in container 2 after rehydrate", ); + + loaderContainerTracker.reset(); + await deltaConnectionServer.close(); }); }); diff --git a/packages/test/local-server-tests/src/test/loadFrozenContainerFromPendingState.spec.ts b/packages/test/local-server-tests/src/test/loadFrozenContainerFromPendingState.spec.ts index 0a270c0ca621..db4fe5be3efe 100644 --- a/packages/test/local-server-tests/src/test/loadFrozenContainerFromPendingState.spec.ts +++ b/packages/test/local-server-tests/src/test/loadFrozenContainerFromPendingState.spec.ts @@ -81,8 +81,14 @@ const initialize = async (): Promise<{ describe("loadFrozenContainerFromPendingState", () => { it("loadFrozenContainerFromPendingState", async () => { - const { container, ITestFluidObject, urlResolver, codeLoader, documentServiceFactory } = - await initialize(); + const { + container, + ITestFluidObject, + urlResolver, + codeLoader, + documentServiceFactory, + deltaConnectionServer, + } = await initialize(); for (let i = 0; i < 10; i++) { ITestFluidObject.root.set(`detached-${i}`, i); @@ -156,11 +162,21 @@ describe("loadFrozenContainerFromPendingState", () => { toComparableArray(frozenEntryPoint.ITestFluidObject.root), "Expected frozen container's data to remain unchanged after new changes in the original container.", ); + + container.dispose(); + frozenContainer.dispose(); + await deltaConnectionServer.close(); }); it("frozen container loads DDS", async () => { - const { container, ITestFluidObject, urlResolver, codeLoader, documentServiceFactory } = - await initialize(); + const { + container, + ITestFluidObject, + urlResolver, + codeLoader, + documentServiceFactory, + deltaConnectionServer, + } = await initialize(); const newSharedMap1 = SharedMap.create(ITestFluidObject.runtime); // Set a value while in local state. newSharedMap1.set("newKey", "newValue"); @@ -200,11 +216,21 @@ describe("loadFrozenContainerFromPendingState", () => { newSharedMap1Retrieved.get("newKey") === "newValue", "Expected newSharedMap1 to have key 'newKey' with value 'newValue', but it did not", ); + + container.dispose(); + frozenContainer.dispose(); + await deltaConnectionServer.close(); }); it("frozen container loads blob", async () => { - const { container, ITestFluidObject, urlResolver, codeLoader, documentServiceFactory } = - await initialize(); + const { + container, + ITestFluidObject, + urlResolver, + codeLoader, + documentServiceFactory, + deltaConnectionServer, + } = await initialize(); await container.attach(urlResolver.createCreateNewRequest("test")); const blobHandle = await ITestFluidObject.runtime.uploadBlob( stringToBuffer("test", "utf-8"), @@ -244,10 +270,20 @@ describe("loadFrozenContainerFromPendingState", () => { bufferToString(newBlobRetrieved, "utf-8") === "test", "Expected newBlobRetrieved to have value 'test', but it did not", ); + + container.dispose(); + frozenContainer.dispose(); + await deltaConnectionServer.close(); }); it("uploading blob on frozen container", async () => { - const { container, urlResolver, codeLoader, documentServiceFactory } = await initialize(); + const { + container, + urlResolver, + codeLoader, + documentServiceFactory, + deltaConnectionServer, + } = await initialize(); await container.attach(urlResolver.createCreateNewRequest("test")); const url = await container.getAbsoluteUrl(""); @@ -284,10 +320,20 @@ describe("loadFrozenContainerFromPendingState", () => { "Error message mismatch", ); } + + container.dispose(); + frozenContainer.dispose(); + await deltaConnectionServer.close(); }); it("trying to attach a frozen container", async () => { - const { container, urlResolver, codeLoader, documentServiceFactory } = await initialize(); + const { + container, + urlResolver, + codeLoader, + documentServiceFactory, + deltaConnectionServer, + } = await initialize(); await container.attach(urlResolver.createCreateNewRequest("test")); const url = await container.getAbsoluteUrl(""); @@ -322,5 +368,9 @@ describe("loadFrozenContainerFromPendingState", () => { "Error message mismatch", ); } + + container.dispose(); + frozenContainer.dispose(); + await deltaConnectionServer.close(); }); }); diff --git a/packages/test/local-server-tests/src/test/localTestServer.spec.ts b/packages/test/local-server-tests/src/test/localTestServer.spec.ts index db92eb8c5d84..0b8ce4662812 100644 --- a/packages/test/local-server-tests/src/test/localTestServer.spec.ts +++ b/packages/test/local-server-tests/src/test/localTestServer.spec.ts @@ -183,6 +183,6 @@ describe("LocalTestServer", () => { }); afterEach(async () => { - await deltaConnectionServer.webSocketServer.close(); + await deltaConnectionServer.close(); }); }); diff --git a/packages/test/local-server-tests/src/test/noDeltaStream.spec.ts b/packages/test/local-server-tests/src/test/noDeltaStream.spec.ts index a166974c2557..1e3b14a623ae 100644 --- a/packages/test/local-server-tests/src/test/noDeltaStream.spec.ts +++ b/packages/test/local-server-tests/src/test/noDeltaStream.spec.ts @@ -46,6 +46,7 @@ describe("No Delta Stream", () => { let deltaConnectionServer: ILocalDeltaConnectionServer; let loaderContainerTracker: LoaderContainerTracker; + let storageOnlyContainers: IContainer[] = []; async function createContainer(): Promise { const createDetachedContainerProps = createLoaderProps( @@ -62,7 +63,7 @@ describe("No Delta Stream", () => { return container; } - async function loadContainer(storageOnly: boolean, track = true): Promise { + async function loadContainer(storageOnly: boolean): Promise { const service = new LocalDocumentServiceFactory(deltaConnectionServer, { storageOnly }); const loaderProps = createLoaderProps( [[codeDetails, factory]], @@ -76,10 +77,12 @@ describe("No Delta Stream", () => { url: documentLoadUrl, }, }); - if (!storageOnly) { + if (storageOnly) { + storageOnlyContainers.push(container); + } else { loaderContainerTracker.addContainer(container); + await loaderContainerTracker.ensureSynchronized(); } - await loaderContainerTracker.ensureSynchronized(); return container; } @@ -92,6 +95,7 @@ describe("No Delta Stream", () => { new LocalResolver(), ); const container = await loader.resolve({ url: documentLoadUrl }); + loaderContainerTracker.addContainer(container); await loaderContainerTracker.ensureSynchronized(); return container; } @@ -99,6 +103,7 @@ describe("No Delta Stream", () => { beforeEach(async () => { deltaConnectionServer = LocalDeltaConnectionServer.create(); loaderContainerTracker = new LoaderContainerTracker(); + storageOnlyContainers = []; // Create a Container for the first client. const container = await createContainer(); @@ -115,6 +120,9 @@ describe("No Delta Stream", () => { }); afterEach(() => { + for (const c of storageOnlyContainers) { + c.dispose?.(); + } loaderContainerTracker.reset(); }); @@ -221,6 +229,6 @@ describe("No Delta Stream", () => { }); afterEach(async () => { - await deltaConnectionServer.webSocketServer.close(); + await deltaConnectionServer.close(); }); }); diff --git a/packages/test/local-server-tests/src/test/opsOnReconnect.spec.ts b/packages/test/local-server-tests/src/test/opsOnReconnect.spec.ts index 16ad95a722f9..afe8b148cac5 100644 --- a/packages/test/local-server-tests/src/test/opsOnReconnect.spec.ts +++ b/packages/test/local-server-tests/src/test/opsOnReconnect.spec.ts @@ -686,7 +686,7 @@ describe("Ops on Reconnect", () => { }); afterEach(async () => { - await deltaConnectionServer.webSocketServer.close(); + await deltaConnectionServer.close(); receivedValues = []; }); }); diff --git a/packages/test/local-server-tests/src/test/pendingLocalStateStore.spec.ts b/packages/test/local-server-tests/src/test/pendingLocalStateStore.spec.ts index 2e13ce9b9245..6408e07a44e7 100644 --- a/packages/test/local-server-tests/src/test/pendingLocalStateStore.spec.ts +++ b/packages/test/local-server-tests/src/test/pendingLocalStateStore.spec.ts @@ -30,6 +30,7 @@ describe("PendingLocalStateStore End-to-End Tests", () => { urlResolver: LocalResolver; codeLoader: ICodeDetailsLoader; loaderProps: ILoaderProps; + deltaConnectionServer: ReturnType; }> => { const deltaConnectionServer = LocalDeltaConnectionServer.create(); const { urlResolver, codeDetails, codeLoader, loaderProps } = createLoader({ @@ -55,6 +56,7 @@ describe("PendingLocalStateStore End-to-End Tests", () => { urlResolver, codeLoader, loaderProps, + deltaConnectionServer, }; }; @@ -63,8 +65,14 @@ describe("PendingLocalStateStore End-to-End Tests", () => { const store = new PendingLocalStateStore(); // Create container and add data - const { container, testFluidObject, urlResolver, codeLoader, loaderProps } = - await initializeContainer(); + const { + container, + testFluidObject, + urlResolver, + codeLoader, + loaderProps, + deltaConnectionServer, + } = await initializeContainer(); // Add data in detached state testFluidObject.root.set("detached-key1", "detached-value1"); @@ -139,14 +147,24 @@ describe("PendingLocalStateStore End-to-End Tests", () => { "offline-value2", "Additional offline data should be restored", ); + + container.dispose(); + frozenContainer.dispose(); + await deltaConnectionServer.close(); }); it("should handle container state deduplication in real scenarios", async () => { const store = new PendingLocalStateStore(); // Create container with initial data - const { container, testFluidObject, urlResolver, codeLoader, loaderProps } = - await initializeContainer(); + const { + container, + testFluidObject, + urlResolver, + codeLoader, + loaderProps, + deltaConnectionServer, + } = await initializeContainer(); // Add initial data testFluidObject.root.set("shared-key", "initial-value"); @@ -213,6 +231,10 @@ describe("PendingLocalStateStore End-to-End Tests", () => { `Op key ${i} should have correct value after deduplication`, ); } + + container.dispose(); + restoredContainer.dispose(); + await deltaConnectionServer.close(); }); }); @@ -221,7 +243,8 @@ describe("PendingLocalStateStore End-to-End Tests", () => { const store = new PendingLocalStateStore(); // Create container and simulate multiple sessions/snapshots for the same container - const { container, testFluidObject, urlResolver } = await initializeContainer(); + const { container, testFluidObject, urlResolver, deltaConnectionServer } = + await initializeContainer(); // Attach to get a consistent URL await container.attach(urlResolver.createCreateNewRequest("test-doc")); @@ -264,6 +287,9 @@ describe("PendingLocalStateStore End-to-End Tests", () => { assert.strictEqual(keys.length, 0, "Keys iterator should be empty"); assert.strictEqual(entriesArray.length, 0, "Entries iterator should be empty"); + + container.dispose(); + await deltaConnectionServer.close(); }); }); }); diff --git a/packages/test/local-server-tests/src/test/readonly.spec.ts b/packages/test/local-server-tests/src/test/readonly.spec.ts index f539918578ee..d576c11da6e0 100644 --- a/packages/test/local-server-tests/src/test/readonly.spec.ts +++ b/packages/test/local-server-tests/src/test/readonly.spec.ts @@ -20,6 +20,7 @@ import type { import { SharedMap, ISharedMap } from "@fluidframework/map/internal"; import type { IFluidDataStoreFactory } from "@fluidframework/runtime-definitions/internal"; import { LocalDeltaConnectionServer } from "@fluidframework/server-local-server"; +import { waitForContainerConnection } from "@fluidframework/test-utils/internal"; import { createLoader } from "../utils.js"; @@ -115,7 +116,11 @@ const runtimeFactory: IRuntimeFactory = { }, }; -async function createContainerAndGetLoadProps(): Promise { +async function createContainerAndGetLoadProps(): Promise< + ILoadExistingContainerProps & { + deltaConnectionServer: ReturnType; + } +> { const deltaConnectionServer = LocalDeltaConnectionServer.create(); const { loaderProps, codeDetails, urlResolver } = createLoader({ @@ -130,7 +135,7 @@ async function createContainerAndGetLoadProps(): Promise { @@ -164,12 +169,14 @@ describe("readonly", () => { entrypoint.DefaultDataStore.readonlyEventCount === 0, "shouldn't be any readonly events", ); + + container.dispose(); + await deltaConnectionServer.close(); }); it("Readonly is correct after container load", async () => { - const loadedContainer = await loadExistingContainer( - await createContainerAndGetLoadProps(), - ); + const { deltaConnectionServer, ...loadProps } = await createContainerAndGetLoadProps(); + const loadedContainer = await loadExistingContainer(loadProps); const entrypoint: FluidObject = await loadedContainer.getEntryPoint(); @@ -183,12 +190,14 @@ describe("readonly", () => { entrypoint.DefaultDataStore.readonlyEventCount === 0, "shouldn't be any readonly events", ); + + loadedContainer.dispose(); + await deltaConnectionServer.close(); }); it("Readonly is correct after datastore load and forceReadonly", async () => { - const loadedContainer = await loadExistingContainer( - await createContainerAndGetLoadProps(), - ); + const { deltaConnectionServer, ...loadProps } = await createContainerAndGetLoadProps(); + const loadedContainer = await loadExistingContainer(loadProps); const entrypoint: FluidObject = await loadedContainer.getEntryPoint(); @@ -199,17 +208,23 @@ describe("readonly", () => { loadedContainer.forceReadonly?.(true); + // Wait for the reconnect triggered by forceReadonly to complete so the + // socketConnectionTimeout (62s) is cleared before we close the server. + await waitForContainerConnection(loadedContainer); + assert(entrypoint.DefaultDataStore.isReadOnly() === true, "should be readonly"); assert( entrypoint.DefaultDataStore.readonlyEventCount === 1, "should be any readonly events", ); + + loadedContainer.dispose(); + await deltaConnectionServer.close(); }); it("Readonly is correct after forceReadonly before datastore load", async () => { - const loadedContainer = await loadExistingContainer( - await createContainerAndGetLoadProps(), - ); + const { deltaConnectionServer, ...loadProps } = await createContainerAndGetLoadProps(); + const loadedContainer = await loadExistingContainer(loadProps); loadedContainer.forceReadonly?.(true); @@ -225,5 +240,8 @@ describe("readonly", () => { entrypoint.DefaultDataStore.readonlyEventCount === 0, "shouldn't be any readonly events", ); + + loadedContainer.dispose(); + await deltaConnectionServer.close(); }); }); diff --git a/packages/test/local-server-tests/src/test/stagingMode.spec.ts b/packages/test/local-server-tests/src/test/stagingMode.spec.ts index 30398007d9e0..14684524d2ad 100644 --- a/packages/test/local-server-tests/src/test/stagingMode.spec.ts +++ b/packages/test/local-server-tests/src/test/stagingMode.spec.ts @@ -271,7 +271,7 @@ const catchUp = async ( ); }; -const createClients = async ( +const createClientsBase = async ( deltaConnectionServer: ILocalDeltaConnectionServer, ): Promise<{ original: { @@ -354,7 +354,7 @@ const createClients = async ( * loads DDSes created by `addDDS`. */ async function assertDeepConsistent( - clients: Awaited>, + clients: Awaited>, message: string, ): Promise { const { original, loaded } = clients; @@ -369,7 +369,7 @@ async function assertDeepConsistent( * Verify clients are consistent via their data representation from `enumerateDataSynchronous`. */ function assertConsistent( - clients: Awaited>, + clients: Awaited>, message: string, ): void { const { original, loaded } = clients; @@ -384,7 +384,7 @@ function assertConsistent( * Verify clients are not consistent via their data representation from `enumerateDataSynchronous`. */ function assertNotConsistent( - clients: Awaited>, + clients: Awaited>, message: string, ): void { const { original, loaded } = clients; @@ -427,15 +427,39 @@ async function ensureConnected(client: Client): Promise { } describe("Staging Mode", () => { + let deltaConnectionServer: ILocalDeltaConnectionServer; + let containersToDispose: IContainer[] = []; + + // Shadow the module-level createClients to track containers for disposal. + async function createClients( + server: ILocalDeltaConnectionServer, + ): Promise>> { + const clients = await createClientsBase(server); + containersToDispose.push(clients.original.container, clients.loaded.container); + return clients; + } + + beforeEach(() => { + deltaConnectionServer = LocalDeltaConnectionServer.create(); + containersToDispose = []; + }); + + afterEach(async () => { + for (const c of containersToDispose) { + if (!c.closed && !c.disposed) { + c.dispose(); + } + } + await deltaConnectionServer.close(); + }); + it("entering staging mode does not change the data model", async () => { - const deltaConnectionServer = LocalDeltaConnectionServer.create(); const clients = await createClients(deltaConnectionServer); clients.original.dataObject.enterStagingMode(); assertConsistent(clients, "states should match after branch"); }); it("blocks outbound changes", async () => { - const deltaConnectionServer = LocalDeltaConnectionServer.create(); const clients = await createClients(deltaConnectionServer); clients.original.dataObject.enterStagingMode(); clients.original.dataObject.makeEdit("branch-only"); @@ -459,7 +483,6 @@ describe("Staging Mode", () => { }); it("allows inbound changes to flow", async () => { - const deltaConnectionServer = LocalDeltaConnectionServer.create(); const clients = await createClients(deltaConnectionServer); clients.original.dataObject.enterStagingMode(); clients.original.dataObject.makeEdit("branch-only"); @@ -476,7 +499,6 @@ describe("Staging Mode", () => { }); it("commitChanges sends changes applied to other clients", async () => { - const deltaConnectionServer = LocalDeltaConnectionServer.create(); const clients = await createClients(deltaConnectionServer); const stagingControls = clients.original.dataObject.enterStagingMode(); @@ -503,7 +525,6 @@ describe("Staging Mode", () => { }); it("discardChanges rolls back all changes applied in staging mode", async () => { - const deltaConnectionServer = LocalDeltaConnectionServer.create(); const clients = await createClients(deltaConnectionServer); const stagingControls = clients.original.dataObject.enterStagingMode(); @@ -531,7 +552,6 @@ describe("Staging Mode", () => { // Analogous to the basic behavioral tests for staging mode above, but is worth testing separately as it involves // an attach op for the created DDS. it("enter staging mode, create dds, and merge", async () => { - const deltaConnectionServer = LocalDeltaConnectionServer.create(); const clients = await createClients(deltaConnectionServer); const branchData = clients.original.dataObject.enterStagingMode(); @@ -563,7 +583,6 @@ describe("Staging Mode", () => { for (const commit of [false, true]) { it(`${commit ? "commitChanges" : "discardChanges"} allows subsequent outbound changes to flow`, async () => { - const deltaConnectionServer = LocalDeltaConnectionServer.create(); const clients = await createClients(deltaConnectionServer); const stagingControls = clients.original.dataObject.enterStagingMode(); clients.original.dataObject.makeEdit("branch-only"); @@ -588,7 +607,6 @@ describe("Staging Mode", () => { } it("can be exited while disconnected and functionality is preserved", async () => { - const deltaConnectionServer = LocalDeltaConnectionServer.create(); const clients = await createClients(deltaConnectionServer); const stagingControls = clients.original.dataObject.enterStagingMode(); clients.original.dataObject.makeEdit("branch-only"); @@ -617,7 +635,6 @@ describe("Staging Mode", () => { squash: [undefined, false, true], })) { it(`respects squash=${squash} when exiting staging mode ${disconnectBeforeCommit ? "while disconnected" : ""}`, async () => { - const deltaConnectionServer = LocalDeltaConnectionServer.create(); const clients = await createClients(deltaConnectionServer); // Use Sinon to spy on the methods @@ -671,7 +688,6 @@ describe("Staging Mode", () => { } it("Aliasing a datastore not supported in staging mode", async () => { - const deltaConnectionServer = LocalDeltaConnectionServer.create(); const clients = await createClients(deltaConnectionServer); clients.original.dataObject.enterStagingMode(); diff --git a/packages/test/local-server-tests/src/test/synchronousDataStoreCreation.spec.ts b/packages/test/local-server-tests/src/test/synchronousDataStoreCreation.spec.ts index d96f926c640c..2118f80dd33f 100644 --- a/packages/test/local-server-tests/src/test/synchronousDataStoreCreation.spec.ts +++ b/packages/test/local-server-tests/src/test/synchronousDataStoreCreation.spec.ts @@ -262,5 +262,7 @@ describe("Scenario Test", () => { } container2.dispose(); } + + await deltaConnectionServer.close(); }); }); diff --git a/packages/test/mocha-test-setup/src/mochaHooks.ts b/packages/test/mocha-test-setup/src/mochaHooks.ts index d9bde8014ad1..5674bba1f8f2 100644 --- a/packages/test/mocha-test-setup/src/mochaHooks.ts +++ b/packages/test/mocha-test-setup/src/mochaHooks.ts @@ -16,6 +16,28 @@ import { pkgName } from "./packageVersion.js"; // https://v8.dev/docs/stack-trace-api Error.stackTraceLimit = Number.POSITIVE_INFINITY; +// Patch global.setTimeout to automatically unref() timers with delay > 10 seconds. +// This prevents long-running timers from keeping the process alive after tests complete. +// Specifically needed for N-1 compat packages (e.g. @fluidframework/container-runtime@2.83.0) +// which create 2-minute setTimeout timers inside Summarizer.runCore() Promise.race() calls +// that cannot be cancelled externally. unref() marks a timer as non-blocking for the event loop, +// so the process can exit naturally without waiting for those timers to fire. +{ + const origSetTimeout = globalThis.setTimeout; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (globalThis as any).setTimeout = function ( + fn: (...args: unknown[]) => void, + delay?: number, + ...args: unknown[] + ) { + const timer = origSetTimeout(fn, delay, ...args); + if (typeof delay === "number" && delay > 10_000) { + timer.unref?.(); + } + return timer; + }; +} + const testVariant = process.env.FLUID_TEST_VARIANT; const envLoggerProps = process.env.FLUID_LOGGER_PROPS != null diff --git a/packages/test/snapshots/.mocharc.cjs b/packages/test/snapshots/.mocharc.cjs index ce3d13cabcdd..5a8f697346d6 100644 --- a/packages/test/snapshots/.mocharc.cjs +++ b/packages/test/snapshots/.mocharc.cjs @@ -9,7 +9,4 @@ const getFluidTestMochaConfig = require("@fluid-internal/mocha-test-setup/mochar const config = getFluidTestMochaConfig(__dirname); config.ignore = config.spec + "/generate/**/*"; -// TODO: figure out why this package needs the --exit flag, tests might not be cleaning up correctly after themselves -// AB#7856 -config.exit = true; module.exports = config; diff --git a/packages/test/snapshots/src/replayMultipleFiles.ts b/packages/test/snapshots/src/replayMultipleFiles.ts index e32aefa5a6b6..f34175c664f6 100644 --- a/packages/test/snapshots/src/replayMultipleFiles.ts +++ b/packages/test/snapshots/src/replayMultipleFiles.ts @@ -406,7 +406,12 @@ async function processNode( worker.on("message", (message: string) => { if (message === "true") { + // Terminate the worker to release its resources (e.g. GC timers on loaded + // containers). Without this, the worker stays alive and its MessagePort + // keeps the parent event loop alive, preventing mocha from exiting. + void worker.terminate(); resolve(); + return; } if (workerData.mode === Mode.Compare) { const extra = diff --git a/packages/test/snapshots/src/test/serialized.spec.ts b/packages/test/snapshots/src/test/serialized.spec.ts index 5130d996a79a..c18a16c87bdd 100644 --- a/packages/test/snapshots/src/test/serialized.spec.ts +++ b/packages/test/snapshots/src/test/serialized.spec.ts @@ -39,6 +39,12 @@ import { getTestContent, skipOrFailIfTestContentMissing } from "../testContent.j describe(`Container Serialization Backwards Compatibility`, () => { const loaderContainerTracker = new LoaderContainerTracker(); const contentFolder = getTestContent("serializedContainerTestContent"); + const deltaConnectionServers: { close(): Promise }[] = []; + + after(async () => { + loaderContainerTracker.reset(); + await Promise.all(deltaConnectionServers.map(async (s) => s.close())); + }); // Ideally we would have each test call this.skip() but in this case they're created dynamically // based on the contents of the folder which might or might not exist, so this is the alternative @@ -164,6 +170,7 @@ describe(`Container Serialization Backwards Compatibility`, () => { function createTestLoaderProps(): ILoaderProps { const deltaConnectionServer = LocalDeltaConnectionServer.create(); + deltaConnectionServers.push(deltaConnectionServer); const documentServiceFactory = new LocalDocumentServiceFactory(deltaConnectionServer); const urlResolver = new LocalResolver(); diff --git a/packages/test/snapshots/src/validateSnapshots.ts b/packages/test/snapshots/src/validateSnapshots.ts index 729149f9fdb8..32ade6e89557 100644 --- a/packages/test/snapshots/src/validateSnapshots.ts +++ b/packages/test/snapshots/src/validateSnapshots.ts @@ -78,6 +78,7 @@ export async function validateSnapshots( const sourceDir = `${srcDir}/${file.name}`; const srcContent = fs.readFileSync(sourceDir, "utf-8"); + let container: IContainer | undefined; try { // This function will be called by the storage service when the container is snapshotted. When that happens, // validate that snapshot with the destination snapshot. @@ -98,7 +99,7 @@ export async function validateSnapshots( onSnapshotCb, ); - const container: IContainer = await loadContainer( + container = await loadContainer( new StaticStorageDocumentServiceFactory(storage), FileStorageDocumentName, true, @@ -119,6 +120,8 @@ export async function validateSnapshots( ); } throw e; + } finally { + container?.dispose(); } } diff --git a/packages/test/test-driver-definitions/src/interfaces.ts b/packages/test/test-driver-definitions/src/interfaces.ts index e6426857841f..7a598c051bcf 100644 --- a/packages/test/test-driver-definitions/src/interfaces.ts +++ b/packages/test/test-driver-definitions/src/interfaces.ts @@ -117,7 +117,7 @@ export interface ITestDriver { */ createContainerUrl(testId: string, containerUrl?: IResolvedUrl): Promise; - dispose?(): void; + dispose?(): void | Promise; } /** diff --git a/packages/test/test-drivers/src/localServerTestDriver.ts b/packages/test/test-drivers/src/localServerTestDriver.ts index 1ce52f2e0e58..a1a5caa502a0 100644 --- a/packages/test/test-drivers/src/localServerTestDriver.ts +++ b/packages/test/test-drivers/src/localServerTestDriver.ts @@ -73,9 +73,7 @@ export class LocalServerTestDriver implements ITestDriver { /** * Local server dispose flows are especially important to avoid leaking memory over the course of a test run. */ - dispose(): void { - this._server.close().catch(() => { - // TODO: We may want to log the error in the future. - }); + async dispose(): Promise { + await this._server.close(); } } diff --git a/packages/test/test-end-to-end-tests/src/test/.mocharc.cjs b/packages/test/test-end-to-end-tests/src/test/.mocharc.cjs index f57963c9448f..0926de130b10 100644 --- a/packages/test/test-end-to-end-tests/src/test/.mocharc.cjs +++ b/packages/test/test-end-to-end-tests/src/test/.mocharc.cjs @@ -9,10 +9,6 @@ const packageDir = `${__dirname}/../..`; const getFluidTestMochaConfig = require("@fluid-private/test-version-utils/mocharc-common"); const config = getFluidTestMochaConfig(packageDir); -// TODO: figure out why this package needs the --exit flag, tests might not be cleaning up correctly after themselves. -// AB#7856 -config.exit = true; - // Heuristic to decide if we're running against our internal r11s deployment in AKS: // driver set to 'r11s' + r11sEndpointName set to 'r11s' or not specified at all (if specified with a value other // than 'r11s' we're probably running against frs or local docker). diff --git a/packages/test/test-end-to-end-tests/src/test/benchmark/.mocharc.memory.cjs b/packages/test/test-end-to-end-tests/src/test/benchmark/.mocharc.memory.cjs index 3705e593ef19..d365c84c87ae 100644 --- a/packages/test/test-end-to-end-tests/src/test/benchmark/.mocharc.memory.cjs +++ b/packages/test/test-end-to-end-tests/src/test/benchmark/.mocharc.memory.cjs @@ -12,7 +12,6 @@ const packageDir = `${__dirname}/../../..`; const config = getFluidTestMochaConfig(packageDir); const newConfig = { "extends": "../.mocharc.cjs", - "exit": true, "fgrep": ["@Benchmark", "@MemoryUsage"], "node-option": ["expose-gc", "gc-global", "unhandled-rejections=strict"], // without leading "--" "recursive": true, diff --git a/packages/test/test-end-to-end-tests/src/test/benchmark/.mocharc.time.cjs b/packages/test/test-end-to-end-tests/src/test/benchmark/.mocharc.time.cjs index 9f51ea4740a4..be8e59983de2 100644 --- a/packages/test/test-end-to-end-tests/src/test/benchmark/.mocharc.time.cjs +++ b/packages/test/test-end-to-end-tests/src/test/benchmark/.mocharc.time.cjs @@ -12,7 +12,6 @@ const packageDir = `${__dirname}/../../..`; const config = getFluidTestMochaConfig(packageDir); const newConfig = { "extends": "../.mocharc.cjs", - "exit": true, "fgrep": ["@Benchmark", "@ExecutionTime"], "node-option": ["expose-gc", "gc-global", "unhandled-rejections=strict"], // without leading "--" "recursive": true, diff --git a/packages/test/test-end-to-end-tests/src/test/compression.spec.ts b/packages/test/test-end-to-end-tests/src/test/compression.spec.ts index 606eab839fbc..b1d0e4e82dd6 100644 --- a/packages/test/test-end-to-end-tests/src/test/compression.spec.ts +++ b/packages/test/test-end-to-end-tests/src/test/compression.spec.ts @@ -178,30 +178,44 @@ describeInstallVersions( requestAbsoluteVersions: [loaderWithoutCompressionField], }, /* timeoutMs: 3 minutes */ 180000, -)("Op Compression self-healing with old loader", (getProvider) => - compressionSuite(async () => { - const provider = getProvider(); +)("Op Compression self-healing with old loader", (getProvider) => { + let versionedProvider: ITestObjectProvider | undefined; + before(async function () { + const baseProvider = getProvider(); // Ensure support for endpoint names for r11s driver. ODSP might need similar help at some point if we have // scenarios that run into issues otherwise. const driverConfig = - provider.driver.endpointName !== undefined + baseProvider.driver.endpointName !== undefined ? { - r11s: { r11sEndpointName: provider.driver.endpointName }, + r11s: { r11sEndpointName: baseProvider.driver.endpointName }, } : undefined; - return getVersionedTestObjectProvider( + versionedProvider = await getVersionedTestObjectProvider( pkgVersion, // base version loaderWithoutCompressionField, // loader version { - type: provider.driver.type, + type: baseProvider.driver.type, version: pkgVersion, config: driverConfig, }, // driver version pkgVersion, // runtime version pkgVersion, // datastore runtime version ); - }), -); + }); + after(async function () { + if (versionedProvider !== undefined) { + versionedProvider.reset(); + await versionedProvider.dispose(); + versionedProvider = undefined; + } + }); + compressionSuite(async () => { + if (versionedProvider === undefined) { + throw new Error("versionedProvider not initialized"); + } + return versionedProvider; + }); +}); const generateRandomStringOfSize = (sizeInBytes: number): string => crypto.randomBytes(sizeInBytes / 2).toString("hex"); diff --git a/packages/test/test-end-to-end-tests/src/test/container.spec.ts b/packages/test/test-end-to-end-tests/src/test/container.spec.ts index 0d20fcf9d19f..e2cbc3192295 100644 --- a/packages/test/test-end-to-end-tests/src/test/container.spec.ts +++ b/packages/test/test-end-to-end-tests/src/test/container.spec.ts @@ -307,19 +307,26 @@ describeCompat("Container", "NoCompat", (getTestObjectProvider) => { runtimeFactory, ); - const container = await localTestObjectProvider.makeTestContainer(); - const dataObject = (await container.getEntryPoint()) as ITestDataObject; + try { + const container = await localTestObjectProvider.makeTestContainer(); + const dataObject = (await container.getEntryPoint()) as ITestDataObject; - let runCount = 0; + let runCount = 0; - dataObject._context.deltaManager.on("readonly", () => { - runCount++; - }); + dataObject._context.deltaManager.on("readonly", () => { + runCount++; + }); - container.forceReadonly?.(true); - assert.strictEqual(container.readOnlyInfo.readonly, true); + container.forceReadonly?.(true); + assert.strictEqual(container.readOnlyInfo.readonly, true); - assert.strictEqual(runCount, 1); + assert.strictEqual(runCount, 1); + } finally { + // Dispose all containers tracked by this local provider, clearing their GC + // sessionExpiryTimers (MAX_INT32 setTimeouts in GarbageCollector). Without this, + // the timers would outlive the test and prevent mocha from exiting cleanly. + localTestObjectProvider.reset(); + } }); it("getPendingLocalState() called on container", async () => { @@ -333,15 +340,23 @@ describeCompat("Container", "NoCompat", (getTestObjectProvider) => { runtimeFactory, ); - const container: ContainerAlpha = asLegacyAlpha( - await localTestObjectProvider.makeTestContainer(), - ); - const pendingString = await container.getPendingLocalState(); - container.close(); - assert.ok(pendingString); - const pendingLocalState: { url?: string } = JSON.parse(pendingString); - assert.strictEqual(container.closed, true); - assert.strictEqual(pendingLocalState.url, container.resolvedUrl?.url); + try { + const container: ContainerAlpha = asLegacyAlpha( + await localTestObjectProvider.makeTestContainer(), + ); + const pendingString = await container.getPendingLocalState(); + container.close(); + assert.ok(pendingString); + const pendingLocalState: { url?: string } = JSON.parse(pendingString); + assert.strictEqual(container.closed, true); + assert.strictEqual(pendingLocalState.url, container.resolvedUrl?.url); + } finally { + // Dispose all containers tracked by this local provider, clearing their GC + // sessionExpiryTimers (MAX_INT32 setTimeouts in GarbageCollector). Without this, + // the timers would outlive the test and prevent mocha from exiting cleanly. + // Note: container.close() above is not sufficient — only dispose() clears GC timers. + localTestObjectProvider.reset(); + } }); it("can call connect() and disconnect() on Container", async () => { @@ -385,68 +400,75 @@ describeCompat("Container", "NoCompat", (getTestObjectProvider) => { runtimeFactory, ); - const container1 = await localTestObjectProvider.makeTestContainer(); - await waitForContainerConnection(container1, false, { - durationMs: timeoutMs, - errorMsg: "container1 initial connect timeout", - }); - assert.strictEqual( - container1.connectionState, - ConnectionState.Connected, - "container is not connected after connected event fires", - ); - - const dataObject = (await container1.getEntryPoint()) as ITestDataObject; - const directory1 = dataObject._root; - directory1.set("key", "value"); - let value1 = await directory1.get("key"); - assert.strictEqual(value1, "value", "value1 is not set"); + try { + const container1 = await localTestObjectProvider.makeTestContainer(); + await waitForContainerConnection(container1, false, { + durationMs: timeoutMs, + errorMsg: "container1 initial connect timeout", + }); + assert.strictEqual( + container1.connectionState, + ConnectionState.Connected, + "container is not connected after connected event fires", + ); - const container2 = await localTestObjectProvider.loadTestContainer(); - await waitForContainerConnection(container2, false, { - durationMs: timeoutMs, - errorMsg: "container2 initial connect timeout", - }); - const dataObjectTest = (await container2.getEntryPoint()) as ITestDataObject; - const directory2 = dataObjectTest._root; - await localTestObjectProvider.ensureSynchronized(); - let value2 = await directory2.get("key"); - assert.strictEqual(value2, "value", "value2 is not set"); + const dataObject = (await container1.getEntryPoint()) as ITestDataObject; + const directory1 = dataObject._root; + directory1.set("key", "value"); + let value1 = await directory1.get("key"); + assert.strictEqual(value1, "value", "value1 is not set"); - let disconnectedEventFired = false; - container2.once("disconnected", () => { - disconnectedEventFired = true; - }); - container2.disconnect(); - assert( - disconnectedEventFired, - "disconnected event didn't fire when calling container.disconnect", - ); - assert.strictEqual( - container2.connectionState, - ConnectionState.Disconnected, - "container can't disconnect()", - ); + const container2 = await localTestObjectProvider.loadTestContainer(); + await waitForContainerConnection(container2, false, { + durationMs: timeoutMs, + errorMsg: "container2 initial connect timeout", + }); + const dataObjectTest = (await container2.getEntryPoint()) as ITestDataObject; + const directory2 = dataObjectTest._root; + await localTestObjectProvider.ensureSynchronized(); + let value2 = await directory2.get("key"); + assert.strictEqual(value2, "value", "value2 is not set"); + + let disconnectedEventFired = false; + container2.once("disconnected", () => { + disconnectedEventFired = true; + }); + container2.disconnect(); + assert( + disconnectedEventFired, + "disconnected event didn't fire when calling container.disconnect", + ); + assert.strictEqual( + container2.connectionState, + ConnectionState.Disconnected, + "container can't disconnect()", + ); - directory1.set("key", "new-value"); - value1 = await directory1.get("key"); - assert.strictEqual(value1, "new-value", "value1 is not changed"); + directory1.set("key", "new-value"); + value1 = await directory1.get("key"); + assert.strictEqual(value1, "new-value", "value1 is not changed"); - const valueChangePromise = timeoutPromise( - (resolve) => directory2.once("valueChanged", () => resolve()), - { durationMs: timeoutMs, errorMsg: "valueChanged timeout (expected error)" }, - ); - await assert.rejects(valueChangePromise, "valueChanged event fired while disconnected"); - value2 = await directory2.get("key"); - assert.notStrictEqual(value1, value2, "container2 processing ops after disconnect()"); + const valueChangePromise = timeoutPromise( + (resolve) => directory2.once("valueChanged", () => resolve()), + { durationMs: timeoutMs, errorMsg: "valueChanged timeout (expected error)" }, + ); + await assert.rejects(valueChangePromise, "valueChanged event fired while disconnected"); + value2 = await directory2.get("key"); + assert.notStrictEqual(value1, value2, "container2 processing ops after disconnect()"); - container2.connect(); - await timeoutPromise((resolve) => directory2.once("valueChanged", () => resolve()), { - durationMs: timeoutMs, - errorMsg: "valueChanged timeout after connect()", - }); - value2 = await directory2.get("key"); - assert.strictEqual(value1, value2, "container2 not processing ops after connect()"); + container2.connect(); + await timeoutPromise((resolve) => directory2.once("valueChanged", () => resolve()), { + durationMs: timeoutMs, + errorMsg: "valueChanged timeout after connect()", + }); + value2 = await directory2.get("key"); + assert.strictEqual(value1, value2, "container2 not processing ops after connect()"); + } finally { + // Dispose all containers tracked by this local provider, clearing their GC + // sessionExpiryTimers (MAX_INT32 setTimeouts in GarbageCollector). Without this, + // the timers would outlive the test and prevent mocha from exiting cleanly. + localTestObjectProvider.reset(); + } }); it("can cancel connect() with disconnect()", async () => { diff --git a/packages/test/test-end-to-end-tests/src/test/dataStoresNested.spec.ts b/packages/test/test-end-to-end-tests/src/test/dataStoresNested.spec.ts index 30e756107242..68b1c34587ba 100644 --- a/packages/test/test-end-to-end-tests/src/test/dataStoresNested.spec.ts +++ b/packages/test/test-end-to-end-tests/src/test/dataStoresNested.spec.ts @@ -38,6 +38,7 @@ describeCompat("Nested DataStores", "NoCompat", (getTestObjectProvider, apis) => const { ContainerRuntimeFactoryWithDefaultDataStore } = apis.containerRuntime; const { SharedMap } = apis.dds; + let driver: LocalServerTestDriver; let provider: ITestObjectProvider; let containers: IContainer[] = []; let summaryCollection: SummaryCollection | undefined; @@ -99,7 +100,7 @@ describeCompat("Nested DataStores", "NoCompat", (getTestObjectProvider, apis) => } beforeEach("getTestObjectProvider", async () => { - const driver = new LocalServerTestDriver(); + driver = new LocalServerTestDriver(); const registry = []; // ADO:7302 We need another test object provider provider = new TestObjectProvider( @@ -115,8 +116,9 @@ describeCompat("Nested DataStores", "NoCompat", (getTestObjectProvider, apis) => loader = provider.createLoader([[provider.defaultCodeDetails, runtimeFactory]]); }); - afterEach(() => { + afterEach(async () => { provider.reset(); + await driver.dispose(); for (const container of containers) { container.close(); } diff --git a/packages/test/test-end-to-end-tests/src/test/noDeltaStream.spec.ts b/packages/test/test-end-to-end-tests/src/test/noDeltaStream.spec.ts index b47cfd3ffb41..3d11345c8e14 100644 --- a/packages/test/test-end-to-end-tests/src/test/noDeltaStream.spec.ts +++ b/packages/test/test-end-to-end-tests/src/test/noDeltaStream.spec.ts @@ -139,6 +139,7 @@ describeCompat( } scenarioToSeqNum.set(scenario, initContainer.deltaManager.lastSequenceNumber); initContainer.close(); + initContainer.dispose?.(); } const containerUrl = await provider.driver.createContainerUrl( provider.documentId, @@ -187,6 +188,7 @@ describeCompat( }); scenarioToSeqNum.set(scenario, summaryContainer.deltaManager.lastSequenceNumber); summaryContainer.close(); + summaryContainer.dispose?.(); } } return { @@ -234,105 +236,120 @@ describeCompat( ...validationLoaderProps, request: { url: containerUrl }, }); - const validationDataObject = - await getContainerEntryPointBackCompat(validationContainer); + try { + const validationDataObject = + await getContainerEntryPointBackCompat(validationContainer); - const storageOnlyDsF = wrapObjectAndOverride( - provider.documentServiceFactory, - { - createDocumentService: { - policies: (ds) => { - const policies: IDocumentService["policies"] = { - ...ds.policies, - storageOnly: true, - }; - return policies; + const storageOnlyDsF = wrapObjectAndOverride( + provider.documentServiceFactory, + { + createDocumentService: { + policies: (ds) => { + const policies: IDocumentService["policies"] = { + ...ds.policies, + storageOnly: true, + }; + return policies; + }, }, }, - }, - ); + ); - const storageOnlyLoaderProps = createLoaderProps( - [ + const storageOnlyLoaderProps = createLoaderProps( [ - provider.defaultCodeDetails, - provider.createFluidEntryPoint(testContainerConfigDisabled), + [ + provider.defaultCodeDetails, + provider.createFluidEntryPoint(testContainerConfigDisabled), + ], ], - ], - storageOnlyDsF, - provider.urlResolver, - ); - - if (testConfig.opsUpToSeqNumber !== undefined) { - // Define sequenceNumber if opsUpToSeqNumber is set, otherwise leave undefined - const sequenceNumber = testConfig.opsUpToSeqNumber ? lastKnownSeqNum : undefined; - const storageOnlyContainer = await loadContainerPaused( - storageOnlyLoaderProps, - { - url: containerUrl, - headers: { - [LoaderHeader.loadMode]: testConfig.loadOptions, - }, - }, - sequenceNumber, + storageOnlyDsF, + provider.urlResolver, ); - const deltaManager = storageOnlyContainer.deltaManager; - const loadedSeqNum = deltaManager.lastSequenceNumber; - - if (testConfig.opsUpToSeqNumber === true) { - // If we tried to freeze after loading a specific sequence number, the loaded sequence number should be the same as the last known sequence number. - assert.strictEqual( - loadedSeqNum, - lastKnownSeqNum, - "loadedSeqNum === lastKnownSeqNum", - ); - } - // The sequence number should still be the same as when we loaded. - assert.strictEqual( - deltaManager.lastSequenceNumber, - loadedSeqNum, - "deltaManager.lastSequenceNumber === loadedSeqNum", - ); - } else { - const storageOnlyContainer = await loadExistingContainer({ - ...storageOnlyLoaderProps, - request: { - url: containerUrl, - headers: { - [LoaderHeader.loadMode]: testConfig.loadOptions, + if (testConfig.opsUpToSeqNumber !== undefined) { + // Define sequenceNumber if opsUpToSeqNumber is set, otherwise leave undefined + const sequenceNumber = testConfig.opsUpToSeqNumber ? lastKnownSeqNum : undefined; + const storageOnlyContainer = await loadContainerPaused( + storageOnlyLoaderProps, + { + url: containerUrl, + headers: { + [LoaderHeader.loadMode]: testConfig.loadOptions, + }, }, - }, - }); + sequenceNumber, + ); + try { + const deltaManager = storageOnlyContainer.deltaManager; + const loadedSeqNum = deltaManager.lastSequenceNumber; - const deltaManager = storageOnlyContainer.deltaManager; + if (testConfig.opsUpToSeqNumber === true) { + // If we tried to freeze after loading a specific sequence number, the loaded sequence number should be the same as the last known sequence number. + assert.strictEqual( + loadedSeqNum, + lastKnownSeqNum, + "loadedSeqNum === lastKnownSeqNum", + ); + } + // The sequence number should still be the same as when we loaded. + assert.strictEqual( + deltaManager.lastSequenceNumber, + loadedSeqNum, + "deltaManager.lastSequenceNumber === loadedSeqNum", + ); + } finally { + storageOnlyContainer.close(); + storageOnlyContainer.dispose?.(); + } + } else { + const storageOnlyContainer = await loadExistingContainer({ + ...storageOnlyLoaderProps, + request: { + url: containerUrl, + headers: { + [LoaderHeader.loadMode]: testConfig.loadOptions, + }, + }, + }); + try { + const deltaManager = storageOnlyContainer.deltaManager; - storageOnlyContainer.connect(); - assert.strictEqual(deltaManager.active, false, "deltaManager.active"); - assert.ok( - deltaManager.readOnlyInfo.readonly, - "deltaManager.readOnlyInfo.readonly", - ); - assert.ok( - deltaManager.readOnlyInfo.permissions, - "deltaManager.readOnlyInfo.permissions", - ); - assert.ok( - deltaManager.readOnlyInfo.storageOnly, - "deltaManager.readOnlyInfo.storageOnly", - ); + storageOnlyContainer.connect(); + assert.strictEqual(deltaManager.active, false, "deltaManager.active"); + assert.ok( + deltaManager.readOnlyInfo.readonly, + "deltaManager.readOnlyInfo.readonly", + ); + assert.ok( + deltaManager.readOnlyInfo.permissions, + "deltaManager.readOnlyInfo.permissions", + ); + assert.ok( + deltaManager.readOnlyInfo.storageOnly, + "deltaManager.readOnlyInfo.storageOnly", + ); - const storageOnlyDataObject = - await getContainerEntryPointBackCompat(storageOnlyContainer); - for (const key of validationDataObject.root.keys()) { - assert.strictEqual( - storageOnlyDataObject.root.get(key), - storageOnlyDataObject.root.get(key), - `${storageOnlyDataObject.root.get(key)} !== ${storageOnlyDataObject.root.get( - key, - )}`, - ); + const storageOnlyDataObject = + await getContainerEntryPointBackCompat( + storageOnlyContainer, + ); + for (const key of validationDataObject.root.keys()) { + assert.strictEqual( + storageOnlyDataObject.root.get(key), + storageOnlyDataObject.root.get(key), + `${storageOnlyDataObject.root.get(key)} !== ${storageOnlyDataObject.root.get( + key, + )}`, + ); + } + } finally { + storageOnlyContainer.close(); + storageOnlyContainer.dispose?.(); + } } + } finally { + validationContainer.close(); + validationContainer.dispose?.(); } } }); diff --git a/packages/test/test-utils/src/loaderContainerTracker.ts b/packages/test/test-utils/src/loaderContainerTracker.ts index a414e7fb869f..6863e18c4f58 100644 --- a/packages/test/test-utils/src/loaderContainerTracker.ts +++ b/packages/test/test-utils/src/loaderContainerTracker.ts @@ -54,6 +54,26 @@ export class LoaderContainerTracker implements IOpProcessingController { private readonly containers = new Map(); private lastProposalSeqNum: number = 0; + /** + * Summarizer containers are tracked separately from interactive containers. They are excluded from the main + * `containers` map (and therefore from `ensureSynchronized`) because they run on a separate client that is + * not part of the test's op flow control. However, we still need to dispose them during `reset()` to clear + * their GC sessionExpiryTimers (MAX_INT32 setTimeout in GarbageCollector). + * + * This matters especially in compat testing: older versions of ContainerRuntime (e.g. N-1) have a + * SummaryManager.dispose() that does NOT close the spawned summarizer container. When the parent container + * is disposed via reset(), the N-1 SummaryManager leaves the summarizer container alive. Without explicit + * tracking here, those containers' timers prevent mocha from exiting. + * + * In the current version, SummaryManager.dispose() does call summarizer.close(), so the summarizer container + * is already cleaned up via the parent chain. Calling dispose() again here is idempotent. + * + * reset() calls dispose() only (not close()) on these containers because close() emits a ContainerClose + * telemetry event that the tracker would record as an unexpected error when the container is already in an + * errored state (e.g. tests that deliberately throw from mixinSummaryHandler). + */ + private readonly trackedSummarizerContainers = new Set(); + constructor(private readonly syncSummarizerClients: boolean = false) {} /** @@ -115,11 +135,17 @@ export class LoaderContainerTracker implements IOpProcessingController { containerWithClone.clone = patch(containerWithClone.clone); } - // ignore summarizer + // Summarizer containers (non-interactive clients) are excluded from the main `containers` map + // because they must not participate in ensureSynchronized() — they process ops independently. + // However, they ARE tracked in a separate set so reset() can dispose them and clear their timers. + // + // When syncSummarizerClients is true the caller explicitly wants to synchronize summarizers too, + // so in that case we fall through and add the container to the main map as usual. if ( !container.deltaManager.clientDetails.capabilities.interactive && !this.syncSummarizerClients ) { + this.trackedSummarizerContainers.add(container); return; } @@ -204,6 +230,29 @@ export class LoaderContainerTracker implements IOpProcessingController { } this.containers.clear(); + // Dispose summarizer containers tracked separately from interactive containers. This is critical for + // compat scenarios using older ContainerRuntime versions (e.g. N-1): those versions have a + // SummaryManager.dispose() that does NOT close the spawned summarizer container. Consequently, when + // the parent interactive container is disposed above, the summarizer container stays alive with its + // GarbageCollector.sessionExpiryTimer (MAX_INT32 setTimeout) still running — preventing mocha from + // exiting. + // + // In the current version, SummaryManager.dispose() already calls summarizer.close(), so the + // container would already be closed when we reach this point; dispose() on an already-closed + // container is idempotent. + // + // IMPORTANT: We call dispose() only (not close() first) because the summarizer container may already + // be in an errored/closed state when reset() is called — e.g. when a test deliberately throws from + // mixinSummaryHandler. In that case, calling close() emits a ContainerClose telemetry event with + // the prior error as its cause, which the LoaderContainerTracker would then pick up as an unexpected + // error in the "Verify Container Telemetry" afterEach hook and fail the test. + // dispose() skips the ContainerClose telemetry path and goes directly to ContainerRuntime.dispose() + // → GarbageCollector.dispose() → sessionExpiryTimer.clear(), which is all we need here. + for (const container of this.trackedSummarizerContainers) { + container.dispose?.(); + } + this.trackedSummarizerContainers.clear(); + // REVIEW: do we need to unpatch the loaders? } diff --git a/packages/test/test-utils/src/testObjectProvider.ts b/packages/test/test-utils/src/testObjectProvider.ts index 79c9860135e9..1c5711c8703c 100644 --- a/packages/test/test-utils/src/testObjectProvider.ts +++ b/packages/test/test-utils/src/testObjectProvider.ts @@ -234,6 +234,17 @@ export interface ITestObjectProvider { * Resets and closes all tracked containers and class states. */ reset(): void; + + /** + * Disposes all drivers used by this provider, closing their underlying local servers (and any timers + * they hold, such as DeliLambda.readClientIdleTimer). + * + * Must be called during test teardown (after `reset()`) to allow the process to exit cleanly. + * Use this instead of `provider.driver.dispose?.()` directly, because some provider implementations + * (e.g. {@link TestObjectProviderWithVersionedLoad}) maintain more than one driver — one for creating + * containers and one for loading them — and only `driver` getter exposes a single one. + */ + dispose(): Promise; } /** @@ -754,6 +765,13 @@ export class TestObjectProvider implements ITestObjectProvider { this._loaderContainerTracker.reset(); this._loaderContainerTracker = new LoaderContainerTracker(syncSummarizerClients); } + + /** + * {@inheritDoc ITestObjectProvider.dispose} + */ + public async dispose(): Promise { + await this.driver.dispose?.(); + } } /** @@ -1142,6 +1160,22 @@ export class TestObjectProviderWithVersionedLoad implements ITestObjectProvider this._loaderContainerTracker.reset(); this._loaderContainerTracker = new LoaderContainerTracker(syncSummarizerClients); } + + /** + * {@inheritDoc ITestObjectProvider.dispose} + * + * @privateremarks + * This override disposes BOTH drivers. Unlike the base {@link TestObjectProvider} which only has a single + * driver, this class maintains two separate drivers — one for creating containers and one for loading + * them — each backed by its own {@link LocalDeltaConnectionServer}. Each server independently holds a + * `DeliLambda.readClientIdleTimer` (60-second setInterval). Calling `provider.driver.dispose?.()` from + * the outside only disposes whichever driver the `driver` getter currently returns, leaving the other + * server's timer alive and preventing mocha from exiting. + */ + public async dispose(): Promise { + await this.driverForCreating.dispose?.(); + await this.driverForLoading.dispose?.(); + } } /** diff --git a/packages/test/test-version-utils/src/describeCompat.ts b/packages/test/test-version-utils/src/describeCompat.ts index ff7a51a0f885..be1c81867aae 100644 --- a/packages/test/test-version-utils/src/describeCompat.ts +++ b/packages/test/test-version-utils/src/describeCompat.ts @@ -175,11 +175,21 @@ function createCompatSuite( // which is problematic for suites that run a large number of test cases (usually combintorially generated). // Heap snapshots for a large number of suites help detect bugs with leaking objects across suites, // which is problematic for issues that tend to get hit "later in the overall test run". - after("Cleanup TestObjectProvider", function () { + after("Cleanup TestObjectProvider", async function () { if (provider === undefined) { throw new Error("Expected provider to be set up by before hook"); } - provider.driver.dispose?.(); + // Reset (dispose all containers) before disposing the drivers, to ensure containers + // are torn down cleanly before their underlying server is shut down. + // This is especially important for GC sessionExpiryTimers: disposing containers + // triggers ContainerRuntime.dispose() → GarbageCollector.dispose() → timer.clear(). + provider.reset(); + // Use provider.dispose() rather than provider.driver.dispose?.() so that all drivers + // are disposed. TestObjectProviderWithVersionedLoad has two drivers (one for creating, + // one for loading containers) and exposes only one via the `driver` getter; calling + // provider.dispose() ensures both LocalDeltaConnectionServer instances are closed and + // their DeliLambda.readClientIdleTimer setIntervals are cleared. + await provider.dispose(); provider = undefined; Object.defineProperty(this, "__fluidTestProvider", { get: () => { diff --git a/packages/test/test-version-utils/src/describeWithVersions.ts b/packages/test/test-version-utils/src/describeWithVersions.ts index b2fe3d6ba0b8..b2bc115ad13a 100644 --- a/packages/test/test-version-utils/src/describeWithVersions.ts +++ b/packages/test/test-version-utils/src/describeWithVersions.ts @@ -139,11 +139,22 @@ function createTestSuiteWithInstalledVersion( }); // See remarks in `createCompatSuite` for cleanup justification + tips on debugging memory leaks - after("Cleanup TestObjectProvider", function () { + after("Cleanup TestObjectProvider", async function () { if (provider === undefined) { - throw new Error("Expected provider to be set up by before hook"); + // The "before" hook failed (e.g. package installation error), so there's nothing to clean up. + return; } - provider.driver.dispose?.(); + // Reset (dispose all containers) before disposing the drivers, to ensure containers + // are torn down cleanly before their underlying server is shut down. + // This is especially important for GC sessionExpiryTimers: disposing containers + // triggers ContainerRuntime.dispose() → GarbageCollector.dispose() → timer.clear(). + provider.reset(); + // Use provider.dispose() rather than provider.driver.dispose?.() so that all drivers + // are disposed. TestObjectProviderWithVersionedLoad has two drivers (one for creating, + // one for loading containers) and exposes only one via the `driver` getter; calling + // provider.dispose() ensures both LocalDeltaConnectionServer instances are closed and + // their DeliLambda.readClientIdleTimer setIntervals are cleared. + await provider.dispose(); provider = undefined; Object.defineProperty(this, "__fluidTestProvider", { get: () => {