Skip to content

Commit 57fd3c4

Browse files
authored
Deflake MatrixChat-test (#31383)
Add a workaround for the fact that MatrixChat attempts to use React state for the state of a state machine: a small `sleep` to let the state settle. As a result, it turns out we may not see the "Syncing..." state, and in general `waitForSyncAndLoad` doesn't seem to be doing anything useful.
1 parent 6228edc commit 57fd3c4

1 file changed

Lines changed: 34 additions & 56 deletions

File tree

test/unit-tests/components/structures/MatrixChat-test.tsx

Lines changed: 34 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,17 @@ describe("<MatrixChat />", () => {
9696
...mockClientMethodsUser(userId),
9797
...mockClientMethodsServer(),
9898
getVersions: jest.fn().mockResolvedValue({ versions: SERVER_SUPPORTED_MATRIX_VERSIONS }),
99-
startClient: function () {
99+
startClient: async function () {
100+
// This `sleep` is a horrible hack, for which I am sorry.
101+
//
102+
// MatrixChat uses its `view` state as the tracker for the current state of its state machine. However,
103+
// React state does not update immediately, with the result that if the client starts too quickly after the
104+
// "OnLoggedIn" action, the state machine will be confused and the view will not be correctly updated.
105+
//
106+
// In practice it takes a little time for the client to start up (it has to read a load of stuff from
107+
// indexedDB, so in some ways this is just a more realistic simulation of the real world 😇
108+
await sleep(1);
109+
100110
// @ts-ignore
101111
this.emit(ClientEvent.Sync, SyncState.Prepared, null);
102112
},
@@ -195,33 +205,6 @@ describe("<MatrixChat />", () => {
195205
localStorage.setItem("mx_device_id", deviceId);
196206
}
197207

198-
/**
199-
* Wait for a bunch of stuff to happen
200-
* between deciding we are logged in and removing the spinner
201-
* including waiting for initial sync
202-
*/
203-
const waitForSyncAndLoad = async (client: MatrixClient, withoutSecuritySetup?: boolean): Promise<void> => {
204-
// need to wait for different elements depending on which flow
205-
// without security setup we go to a loading page
206-
if (withoutSecuritySetup) {
207-
// wait for logged in view to load
208-
await screen.findByLabelText("User menu");
209-
210-
// otherwise we stay on login and load from there for longer
211-
} else {
212-
// we are logged in, but are still waiting for the /sync to complete
213-
await screen.findByText("Syncing…");
214-
// initial sync
215-
await act(() => client.emit(ClientEvent.Sync, SyncState.Prepared, null));
216-
}
217-
218-
// let things settle
219-
await flushPromises();
220-
// and some more for good measure
221-
// this proved to be a little flaky
222-
await flushPromises();
223-
};
224-
225208
beforeEach(async () => {
226209
await clearStorage();
227210
Lifecycle.setSessionLockNotStolen();
@@ -1281,16 +1264,14 @@ describe("<MatrixChat />", () => {
12811264
return renderResult;
12821265
};
12831266

1284-
const getComponentAndLogin = async (withoutSecuritySetup?: boolean): Promise<void> => {
1267+
const getComponentAndLogin = async (): Promise<void> => {
12851268
await getComponentAndWaitForReady();
12861269

12871270
fireEvent.change(screen.getByLabelText("Username"), { target: { value: userName } });
12881271
fireEvent.change(screen.getByLabelText("Password"), { target: { value: password } });
12891272

12901273
// sign in button is an input
12911274
fireEvent.click(screen.getByDisplayValue("Sign in"));
1292-
1293-
await waitForSyncAndLoad(loginClient, withoutSecuritySetup);
12941275
};
12951276

12961277
beforeEach(() => {
@@ -1337,12 +1318,15 @@ describe("<MatrixChat />", () => {
13371318
it("should go straight to logged in view when crypto is not enabled", async () => {
13381319
loginClient.getCrypto.mockReturnValue(undefined);
13391320

1340-
await getComponentAndLogin(true);
1321+
await getComponentAndLogin();
1322+
1323+
// wait for logged in view to load
1324+
await screen.findByLabelText("User menu");
13411325

13421326
expect(screen.getByRole("heading", { name: "Welcome Ernie" })).toBeInTheDocument();
13431327
});
13441328

1345-
describe("when server supports cross signing and user does not have cross signing setup", () => {
1329+
describe("when user does not have cross signing set up", () => {
13461330
beforeEach(() => {
13471331
jest.spyOn(loginClient.getCrypto()!, "userHasCrossSigningKeys").mockResolvedValue(false);
13481332
});
@@ -1370,53 +1354,48 @@ describe("<MatrixChat />", () => {
13701354

13711355
it("should go straight to logged in view when user is not in any encrypted rooms", async () => {
13721356
loginClient.getRooms.mockReturnValue([unencryptedRoom]);
1373-
await getComponentAndLogin(false);
1374-
1375-
await flushPromises();
1357+
await getComponentAndLogin();
13761358

1377-
// logged in, did not setup keys
1359+
// logged in, did not set up keys
13781360
await screen.findByLabelText("User menu");
13791361
});
13801362

1381-
it("should go to setup e2e screen when user is in encrypted rooms", async () => {
1363+
it("should go to set up e2e screen when user is in encrypted rooms", async () => {
13821364
loginClient.getRooms.mockReturnValue([unencryptedRoom, encryptedRoom]);
13831365
await getComponentAndLogin();
1384-
await flushPromises();
13851366
// set up keys screen is rendered
1386-
expect(screen.getByText("Setting up keys")).toBeInTheDocument();
1367+
await screen.findByText("Setting up keys");
13871368
});
13881369
});
13891370

1390-
it("should go to setup e2e screen", async () => {
1371+
it("should go to set up e2e screen", async () => {
13911372
await getComponentAndLogin();
13921373

1393-
expect(loginClient.getCrypto()!.userHasCrossSigningKeys).toHaveBeenCalled();
1394-
13951374
// set up keys screen is rendered
1396-
await expect(await screen.findByText("Setting up keys")).toBeInTheDocument();
1375+
await screen.findByText("Setting up keys");
1376+
1377+
expect(loginClient.getCrypto()!.userHasCrossSigningKeys).toHaveBeenCalled();
13971378
});
13981379
});
13991380

1400-
it("should show complete security screen when user has cross signing setup", async () => {
1381+
it("should show complete security screen when user has cross signing set up", async () => {
14011382
jest.spyOn(loginClient.getCrypto()!, "userHasCrossSigningKeys").mockResolvedValue(true);
14021383

14031384
await getComponentAndLogin();
14041385

1405-
expect(loginClient.getCrypto()!.userHasCrossSigningKeys).toHaveBeenCalled();
1406-
1407-
await flushPromises();
1408-
14091386
// Complete security begin screen is rendered
1410-
expect(screen.getByText("Confirm your identity")).toBeInTheDocument();
1387+
await screen.findByText("Confirm your identity");
1388+
1389+
expect(loginClient.getCrypto()!.userHasCrossSigningKeys).toHaveBeenCalled();
14111390
});
14121391

1413-
it("should setup e2e", async () => {
1392+
it("should set up e2e", async () => {
14141393
await getComponentAndLogin();
14151394

1416-
expect(loginClient.getCrypto()!.userHasCrossSigningKeys).toHaveBeenCalled();
1417-
14181395
// set up keys screen is rendered
1419-
expect(screen.getByText("Setting up keys")).toBeInTheDocument();
1396+
await screen.findByText("Setting up keys");
1397+
1398+
expect(loginClient.getCrypto()!.userHasCrossSigningKeys).toHaveBeenCalled();
14201399
});
14211400
});
14221401
});
@@ -1707,8 +1686,7 @@ describe("<MatrixChat />", () => {
17071686
jest.spyOn(MatrixJs, "createClient").mockReturnValue(client);
17081687

17091688
const rendered = getComponent({});
1710-
await waitForSyncAndLoad(client, true);
1711-
rendered.getByText("Welcome Ernie");
1689+
await rendered.findByText("Welcome Ernie");
17121690

17131691
// we're now at the welcome page. Another session wants the lock...
17141692
simulateSessionLockClaim();

0 commit comments

Comments
 (0)