From 07e4568c935cd7e7e654a73a1332955759f38146 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 2 May 2024 15:24:58 +0100 Subject: [PATCH] Fix flaky jest tests ...and remove the code that causes them to be retried in CI. Most of these were just lack of waiting for async things to happen, mostly lazy loading components, hence whythey worked on the retry: because the code had been loaded by then. --- .../components/structures/ThreadView-test.tsx | 4 +++- .../views/beacon/BeaconMarker-test.tsx | 8 +++++--- .../views/beacon/BeaconViewDialog-test.tsx | 8 +++++--- .../views/dialogs/ForwardDialog-test.tsx | 4 ++-- .../views/messages/MLocationBody-test.tsx | 9 +++++++-- .../rooms/MessageComposerButtons-test.tsx | 20 +++++++++++-------- .../views/settings/Notifications-test.tsx | 3 ++- test/setupTests.ts | 7 ------- 8 files changed, 36 insertions(+), 27 deletions(-) diff --git a/test/components/structures/ThreadView-test.tsx b/test/components/structures/ThreadView-test.tsx index 83eed5eb9d3..d7cbfa17565 100644 --- a/test/components/structures/ThreadView-test.tsx +++ b/test/components/structures/ThreadView-test.tsx @@ -197,7 +197,9 @@ describe("ThreadView", () => { it("sets the correct thread in the room view store", async () => { // expect(SdkContextClass.instance.roomViewStore.getThreadId()).toBeNull(); const { unmount } = await getComponent(); - expect(SdkContextClass.instance.roomViewStore.getThreadId()).toBe(rootEvent.getId()); + waitFor(() => { + expect(SdkContextClass.instance.roomViewStore.getThreadId()).toBe(rootEvent.getId()); + }); unmount(); await waitFor(() => expect(SdkContextClass.instance.roomViewStore.getThreadId()).toBeNull()); diff --git a/test/components/views/beacon/BeaconMarker-test.tsx b/test/components/views/beacon/BeaconMarker-test.tsx index 15b1335d90e..4a5b5b87111 100644 --- a/test/components/views/beacon/BeaconMarker-test.tsx +++ b/test/components/views/beacon/BeaconMarker-test.tsx @@ -15,7 +15,7 @@ limitations under the License. */ import React from "react"; -import { act, render, screen } from "@testing-library/react"; +import { act, render, screen, waitFor } from "@testing-library/react"; import * as maplibregl from "maplibre-gl"; import { Beacon, Room, RoomMember, MatrixEvent, getBeaconInfoIdentifier } from "matrix-js-sdk/src/matrix"; @@ -111,13 +111,15 @@ describe("", () => { expect(screen.queryByTestId("avatar-img")).not.toBeInTheDocument(); }); - it("renders marker when beacon has location", () => { + it("renders marker when beacon has location", async () => { const room = setupRoom([defaultEvent]); const beacon = room.currentState.beacons.get(getBeaconInfoIdentifier(defaultEvent)); beacon?.addLocations([location1]); const { asFragment } = renderComponent({ beacon }); + await waitFor(() => { + expect(screen.getByTestId("avatar-img")).toBeInTheDocument(); + }); expect(asFragment()).toMatchSnapshot(); - expect(screen.getByTestId("avatar-img")).toBeInTheDocument(); }); it("updates with new locations", () => { diff --git a/test/components/views/beacon/BeaconViewDialog-test.tsx b/test/components/views/beacon/BeaconViewDialog-test.tsx index cee880c966e..8347fca18b9 100644 --- a/test/components/views/beacon/BeaconViewDialog-test.tsx +++ b/test/components/views/beacon/BeaconViewDialog-test.tsx @@ -15,7 +15,7 @@ limitations under the License. */ import React from "react"; -import { act, fireEvent, render, RenderResult } from "@testing-library/react"; +import { act, fireEvent, render, RenderResult, waitFor } from "@testing-library/react"; import { MatrixClient, MatrixEvent, Room, RoomMember, getBeaconInfoIdentifier } from "matrix-js-sdk/src/matrix"; import * as maplibregl from "maplibre-gl"; import { mocked } from "jest-mock"; @@ -92,7 +92,7 @@ describe("", () => { jest.clearAllMocks(); }); - it("renders a map with markers", () => { + it("renders a map with markers", async () => { const room = setupRoom([defaultEvent]); const beacon = room.currentState.beacons.get(getBeaconInfoIdentifier(defaultEvent))!; beacon.addLocations([location1]); @@ -103,7 +103,9 @@ describe("", () => { lat: 51, }); // marker added - expect(mockMarker.addTo).toHaveBeenCalledWith(mockMap); + await waitFor(() => { + expect(mockMarker.addTo).toHaveBeenCalledWith(mockMap); + }); }); it("does not render any own beacon status when user is not live sharing", () => { diff --git a/test/components/views/dialogs/ForwardDialog-test.tsx b/test/components/views/dialogs/ForwardDialog-test.tsx index 6206087ab85..12c6048e618 100644 --- a/test/components/views/dialogs/ForwardDialog-test.tsx +++ b/test/components/views/dialogs/ForwardDialog-test.tsx @@ -130,9 +130,9 @@ describe("ForwardDialog", () => { expect(container.querySelectorAll(".mx_ForwardList_entry")).toHaveLength(3); const searchInput = getByTestId(container, "searchbox-input"); - act(() => userEvent.type(searchInput, "a")); + await userEvent.type(searchInput, "a"); - expect(container.querySelectorAll(".mx_ForwardList_entry")).toHaveLength(3); + expect(container.querySelectorAll(".mx_ForwardList_entry")).toHaveLength(2); }); it("should be navigable using arrow keys", async () => { diff --git a/test/components/views/messages/MLocationBody-test.tsx b/test/components/views/messages/MLocationBody-test.tsx index 7f3bddaa4c5..1cdcb657676 100644 --- a/test/components/views/messages/MLocationBody-test.tsx +++ b/test/components/views/messages/MLocationBody-test.tsx @@ -15,7 +15,7 @@ limitations under the License. */ import React, { ComponentProps } from "react"; -import { fireEvent, render } from "@testing-library/react"; +import { fireEvent, render, waitFor } from "@testing-library/react"; import { LocationAssetType, ClientEvent, RoomMember, SyncState } from "matrix-js-sdk/src/matrix"; import * as maplibregl from "maplibre-gl"; import { logger } from "matrix-js-sdk/src/logger"; @@ -90,8 +90,13 @@ describe("MLocationBody", () => { jest.spyOn(logger, "error").mockRestore(); }); - it("displays correct fallback content without error style when map_style_url is not configured", () => { + it("displays correct fallback content without error style when map_style_url is not configured", async () => { const component = getComponent(); + + // The map code needs to be lazy loaded so this will take some time to appear + await waitFor(() => + expect(component.container.querySelector(".mx_EventTile_body")).toBeInTheDocument(), + ); expect(component.container.querySelector(".mx_EventTile_body")).toMatchSnapshot(); }); diff --git a/test/components/views/rooms/MessageComposerButtons-test.tsx b/test/components/views/rooms/MessageComposerButtons-test.tsx index 2ddd8d9b016..21abb2b9c73 100644 --- a/test/components/views/rooms/MessageComposerButtons-test.tsx +++ b/test/components/views/rooms/MessageComposerButtons-test.tsx @@ -15,7 +15,7 @@ limitations under the License. */ import React from "react"; -import { render, screen } from "@testing-library/react"; +import { render, screen, waitFor } from "@testing-library/react"; import MatrixClientContext from "../../../../src/contexts/MatrixClientContext"; import RoomContext from "../../../../src/contexts/RoomContext"; @@ -82,7 +82,7 @@ describe("MessageComposerButtons", () => { expect(getButtonLabels()).toEqual(["Emoji", "Attachment", "More options"]); }); - it("Renders other buttons in menu in wide mode", () => { + it("Renders other buttons in menu in wide mode", async () => { wrapAndRender( { false, ); - expect(getButtonLabels()).toEqual([ - "Emoji", - "Attachment", - "More options", - ["Sticker", "Voice Message", "Poll", "Location"], - ]); + // The location code is lazy loaded, so the button will take a little while + // to appear, so we need to wait. + await waitFor(() => { + expect(getButtonLabels()).toEqual([ + "Emoji", + "Attachment", + "More options", + ["Sticker", "Voice Message", "Poll", "Location"], + ]); + }); }); it("Renders only some buttons in narrow mode", () => { diff --git a/test/components/views/settings/Notifications-test.tsx b/test/components/views/settings/Notifications-test.tsx index b12cf9239ea..24a23832c1d 100644 --- a/test/components/views/settings/Notifications-test.tsx +++ b/test/components/views/settings/Notifications-test.tsx @@ -30,7 +30,7 @@ import { ThreepidMedium, } from "matrix-js-sdk/src/matrix"; import { randomString } from "matrix-js-sdk/src/randomstring"; -import { act, fireEvent, getByTestId, render, screen, within } from "@testing-library/react"; +import { act, fireEvent, getByTestId, render, screen, waitFor, within } from "@testing-library/react"; import { mocked } from "jest-mock"; import userEvent from "@testing-library/user-event"; @@ -907,6 +907,7 @@ describe("", () => { fireEvent.click(clearNotificationEl); expect(clearNotificationEl.className).toContain("mx_AccessibleButton_disabled"); + await waitFor(() => expect(clearNotificationEl.className).not.toContain("mx_AccessibleButton_disabled")); expect(mockClient.sendReadReceipt).toHaveBeenCalled(); }); }); diff --git a/test/setupTests.ts b/test/setupTests.ts index fa99913b564..a7044c04744 100644 --- a/test/setupTests.ts +++ b/test/setupTests.ts @@ -37,13 +37,6 @@ beforeEach(() => { }); }); -// Retry to work around our flaky app & tests -if (process.env.CI) { - jest.retryTimes(2, { - logErrorsBeforeRetry: true, - }); -} - // Very carefully enable the mocks for everything else in // a specific order. We use this order to ensure we properly // establish an application state that actually works.