diff --git a/packages/runtime/container-runtime/src/containerRuntime.ts b/packages/runtime/container-runtime/src/containerRuntime.ts index eb024f6d4087..79a191f368e3 100644 --- a/packages/runtime/container-runtime/src/containerRuntime.ts +++ b/packages/runtime/container-runtime/src/containerRuntime.ts @@ -1890,27 +1890,20 @@ export class ContainerRuntime this.mc.config.getNumber("Fluid.ContainerRuntime.StagingModeAutoFlushThreshold") ?? runtimeOptions.stagingModeAutoFlushThreshold ?? defaultStagingModeAutoFlushThreshold; - // BatchId tracking powers DuplicateBatchDetector (catching forked-container duplicates) - // and is also a prerequisite for the Offline Load feature. It is enabled by default in - // TurnBased mode; the kill-switch below allows disabling it without a code change if a - // regression is observed. Offline Load still requires TurnBased, so consumers that - // explicitly opt into it with FlushMode.Immediate continue to get a UsageError. - const offlineLoadRequested = - this.mc.config.getBoolean("Fluid.Container.enableOfflineFull") === true; - const disableBatchIdTracking = - this.mc.config.getBoolean("Fluid.ContainerRuntime.DisableBatchIdTracking") === true; - - if (offlineLoadRequested && this._flushMode !== FlushMode.TurnBased) { + this.batchIdTrackingEnabled = + this.mc.config.getBoolean("Fluid.Container.enableOfflineFull") ?? + this.mc.config.getBoolean("Fluid.ContainerRuntime.enableBatchIdTracking") ?? + false; + + if (this.batchIdTrackingEnabled && this._flushMode !== FlushMode.TurnBased) { const error = new UsageError("Offline mode is only supported in turn-based mode"); this.closeFn(error); throw error; } - this.batchIdTrackingEnabled = - !disableBatchIdTracking && this._flushMode === FlushMode.TurnBased; - - // DuplicateBatchDetector maintains a cache of all batchIds/sequenceNumbers within the - // collab window. Skip allocating it when batchId tracking is off. + // DuplicateBatchDetection is only enabled if Offline Load is enabled + // It maintains a cache of all batchIds/sequenceNumbers within the collab window. + // Don't waste resources doing so if not needed. if (this.batchIdTrackingEnabled) { this.duplicateBatchDetector = new DuplicateBatchDetector(recentBatchInfo); } diff --git a/packages/runtime/container-runtime/src/opLifecycle/duplicateBatchDetector.ts b/packages/runtime/container-runtime/src/opLifecycle/duplicateBatchDetector.ts index b90c0c5eea5c..bb79197292b3 100644 --- a/packages/runtime/container-runtime/src/opLifecycle/duplicateBatchDetector.ts +++ b/packages/runtime/container-runtime/src/opLifecycle/duplicateBatchDetector.ts @@ -28,16 +28,6 @@ export class DuplicateBatchDetector { */ private readonly batchIdsBySeqNum = new Map(); - /** - * Number of inbound batches processed since the last summary. Reset by getRecentBatchInfoForSummary. - */ - private processedBatchCount = 0; - - /** - * Largest tracked-batch count observed since the last summary. Reset by getRecentBatchInfoForSummary. - */ - private peakTrackedBatchCount = 0; - /** * Initialize from snapshot data if provided - otherwise initialize empty */ @@ -47,7 +37,6 @@ export class DuplicateBatchDetector { this.batchIdsBySeqNum.set(seqNum, batchId); this.seqNumByBatchId.set(batchId, seqNum); } - this.peakTrackedBatchCount = this.batchIdsBySeqNum.size; } } @@ -61,7 +50,6 @@ export class DuplicateBatchDetector { batchStart: BatchStartInfo, ): { duplicate: true; otherSequenceNumber: number } | { duplicate: false } { const { sequenceNumber, minimumSequenceNumber } = batchStart.keyMessage; - this.processedBatchCount++; // Glance at this batch's MSN. Any batchIds we're tracking with a lower sequence number are now safe to forget. // Why? Because any other client holding the same batch locally would have seen the earlier batch and closed before submitting its duplicate. @@ -92,9 +80,6 @@ export class DuplicateBatchDetector { // Add new batch this.batchIdsBySeqNum.set(sequenceNumber, batchId); this.seqNumByBatchId.set(batchId, sequenceNumber); - if (this.batchIdsBySeqNum.size > this.peakTrackedBatchCount) { - this.peakTrackedBatchCount = this.batchIdsBySeqNum.size; - } return { duplicate: false }; } @@ -123,22 +108,16 @@ export class DuplicateBatchDetector { public getRecentBatchInfoForSummary( telemetryContext?: ITelemetryContext, ): [number, string][] | undefined { - if (telemetryContext !== undefined) { - const prefix = "fluid_DuplicateBatchDetector_"; - telemetryContext.set(prefix, "recentBatchCount", this.batchIdsBySeqNum.size); - telemetryContext.set(prefix, "peakRecentBatchCount", this.peakTrackedBatchCount); - telemetryContext.set(prefix, "processedBatchCount", this.processedBatchCount); - } - - // Reset per-window perf counters so each summary covers only the activity since the - // previous one. Peak resets to the current size (the floor for the next window). - this.processedBatchCount = 0; - this.peakTrackedBatchCount = this.batchIdsBySeqNum.size; - if (this.batchIdsBySeqNum.size === 0) { return undefined; } + telemetryContext?.set( + "fluid_DuplicateBatchDetector_", + "recentBatchCount", + this.batchIdsBySeqNum.size, + ); + return [...this.batchIdsBySeqNum.entries()]; } } diff --git a/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts b/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts index 96ae249eb4af..716a3d81223f 100644 --- a/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts +++ b/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts @@ -437,7 +437,11 @@ describe("Runtime", () => { let batchEnd = 0; let callsToEnsure = 0; const { runtime: containerRuntime } = await ContainerRuntime.loadRuntime2({ - context: getMockContext() as IContainerContext, + context: getMockContext({ + settings: { + "Fluid.ContainerRuntime.enableBatchIdTracking": true, + }, + }) as IContainerContext, registry: new FluidDataStoreRegistry([]), existing: false, runtimeOptions: {}, @@ -477,19 +481,13 @@ describe("Runtime", () => { assert.strictEqual(containerRuntime.isDirty, false); }); - // BatchId tracking is on by default (TurnBased mode); the kill-switch suppresses stamping. - for (const variant of [ - { name: "default (no settings)", settings: {}, expectStamped: true }, - { - name: "kill-switch active", - settings: { "Fluid.ContainerRuntime.DisableBatchIdTracking": true }, - expectStamped: false, - }, - ]) - it(`Replaying ops should resend in correct order, with batch ID if applicable (${variant.name})`, async () => { + for (const enableBatchIdTracking of [true, undefined]) + it("Replaying ops should resend in correct order, with batch ID if applicable", async () => { const { runtime } = await ContainerRuntime.loadRuntime2({ context: getMockContext({ - settings: variant.settings, + settings: { + "Fluid.ContainerRuntime.enableBatchIdTracking": enableBatchIdTracking, // batchId only stamped if true + }, }) as IContainerContext, registry: new FluidDataStoreRegistry([]), existing: false, @@ -527,7 +525,7 @@ describe("Runtime", () => { ); } - if (variant.expectStamped) { + if (enableBatchIdTracking === true) { assert( batchIdMatchesUnsentFormat( // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access @@ -923,20 +921,8 @@ describe("Runtime", () => { { batch: false }, ]; - // In TurnBased mode batchId tracking is on by default, so resubmitted batches - // will also carry a batchId on their first message. Strip it before comparing - // — this test cares about batch boundaries, not batchId stamping. - const normalizedMetadata = submittedOpsMetadata.map((m) => { - if (m === undefined) { - return undefined; - } - // eslint-disable-next-line @typescript-eslint/no-unused-vars, @typescript-eslint/no-unsafe-assignment - const { batchId: _ignored, ...rest } = m as { batchId?: string }; - return Object.keys(rest).length === 0 ? undefined : rest; - }); - assert.deepStrictEqual( - normalizedMetadata, + submittedOpsMetadata, expectedBatchMetadata, "batch metadata does not match", ); @@ -2357,19 +2343,13 @@ describe("Runtime", () => { }); describe("Duplicate Batch Detection", () => { - // BatchId tracking is on by default (TurnBased); the kill-switch suppresses detection. - for (const variant of [ - { name: "default (no settings)", settings: {}, expectDetection: true }, - { - name: "kill-switch active", - settings: { "Fluid.ContainerRuntime.DisableBatchIdTracking": true }, - expectDetection: false, - }, - ]) { - it(`DuplicateBatchDetector reflects batch-id tracking enablement (${variant.name})`, async () => { + for (const enableBatchIdTracking of [undefined, true]) { + it(`DuplicateBatchDetector is disabled if Batch Id Tracking isn't needed (${enableBatchIdTracking === true ? "ENABLED" : "DISABLED"})`, async () => { const { runtime: containerRuntime } = await ContainerRuntime.loadRuntime2({ context: getMockContext({ - settings: variant.settings, + settings: { + "Fluid.ContainerRuntime.enableBatchIdTracking": enableBatchIdTracking, + }, }) as IContainerContext, registry: new FluidDataStoreRegistry([]), existing: false, @@ -2390,9 +2370,8 @@ describe("Runtime", () => { false, ); // Process a duplicate batch "batchId1" with different seqNum 234 - const assertThrowsOnlyIfExpected = variant.expectDetection - ? assert.throws - : assert.doesNotThrow; + const assertThrowsOnlyIfExpected = + enableBatchIdTracking === true ? assert.throws : assert.doesNotThrow; const errorPredicate = (e: Error) => e.message === "Duplicate batch - The same batch was sequenced twice"; assertThrowsOnlyIfExpected( @@ -2408,76 +2387,20 @@ describe("Runtime", () => { ); }, errorPredicate, - "Expected duplicate batch detection to match enablement", + "Expected duplicate batch detection to match Offline Load enablement", ); }); } - it("Default-on tracking is silently skipped in FlushMode.Immediate (no UsageError)", async () => { - const { runtime: containerRuntime } = await ContainerRuntime.loadRuntime2({ - context: getMockContext() as IContainerContext, - registry: new FluidDataStoreRegistry([]), - existing: false, - runtimeOptions: { - flushMode: FlushMode.Immediate, - enableRuntimeIdCompressor: "on", - }, - provideEntryPoint: mockProvideEntryPoint, - }); - // Sending a duplicate batchId should not throw because tracking is inactive in Immediate mode. - containerRuntime.process( - { - sequenceNumber: 123, - type: MessageType.Operation, - contents: { type: ContainerMessageType.Rejoin, contents: undefined }, - metadata: { batchId: "batchId1" }, - } satisfies Partial as ISequencedDocumentMessage, - false, - ); - assert.doesNotThrow(() => - containerRuntime.process( - { - sequenceNumber: 234, - type: MessageType.Operation, - contents: { type: ContainerMessageType.Rejoin, contents: undefined }, - metadata: { batchId: "batchId1" }, - } satisfies Partial as ISequencedDocumentMessage, - false, - ), - ); - }); - - it("Offline Load opt-in still errors in FlushMode.Immediate (back-compat)", async () => { - const containerErrors: ICriticalContainerError[] = []; - const context = { - ...getMockContext({ - settings: { - "Fluid.Container.enableOfflineFull": true, - }, - }), - closeFn: (error?: ICriticalContainerError): void => { - if (error !== undefined) { - containerErrors.push(error); - } - }, + it("Can roundrip DuplicateBatchDetector state through summary/snapshot", async () => { + // Duplicate Batch Detection requires OfflineLoad enabled + const settings_enableOfflineLoad = { + "Fluid.ContainerRuntime.enableBatchIdTracking": true, }; - await assert.rejects( - ContainerRuntime.loadRuntime2({ - context: context as IContainerContext, - registry: new FluidDataStoreRegistry([]), - existing: false, - runtimeOptions: { flushMode: FlushMode.Immediate }, - provideEntryPoint: mockProvideEntryPoint, - }), - (error: Error) => error instanceof UsageError, - ); - assert.strictEqual(containerErrors.length, 1); - }); - - it("Can roundtrip DuplicateBatchDetector state through summary/snapshot", async () => { - // Duplicate Batch Detection is on by default in TurnBased mode. const { runtime: containerRuntime } = await ContainerRuntime.loadRuntime2({ - context: getMockContext() as IContainerContext, + context: getMockContext({ + settings: settings_enableOfflineLoad, + }) as IContainerContext, registry: new FluidDataStoreRegistry([]), existing: false, runtimeOptions: { @@ -2509,6 +2432,7 @@ describe("Runtime", () => { }; const { runtime: containerRuntime2 } = await ContainerRuntime.loadRuntime2({ context: getMockContext({ + settings: settings_enableOfflineLoad, baseSnapshot: { trees: {}, blobs: { [recentBatchInfoBlobName]: "nonempty_id_ignored_by_mockStorage" }, diff --git a/packages/runtime/container-runtime/src/test/opLifecycle/duplicateBatchDetector.spec.ts b/packages/runtime/container-runtime/src/test/opLifecycle/duplicateBatchDetector.spec.ts index f0ff435874ec..b415efbbdc94 100644 --- a/packages/runtime/container-runtime/src/test/opLifecycle/duplicateBatchDetector.spec.ts +++ b/packages/runtime/container-runtime/src/test/opLifecycle/duplicateBatchDetector.spec.ts @@ -217,11 +217,13 @@ describe("DuplicateBatchDetector", () => { detector.processInboundBatch(inboundBatch1); detector.processInboundBatch(inboundBatch2); - const telemetrySets = new Map(); + let setCalled = 0; const telemetryContext = { set: (key: string, subKey: string, value: unknown) => { + ++setCalled; assert.equal(key, "fluid_DuplicateBatchDetector_"); - telemetrySets.set(subKey, value); + assert.equal(subKey, "recentBatchCount"); + assert.equal(value, 2); }, } satisfies Partial as ITelemetryContext; @@ -235,57 +237,7 @@ describe("DuplicateBatchDetector", () => { ], "Incorrect recentBatchInfo", ); - assert.equal(telemetrySets.get("recentBatchCount"), 2); - assert.equal(telemetrySets.get("peakRecentBatchCount"), 2); - assert.equal(telemetrySets.get("processedBatchCount"), 2); - }); - - it("Per-window perf counters reset after each summary", () => { - detector.processInboundBatch( - makeBatch({ - sequenceNumber: seqNum++, // 1 - minimumSequenceNumber: 0, - batchId: "batch1", - }), - ); - detector.processInboundBatch( - makeBatch({ - sequenceNumber: seqNum++, // 2 - minimumSequenceNumber: 0, - batchId: "batch2", - }), - ); - - const firstWindow = new Map(); - detector.getRecentBatchInfoForSummary({ - set: (_key: string, subKey: string, value: unknown) => { - firstWindow.set(subKey, value); - }, - } satisfies Partial as ITelemetryContext); - assert.equal(firstWindow.get("processedBatchCount"), 2); - assert.equal(firstWindow.get("peakRecentBatchCount"), 2); - - // Process one more batch; MSN advances enough to drop both prior entries. - detector.processInboundBatch( - makeBatch({ - sequenceNumber: seqNum++, // 3 - minimumSequenceNumber: 3, - batchId: "batch3", - }), - ); - - const secondWindow = new Map(); - detector.getRecentBatchInfoForSummary({ - set: (_key: string, subKey: string, value: unknown) => { - secondWindow.set(subKey, value); - }, - } satisfies Partial as ITelemetryContext); - // Only one batch processed since the prior summary. - assert.equal(secondWindow.get("processedBatchCount"), 1); - // Peak in this window starts at the size carried over from the prior window (2) - // — peak only ever grows during a window. Current size after cleanup is 1. - assert.equal(secondWindow.get("peakRecentBatchCount"), 2); - assert.equal(secondWindow.get("recentBatchCount"), 1); + assert.equal(setCalled, 1, "Expected telemetryContext.set to be called once"); }); }); }); diff --git a/packages/test/functional-tests/src/test/containerRuntime.spec.ts b/packages/test/functional-tests/src/test/containerRuntime.spec.ts index 9dbea4720ebf..9dd1a9c36307 100644 --- a/packages/test/functional-tests/src/test/containerRuntime.spec.ts +++ b/packages/test/functional-tests/src/test/containerRuntime.spec.ts @@ -97,7 +97,6 @@ describe("Container Runtime", () => { for (let i = 0; i < count; i++) { const message: Partial = { clientId, - clientSequenceNumber: seq, minimumSequenceNumber: 0, sequenceNumber: seq++, type: MessageType.Operation, diff --git a/packages/test/snapshots/content b/packages/test/snapshots/content index a3451f34c6c6..b2399d55fd8e 160000 --- a/packages/test/snapshots/content +++ b/packages/test/snapshots/content @@ -1 +1 @@ -Subproject commit a3451f34c6c6158196fd65ec77bdfabe54e4cad7 +Subproject commit b2399d55fd8e4b846e641295dcb8af0516be9335 diff --git a/packages/test/snapshots/src/validateSnapshots.ts b/packages/test/snapshots/src/validateSnapshots.ts index 8126cd119c61..729149f9fdb8 100644 --- a/packages/test/snapshots/src/validateSnapshots.ts +++ b/packages/test/snapshots/src/validateSnapshots.ts @@ -3,15 +3,12 @@ * Licensed under the MIT License. */ -/* eslint-disable @typescript-eslint/no-unsafe-call */ -import { strict } from "assert"; import fs from "fs"; import { compareWithReferenceSnapshot, getNormalizedFileSnapshot, loadContainer, - normalizePackageVersions, uploadSummary, } from "@fluid-internal/replay-tool"; import { IContainer } from "@fluidframework/container-definitions/internal"; @@ -29,66 +26,6 @@ import { import { SnapshotStorageService } from "./snapshotStorageService.js"; const metadataBlobName = ".metadata"; -const recentBatchInfoBlobName = ".recentBatchInfo"; - -/** - * Returns true if the snapshot tree contains a top-level blob with the given path. - */ -function snapshotHasBlob(snapshot: IFileSnapshot, blobPath: string): boolean { - return snapshot.tree.entries.some( - (entry) => entry.path === blobPath && entry.type === TreeEntry.Blob, - ); -} - -/** - * Returns a shallow copy of the snapshot with the named top-level blob removed. - * - * Used so back-compat tests (loading older snapshots that pre-date a new summary blob and - * re-summarizing them) can still compare against the current reference, which does include - * that blob. The runtime cannot reconstruct the new state when loading an older snapshot, - * so the strict byte-equal comparison must ignore that blob in that direction. - */ -function withoutTopLevelBlob(snapshot: IFileSnapshot, blobPath: string): IFileSnapshot { - return { - commits: snapshot.commits, - tree: { - ...snapshot.tree, - entries: snapshot.tree.entries.filter((entry) => entry.path !== blobPath), - }, - }; -} - -/** - * Compare a produced snapshot against a reference file, ignoring a named top-level blob on - * both sides. Mirrors the relevant parts of {@link compareWithReferenceSnapshot} but applies - * an additional filter so we can validate back-compat scenarios where the source snapshot - * pre-dates a new state-carrying blob. - */ -function compareIgnoringBlob( - produced: IFileSnapshot, - referenceSnapshotFilename: string, - blobToIgnore: string, - errorHandler: (description: string, error?: unknown) => void, -): void { - const referenceSnapshotString = fs.readFileSync( - `${referenceSnapshotFilename}.json`, - "utf-8", - ); - const referenceSnapshot = JSON.parse(referenceSnapshotString) as IFileSnapshot; - - const filteredProduced = normalizePackageVersions( - withoutTopLevelBlob(produced, blobToIgnore), - ); - const filteredReference = normalizePackageVersions( - withoutTopLevelBlob(getNormalizedFileSnapshot(referenceSnapshot), blobToIgnore), - ); - - try { - strict.deepStrictEqual(filteredProduced, filteredReference); - } catch (error) { - errorHandler(`Mismatch in snapshot ${referenceSnapshotFilename}.json`, error); - } -} /** * Validates snapshots in the source directory with corresponding snapshots in the destination directory: @@ -140,39 +77,26 @@ export async function validateSnapshots( const snapshotFileName = file.name.split(".")[0]; const sourceDir = `${srcDir}/${file.name}`; const srcContent = fs.readFileSync(sourceDir, "utf-8"); - const sourceSnapshot = JSON.parse(srcContent) as IFileSnapshot; - // Old-format snapshots predate the .recentBatchInfo blob (added when batchId tracking - // became enabled by default). Loading one and re-summarizing produces a summary without - // that blob, but the reference (regenerated by the current runtime) has it. Strip the - // blob from the comparison in that case so we still validate the rest of the summary. - const sourceLacksRecentBatchInfo = !snapshotHasBlob( - sourceSnapshot, - recentBatchInfoBlobName, - ); 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. - const onSnapshotCb = (snapshot: IFileSnapshot): void => { - const produced = getNormalizedFileSnapshot( - addSummaryMessage(snapshot, seqToMessage, container.deltaManager.lastSequenceNumber), + const onSnapshotCb = (snapshot: IFileSnapshot): void => + compareWithReferenceSnapshot( + getNormalizedFileSnapshot( + addSummaryMessage( + snapshot, + seqToMessage, + container.deltaManager.lastSequenceNumber, + ), + ), + `${destDir}/${snapshotFileName}`, + reportError, ); - if (sourceLacksRecentBatchInfo) { - compareIgnoringBlob( - produced, - `${destDir}/${snapshotFileName}`, - recentBatchInfoBlobName, - reportError, - ); - } else { - compareWithReferenceSnapshot( - produced, - `${destDir}/${snapshotFileName}`, - reportError, - ); - } - }; - const storage = new SnapshotStorageService(sourceSnapshot, onSnapshotCb); + const storage = new SnapshotStorageService( + JSON.parse(srcContent) as IFileSnapshot, + onSnapshotCb, + ); const container: IContainer = await loadContainer( new StaticStorageDocumentServiceFactory(storage), diff --git a/packages/test/test-end-to-end-tests/src/test/batching.spec.ts b/packages/test/test-end-to-end-tests/src/test/batching.spec.ts index b13f444590c6..8e066720a2fe 100644 --- a/packages/test/test-end-to-end-tests/src/test/batching.spec.ts +++ b/packages/test/test-end-to-end-tests/src/test/batching.spec.ts @@ -105,7 +105,7 @@ describeCompat("Flushing ops", "NoCompat", (getTestObjectProvider, apis) => { registry, loaderProps: { configProvider: configProvider({ - "Fluid.Container.enableOfflineFull": true, + "Fluid.ContainerRuntime.enableBatchIdTracking": true, }), }, }; diff --git a/packages/test/test-end-to-end-tests/src/test/fewerBatches.spec.ts b/packages/test/test-end-to-end-tests/src/test/fewerBatches.spec.ts index 059c75ccd24a..31ba5a860f37 100644 --- a/packages/test/test-end-to-end-tests/src/test/fewerBatches.spec.ts +++ b/packages/test/test-end-to-end-tests/src/test/fewerBatches.spec.ts @@ -164,13 +164,7 @@ describeCompat("Fewer batches", "NoCompat", (getTestObjectProvider, apis) => { "Op reentry submits two batches due to flush before processing", expectedErrors, async () => { - // The fake sequenced op injected below reuses a sequence number that the - // DuplicateBatchDetector has already seen, which would trip its invariants and - // short-circuit the flow before the deltaManager's non-Sequential check fires. - // Disable batchId tracking here so the test can exercise its actual scenario. - await processOutOfOrderOp({ - "Fluid.ContainerRuntime.DisableBatchIdTracking": true, - }); + await processOutOfOrderOp({}); assert.strictEqual(capturedBatches.length, 2); }, ); diff --git a/packages/tools/replay-tool/src/helpers.ts b/packages/tools/replay-tool/src/helpers.ts index d4063d2b8b12..ca7e296dc12d 100644 --- a/packages/tools/replay-tool/src/helpers.ts +++ b/packages/tools/replay-tool/src/helpers.ts @@ -68,35 +68,6 @@ export function getNormalizedFileSnapshot(snapshot: IFileSnapshot): IFileSnapsho return normalizedSnapshot; } -/** - * Replaces all `packageVersion` values inside JSON-encoded blob contents with a stable - * placeholder, so snapshots produced by different runtime versions can be compared without - * failing solely on the embedded version string. - * - * @example - * - * Before replace: - * - * ``` - * "{\"type\":\"https://graph.microsoft.com/types/map\",\"packageVersion\":\"0.28.0-214\"}" - * ``` - * - * After replace: - * - * ``` - * "{\"type\":\"https://graph.microsoft.com/types/map\",\"packageVersion\":\"X\"}" - * ``` - * - * @internal - */ -export function normalizePackageVersions(snapshot: IFileSnapshot): IFileSnapshot { - const packageVersionRegex = /\\"packageversion\\":\\"[^"]+\\"/gi; - const packageVersionPlaceholder = '\\"packageVersion\\":\\"X\\"'; - return JSON.parse( - stringify(snapshot, { space: 2 }).replace(packageVersionRegex, packageVersionPlaceholder), - ) as IFileSnapshot; -} - /** * Compares a snapshot against a reference snapshot file and reports any differences. * @@ -114,9 +85,39 @@ export function compareWithReferenceSnapshot( ); const referenceSnapshot = JSON.parse(referenceSnapshotString); - const normalizedSnapshot = normalizePackageVersions(getNormalizedFileSnapshot(snapshot)); - const normalizedReferenceSnapshot = normalizePackageVersions( - getNormalizedFileSnapshot(referenceSnapshot), + /** + * The packageVersion of the snapshot could be different from the reference snapshot. Replace all package + * package versions with X before we compare them. + * + * @example + * + * This is how it will look: + * Before replace: + * + * ``` + * "{\"type\":\"https://graph.microsoft.com/types/map\",\"packageVersion\":\"0.28.0-214\"}" + * ``` + * + * After replace: + * + * ``` + * "{\"type\":\"https://graph.microsoft.com/types/map\",\"packageVersion\":\"X\"}" + * ``` + */ + const packageVersionRegex = /\\"packageversion\\":\\"[^"]+\\"/gi; + const packageVersionPlaceholder = '\\"packageVersion\\":\\"X\\"'; + + const normalizedSnapshot = JSON.parse( + stringify(getNormalizedFileSnapshot(snapshot), { space: 2 }).replace( + packageVersionRegex, + packageVersionPlaceholder, + ), + ); + const normalizedReferenceSnapshot = JSON.parse( + stringify(getNormalizedFileSnapshot(referenceSnapshot), { space: 2 }).replace( + packageVersionRegex, + packageVersionPlaceholder, + ), ); // Put the assert in a try catch block, so that we can report errors, if any. diff --git a/packages/tools/replay-tool/src/index.ts b/packages/tools/replay-tool/src/index.ts index 7692a9cbd76a..8a339f80bf75 100644 --- a/packages/tools/replay-tool/src/index.ts +++ b/packages/tools/replay-tool/src/index.ts @@ -7,7 +7,6 @@ export { compareWithReferenceSnapshot, getNormalizedFileSnapshot, loadContainer, - normalizePackageVersions, uploadSummary, } from "./helpers.js"; export { ReplayArgs } from "./replayArgs.js";