Skip to content

Commit

Permalink
Fix flaky jest tests (#12486)
Browse files Browse the repository at this point in the history
...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.
  • Loading branch information
dbkr authored May 2, 2024
1 parent 7193d4c commit 374cee9
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 27 deletions.
4 changes: 3 additions & 1 deletion test/components/structures/ThreadView-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
8 changes: 5 additions & 3 deletions test/components/views/beacon/BeaconMarker-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -111,13 +111,15 @@ describe("<BeaconMarker />", () => {
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", () => {
Expand Down
8 changes: 5 additions & 3 deletions test/components/views/beacon/BeaconViewDialog-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -92,7 +92,7 @@ describe("<BeaconViewDialog />", () => {
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]);
Expand All @@ -103,7 +103,9 @@ describe("<BeaconViewDialog />", () => {
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", () => {
Expand Down
4 changes: 2 additions & 2 deletions test/components/views/dialogs/ForwardDialog-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
9 changes: 7 additions & 2 deletions test/components/views/messages/MLocationBody-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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();
});

Expand Down
20 changes: 12 additions & 8 deletions test/components/views/rooms/MessageComposerButtons-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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(
<MessageComposerButtons
{...mockProps}
Expand All @@ -94,12 +94,16 @@ describe("MessageComposerButtons", () => {
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", () => {
Expand Down
3 changes: 2 additions & 1 deletion test/components/views/settings/Notifications-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -907,6 +907,7 @@ describe("<Notifications />", () => {
fireEvent.click(clearNotificationEl);

expect(clearNotificationEl.className).toContain("mx_AccessibleButton_disabled");
await waitFor(() => expect(clearNotificationEl.className).not.toContain("mx_AccessibleButton_disabled"));
expect(mockClient.sendReadReceipt).toHaveBeenCalled();
});
});
Expand Down
7 changes: 0 additions & 7 deletions test/setupTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 374cee9

Please sign in to comment.