From cdd391b3a27a5086d299d7dd397f38f9d31c074f Mon Sep 17 00:00:00 2001 From: Valere Date: Fri, 28 Nov 2025 09:12:05 +0100 Subject: [PATCH 1/2] test: Add simple tests for scope.reconcile --- src/state/ObservableScope.test.ts | 92 ++++++++++++++++++++++++++++++- 1 file changed, 91 insertions(+), 1 deletion(-) diff --git a/src/state/ObservableScope.test.ts b/src/state/ObservableScope.test.ts index 99f2b424..a0c71268 100644 --- a/src/state/ObservableScope.test.ts +++ b/src/state/ObservableScope.test.ts @@ -5,9 +5,10 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial Please see LICENSE in the repository root for full details. */ -import { describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { BehaviorSubject, combineLatest, Subject } from "rxjs"; import { logger } from "matrix-js-sdk/lib/logger"; +import { sleep } from "matrix-js-sdk/lib/utils"; import { Epoch, @@ -102,3 +103,92 @@ describe("Epoch", () => { s$.complete(); }); }); + +describe("Reconcile", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("should wait clean up before processing next", async () => { + vi.useFakeTimers(); + const scope = new ObservableScope(); + const behavior$ = new BehaviorSubject(0); + + const setup = vi.fn().mockImplementation(async () => await sleep(100)); + const cleanup = vi + .fn() + .mockImplementation(async (n: number) => await sleep(100)); + scope.reconcile(behavior$, async (value) => { + await setup(); + return async (): Promise => { + await cleanup(value); + }; + }); + // Let the initial setup process + await vi.advanceTimersByTimeAsync(120); + expect(setup).toHaveBeenCalledTimes(1); + expect(cleanup).toHaveBeenCalledTimes(0); + + // Send next value + behavior$.next(1); + await vi.advanceTimersByTimeAsync(50); + // Should not have started setup for 1 yet + expect(setup).toHaveBeenCalledTimes(1); + expect(cleanup).toHaveBeenCalledTimes(1); + expect(cleanup).toHaveBeenCalledWith(0); + + // Let cleanup finish + await vi.advanceTimersByTimeAsync(50); + // Now setup for 1 should have started + expect(setup).toHaveBeenCalledTimes(2); + }); + + it("should skip intermediates values that are not setup", async () => { + vi.useFakeTimers(); + const scope = new ObservableScope(); + const behavior$ = new BehaviorSubject(0); + + const setup = vi + .fn() + .mockImplementation(async (n: number) => await sleep(100)); + + const cleanupLock = Promise.withResolvers(); + const cleanup = vi + .fn() + .mockImplementation(async (n: number) => await cleanupLock.promise); + + scope.reconcile(behavior$, async (value) => { + await setup(value); + return async (): Promise => { + await cleanup(value); + }; + }); + // Let the initial setup process (0) + await vi.advanceTimersByTimeAsync(120); + + // Send 4 next values quickly + behavior$.next(1); + behavior$.next(2); + behavior$.next(3); + behavior$.next(4); + + await vi.advanceTimersByTimeAsync(3000); + // should have only called cleanup for 0 + expect(cleanup).toHaveBeenCalledTimes(1); + expect(cleanup).toHaveBeenCalledWith(0); + // Let cleanup finish + cleanupLock.resolve(undefined); + await vi.advanceTimersByTimeAsync(120); + + // Now setup for 4 should have started, skipping 1,2,3 + expect(setup).toHaveBeenCalledTimes(2); + expect(setup).toHaveBeenCalledWith(4); + expect(setup).not.toHaveBeenCalledWith(1); + expect(setup).not.toHaveBeenCalledWith(2); + expect(setup).not.toHaveBeenCalledWith(3); + }); +}); From c6bfda94cff76c900cc812e54bf8025362c97091 Mon Sep 17 00:00:00 2001 From: Valere Date: Fri, 28 Nov 2025 09:53:18 +0100 Subject: [PATCH 2/2] test: additional reconcile test --- src/state/ObservableScope.test.ts | 45 +++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/src/state/ObservableScope.test.ts b/src/state/ObservableScope.test.ts index a0c71268..31728f39 100644 --- a/src/state/ObservableScope.test.ts +++ b/src/state/ObservableScope.test.ts @@ -191,4 +191,49 @@ describe("Reconcile", () => { expect(setup).not.toHaveBeenCalledWith(2); expect(setup).not.toHaveBeenCalledWith(3); }); + + it("should wait for setup to complete before starting cleanup", async () => { + vi.useFakeTimers(); + const scope = new ObservableScope(); + const behavior$ = new BehaviorSubject(0); + + const setup = vi + .fn() + .mockImplementation(async (n: number) => await sleep(3000)); + + const cleanupLock = Promise.withResolvers(); + const cleanup = vi + .fn() + .mockImplementation(async (n: number) => await cleanupLock.promise); + + scope.reconcile(behavior$, async (value) => { + await setup(value); + return async (): Promise => { + await cleanup(value); + }; + }); + + await vi.advanceTimersByTimeAsync(500); + // Setup for 0 should be in progress + expect(setup).toHaveBeenCalledTimes(1); + + behavior$.next(1); + await vi.advanceTimersByTimeAsync(500); + + // Should not have started setup for 1 yet + expect(setup).not.toHaveBeenCalledWith(1); + // Should not have called cleanup yet, because the setup for 0 is not done + expect(cleanup).toHaveBeenCalledTimes(0); + + // Let setup for 0 finish + await vi.advanceTimersByTimeAsync(2500 + 100); + // Now cleanup for 0 should have started + expect(cleanup).toHaveBeenCalledTimes(1); + expect(cleanup).toHaveBeenCalledWith(0); + + cleanupLock.resolve(undefined); + await vi.advanceTimersByTimeAsync(100); + // Now setup for 1 should have started + expect(setup).toHaveBeenCalledWith(1); + }); });