From d8974dda0a9e5442eafa5567996b6ec140c7f3bb Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 29 May 2024 17:59:53 +0100 Subject: [PATCH] Fix the queueToDevice tests for the new fakeindexeddb https://github.com/dumbmatter/fakeIndexedDB/pull/93 causes a bunch of tests to start failing because the fake timers need running in order for fake indexeddb to work. It also seems to cause failures to bleed between tests somehow if fake timers are enabled/disabled. This keeps all the fake timer tests in one suite and all the others in another, which appears to work. This should allow https://github.com/matrix-org/matrix-js-sdk/pull/4224 to be merged. --- spec/unit/queueToDevice.spec.ts | 428 ++++++++++++++++++-------------- 1 file changed, 235 insertions(+), 193 deletions(-) diff --git a/spec/unit/queueToDevice.spec.ts b/spec/unit/queueToDevice.spec.ts index f9b90304e0..362d410af1 100644 --- a/spec/unit/queueToDevice.spec.ts +++ b/spec/unit/queueToDevice.spec.ts @@ -61,251 +61,293 @@ describe.each([[StoreType.Memory], [StoreType.IndexedDB]])("queueToDevice (%s st let httpBackend: MockHttpBackend; let client: MatrixClient; - beforeEach(async function () { - jest.runOnlyPendingTimers(); - jest.useRealTimers(); - httpBackend = new MockHttpBackend(); - - let store: IStore; - if (storeType === StoreType.IndexedDB) { - const idbStore = new IndexedDBStore({ indexedDB: fakeIndexedDB }); - await idbStore.startup(); - store = idbStore; - } else { - store = new MemoryStore(); - } - - client = new MatrixClient({ - baseUrl: "https://my.home.server", - accessToken: "my.access.token", - fetchFn: httpBackend.fetchFn as typeof global.fetch, - store, + /** + * We need to split the tests into regular ones (these) and ones that use fake timers, + * because the fake indexeddb uses timers too and appears make tests cause other tests + * to fail if we keep enabling/disabling fake timers within the same test suite. + */ + describe("non-timed tests", () => { + beforeEach(async function () { + httpBackend = new MockHttpBackend(); + + let store: IStore; + if (storeType === StoreType.IndexedDB) { + const idbStore = new IndexedDBStore({ indexedDB: fakeIndexedDB }); + await idbStore.startup(); + store = idbStore; + } else { + store = new MemoryStore(); + } + + client = new MatrixClient({ + baseUrl: "https://my.home.server", + accessToken: "my.access.token", + fetchFn: httpBackend.fetchFn as typeof global.fetch, + store, + }); }); - }); - - afterEach(function () { - jest.useRealTimers(); - client.stopClient(); - }); - it("sends a to-device message", async function () { - httpBackend - .when("PUT", "/sendToDevice/org.example.foo/") - .check((request) => { - expect(request.data).toEqual(EXPECTED_BODY); - }) - .respond(200, {}); - - await client.queueToDevice({ - eventType: "org.example.foo", - batch: [FAKE_MSG], + afterEach(function () { + client.stopClient(); }); - await httpBackend.flushAllExpected(); - // let the code handle the response to the request so we don't get - // log output after the test has finished (apparently stopping the - // client in aftereach is not sufficient.) - await flushPromises(); - }); - - it("retries on error", async function () { - jest.useFakeTimers(); - - httpBackend.when("PUT", "/sendToDevice/org.example.foo/").respond(500); - - httpBackend - .when("PUT", "/sendToDevice/org.example.foo/") - .check((request) => { - expect(request.data).toEqual(EXPECTED_BODY); - }) - .respond(200, {}); + it("sends a to-device message", async function () { + httpBackend + .when("PUT", "/sendToDevice/org.example.foo/") + .check((request) => { + expect(request.data).toEqual(EXPECTED_BODY); + }) + .respond(200, {}); + + await client.queueToDevice({ + eventType: "org.example.foo", + batch: [FAKE_MSG], + }); - await client.queueToDevice({ - eventType: "org.example.foo", - batch: [FAKE_MSG], + await httpBackend.flushAllExpected(); + // let the code handle the response to the request so we don't get + // log output after the test has finished (apparently stopping the + // client in aftereach is not sufficient.) + await flushPromises(); }); - await flushAndRunTimersUntil(() => httpBackend.requests.length > 0); - expect(httpBackend.flushSync(undefined, 1)).toEqual(1); - await flushAndRunTimersUntil(() => httpBackend.requests.length > 0); + it("retries on retryImmediately()", async function () { + httpBackend.when("GET", "/_matrix/client/versions").respond(200, { + versions: ["v1.1"], + }); - expect(httpBackend.flushSync(undefined, 1)).toEqual(1); + await Promise.all([client.startClient(), httpBackend.flush(undefined, 1, 20)]); - // flush, as per comment in first test - await flushPromises(); - }); + httpBackend.when("PUT", "/sendToDevice/org.example.foo/").respond(500); + + httpBackend.when("PUT", "/sendToDevice/org.example.foo/").respond(200, {}); - it("stops retrying on 4xx errors", async function () { - jest.useFakeTimers(); + await client.queueToDevice({ + eventType: "org.example.foo", + batch: [FAKE_MSG], + }); + expect(await httpBackend.flush(undefined, 1, 1)).toEqual(1); + await flushPromises(); - httpBackend.when("PUT", "/sendToDevice/org.example.foo/").respond(400); + client.retryImmediately(); - await client.queueToDevice({ - eventType: "org.example.foo", - batch: [FAKE_MSG], + // longer timeout here to try & avoid flakiness + expect(await httpBackend.flush(undefined, 1, 3000)).toEqual(1); }); - await flushAndRunTimersUntil(() => httpBackend.requests.length > 0); - expect(httpBackend.flushSync(undefined, 1)).toEqual(1); - // Asserting that another request is never made is obviously - // a bit tricky - we just flush the queue what should hopefully - // be plenty of times and assert that nothing comes through. - let tries = 0; - await flushAndRunTimersUntil(() => ++tries === 10); + it("retries on when client is started", async function () { + httpBackend.when("GET", "/_matrix/client/versions").respond(200, { + versions: ["v1.1"], + }); - expect(httpBackend.requests.length).toEqual(0); - }); + await Promise.all([client.startClient(), httpBackend.flush("/_matrix/client/versions", 1, 20)]); - it("honours ratelimiting", async function () { - jest.useFakeTimers(); + httpBackend.when("PUT", "/sendToDevice/org.example.foo/").respond(500); - // pick something obscure enough it's unlikley to clash with a - // retry delay the algorithm uses anyway - const retryDelay = 279 * 1000; + httpBackend.when("PUT", "/sendToDevice/org.example.foo/").respond(200, {}); - httpBackend.when("PUT", "/sendToDevice/org.example.foo/").respond(429, { - errcode: "M_LIMIT_EXCEEDED", - retry_after_ms: retryDelay, - }); + await client.queueToDevice({ + eventType: "org.example.foo", + batch: [FAKE_MSG], + }); + expect(await httpBackend.flush(undefined, 1, 1)).toEqual(1); + await flushPromises(); - httpBackend.when("PUT", "/sendToDevice/org.example.foo/").respond(200, {}); + client.stopClient(); + await Promise.all([client.startClient(), httpBackend.flush("/_matrix/client/versions", 1, 20)]); - await client.queueToDevice({ - eventType: "org.example.foo", - batch: [FAKE_MSG], + expect(await httpBackend.flush(undefined, 1, 20)).toEqual(1); }); - await flushAndRunTimersUntil(() => httpBackend.requests.length > 0); - expect(httpBackend.flushSync(undefined, 1)).toEqual(1); - await flushPromises(); - logger.info("Advancing clock to just before expected retry time..."); + it("retries when a message is retried", async function () { + httpBackend.when("GET", "/_matrix/client/versions").respond(200, { + versions: ["v1.1"], + }); - jest.advanceTimersByTime(retryDelay - 1000); - await flushPromises(); + await Promise.all([client.startClient(), httpBackend.flush(undefined, 1, 20)]); - expect(httpBackend.requests.length).toEqual(0); + httpBackend.when("PUT", "/sendToDevice/org.example.foo/").respond(500); - logger.info("Advancing clock past expected retry time..."); + httpBackend.when("PUT", "/sendToDevice/org.example.foo/").respond(200, {}); - jest.advanceTimersByTime(2000); - await flushPromises(); + await client.queueToDevice({ + eventType: "org.example.foo", + batch: [FAKE_MSG], + }); - expect(httpBackend.flushSync(undefined, 1)).toEqual(1); - }); + expect(await httpBackend.flush(undefined, 1, 20)).toEqual(1); + await flushPromises(); + + const dummyEvent = new MatrixEvent({ + event_id: "!fake:example.org", + }); + const mockRoom = { + updatePendingEvent: jest.fn(), + hasEncryptionStateEvent: jest.fn().mockReturnValue(false), + } as unknown as Room; + client.resendEvent(dummyEvent, mockRoom); - it("retries on retryImmediately()", async function () { - httpBackend.when("GET", "/_matrix/client/versions").respond(200, { - versions: ["v1.1"], + expect(await httpBackend.flush(undefined, 1, 20)).toEqual(1); }); - await Promise.all([client.startClient(), httpBackend.flush(undefined, 1, 20)]); + it("splits many messages into multiple HTTP requests", async function () { + const batch: ToDeviceBatch = { + eventType: "org.example.foo", + batch: [], + }; + + for (let i = 0; i <= 20; ++i) { + batch.batch.push({ + userId: `@user${i}:example.org`, + deviceId: FAKE_DEVICE_ID, + payload: FAKE_PAYLOAD, + }); + } + + const expectedCounts = [20, 1]; + httpBackend + .when("PUT", "/sendToDevice/org.example.foo/") + .check((request) => { + expect( + removeElement(expectedCounts, (c) => c === Object.keys(request.data.messages).length), + ).toBeTruthy(); + }) + .respond(200, {}); + httpBackend + .when("PUT", "/sendToDevice/org.example.foo/") + .check((request) => { + expect(Object.keys(request.data.messages).length).toEqual(1); + }) + .respond(200, {}); + + await client.queueToDevice(batch); + await httpBackend.flushAllExpected(); + + // flush, as per comment in first test + await flushPromises(); + }); + }); - httpBackend.when("PUT", "/sendToDevice/org.example.foo/").respond(500); + describe("async tests", () => { + beforeAll(() => { + jest.useFakeTimers(); + }); - httpBackend.when("PUT", "/sendToDevice/org.example.foo/").respond(200, {}); + afterAll(() => { + jest.useRealTimers(); + }); - await client.queueToDevice({ - eventType: "org.example.foo", - batch: [FAKE_MSG], + beforeEach(async function () { + httpBackend = new MockHttpBackend(); + + let store: IStore; + if (storeType === StoreType.IndexedDB) { + const idbStore = new IndexedDBStore({ indexedDB: fakeIndexedDB }); + let storeStarted = false; + idbStore.startup().then(() => { + storeStarted = true; + }); + await flushAndRunTimersUntil(() => storeStarted); + store = idbStore; + } else { + store = new MemoryStore(); + } + + client = new MatrixClient({ + baseUrl: "https://my.home.server", + accessToken: "my.access.token", + fetchFn: httpBackend.fetchFn as typeof global.fetch, + store, + }); }); - expect(await httpBackend.flush(undefined, 1, 1)).toEqual(1); - await flushPromises(); - client.retryImmediately(); + afterEach(function () { + client.stopClient(); + }); - // longer timeout here to try & avoid flakiness - expect(await httpBackend.flush(undefined, 1, 3000)).toEqual(1); - }); + it("retries on error", async function () { + httpBackend.when("PUT", "/sendToDevice/org.example.foo/").respond(500); - it("retries on when client is started", async function () { - httpBackend.when("GET", "/_matrix/client/versions").respond(200, { - versions: ["v1.1"], - }); + httpBackend + .when("PUT", "/sendToDevice/org.example.foo/") + .check((request) => { + expect(request.data).toEqual(EXPECTED_BODY); + }) + .respond(200, {}); - await Promise.all([client.startClient(), httpBackend.flush("/_matrix/client/versions", 1, 20)]); + client + .queueToDevice({ + eventType: "org.example.foo", + batch: [FAKE_MSG], + }) + .then(); + await flushAndRunTimersUntil(() => httpBackend.requests.length > 0); + expect(httpBackend.flushSync(undefined, 1)).toEqual(1); - httpBackend.when("PUT", "/sendToDevice/org.example.foo/").respond(500); + await flushAndRunTimersUntil(() => httpBackend.requests.length > 0); - httpBackend.when("PUT", "/sendToDevice/org.example.foo/").respond(200, {}); + expect(httpBackend.flushSync(undefined, 1)).toEqual(1); - await client.queueToDevice({ - eventType: "org.example.foo", - batch: [FAKE_MSG], + // flush, as per comment in first test + await flushPromises(); }); - expect(await httpBackend.flush(undefined, 1, 1)).toEqual(1); - await flushPromises(); - client.stopClient(); - await Promise.all([client.startClient(), httpBackend.flush("/_matrix/client/versions", 1, 20)]); + it("stops retrying on 4xx errors", async function () { + httpBackend.when("PUT", "/sendToDevice/org.example.foo/").respond(400); + + client + .queueToDevice({ + eventType: "org.example.foo", + batch: [FAKE_MSG], + }) + .then(); + await flushAndRunTimersUntil(() => httpBackend.requests.length > 0); + expect(httpBackend.flushSync(undefined, 1)).toEqual(1); + + // Asserting that another request is never made is obviously + // a bit tricky - we just flush the queue what should hopefully + // be plenty of times and assert that nothing comes through. + let tries = 0; + await flushAndRunTimersUntil(() => ++tries === 10); + + expect(httpBackend.requests.length).toEqual(0); + }); - expect(await httpBackend.flush(undefined, 1, 20)).toEqual(1); - }); + it("honours ratelimiting", async function () { + // pick something obscure enough it's unlikley to clash with a + // retry delay the algorithm uses anyway + const retryDelay = 279 * 1000; - it("retries when a message is retried", async function () { - httpBackend.when("GET", "/_matrix/client/versions").respond(200, { - versions: ["v1.1"], - }); + httpBackend.when("PUT", "/sendToDevice/org.example.foo/").respond(429, { + errcode: "M_LIMIT_EXCEEDED", + retry_after_ms: retryDelay, + }); - await Promise.all([client.startClient(), httpBackend.flush(undefined, 1, 20)]); + httpBackend.when("PUT", "/sendToDevice/org.example.foo/").respond(200, {}); - httpBackend.when("PUT", "/sendToDevice/org.example.foo/").respond(500); + client + .queueToDevice({ + eventType: "org.example.foo", + batch: [FAKE_MSG], + }) + .then(); + await flushAndRunTimersUntil(() => httpBackend.requests.length > 0); + expect(httpBackend.flushSync(undefined, 1)).toEqual(1); + await flushPromises(); - httpBackend.when("PUT", "/sendToDevice/org.example.foo/").respond(200, {}); + logger.info("Advancing clock to just before expected retry time..."); - await client.queueToDevice({ - eventType: "org.example.foo", - batch: [FAKE_MSG], - }); + jest.advanceTimersByTime(retryDelay - 1000); + await flushPromises(); - expect(await httpBackend.flush(undefined, 1, 20)).toEqual(1); - await flushPromises(); + expect(httpBackend.requests.length).toEqual(0); - const dummyEvent = new MatrixEvent({ - event_id: "!fake:example.org", - }); - const mockRoom = { - updatePendingEvent: jest.fn(), - hasEncryptionStateEvent: jest.fn().mockReturnValue(false), - } as unknown as Room; - client.resendEvent(dummyEvent, mockRoom); + logger.info("Advancing clock past expected retry time..."); - expect(await httpBackend.flush(undefined, 1, 20)).toEqual(1); - }); + jest.advanceTimersByTime(2000); + await flushPromises(); - it("splits many messages into multiple HTTP requests", async function () { - const batch: ToDeviceBatch = { - eventType: "org.example.foo", - batch: [], - }; - - for (let i = 0; i <= 20; ++i) { - batch.batch.push({ - userId: `@user${i}:example.org`, - deviceId: FAKE_DEVICE_ID, - payload: FAKE_PAYLOAD, - }); - } - - const expectedCounts = [20, 1]; - httpBackend - .when("PUT", "/sendToDevice/org.example.foo/") - .check((request) => { - expect( - removeElement(expectedCounts, (c) => c === Object.keys(request.data.messages).length), - ).toBeTruthy(); - }) - .respond(200, {}); - httpBackend - .when("PUT", "/sendToDevice/org.example.foo/") - .check((request) => { - expect(Object.keys(request.data.messages).length).toEqual(1); - }) - .respond(200, {}); - - await client.queueToDevice(batch); - await httpBackend.flushAllExpected(); - - // flush, as per comment in first test - await flushPromises(); + expect(httpBackend.flushSync(undefined, 1)).toEqual(1); + }); }); });