Fix jest/no-conditional-expect lint and enable it (#3194)

.eslintrc.js

@@ -69,9 +69,6 @@ module.exports = {
         // TODO: There are many tests with invalid expects that should be fixed,
         // https://github.com/matrix-org/matrix-js-sdk/issues/2976
         "jest/valid-expect": "off",
-        // TODO: There are many cases to refactor away,
-        // https://github.com/matrix-org/matrix-js-sdk/issues/2978
-        "jest/no-conditional-expect": "off",
         // Also treat "oldBackendOnly" as a test function.
         // Used in some crypto tests.
         "jest/no-standalone-expect": [

spec/integ/matrix-client-opts.spec.ts

@@ -157,14 +157,10 @@ describe("MatrixClient opts", function () {
                     error: "Ruh roh",
                 }),
             );
-            try {
-                await Promise.all([
-                    expect(client.sendTextMessage("!foo:bar", "a body", "txn1")).rejects.toThrow(),
-                    httpBackend.flush("/txn1", 1),
-                ]);
-            } catch (err) {
-                expect((<MatrixError>err).errcode).toEqual("M_SOMETHING");
-            }
+
+            await expect(
+                Promise.all([client.sendTextMessage("!foo:bar", "a body", "txn1"), httpBackend.flush("/txn1", 1)]),
+            ).rejects.toThrow("MatrixError: [500] Unknown message");
         });
 
         it("shouldn't queue events", async () => {

spec/integ/sliding-sync-sdk.spec.ts

@@ -891,11 +891,9 @@ describe("SlidingSyncSdk", () => {
                 const evType = ev.getType();
                 expect(seen[evType]).toBeFalsy();
                 seen[evType] = true;
-                if (evType === "m.key.verification.start" || evType === "m.key.verification.request") {
-                    expect(ev.isCancelled()).toEqual(true);
-                } else {
-                    expect(ev.isCancelled()).toEqual(false);
-                }
+                expect(ev.isCancelled()).toEqual(
+                    evType === "m.key.verification.start" || evType === "m.key.verification.request",
+                );
             });
             ext.onResponse({
                 next_batch: "45678",

spec/unit/NamespacedValue.spec.ts

@@ -46,13 +46,7 @@ describe("NamespacedValue", () => {
     });
 
     it("should not permit falsey values for both parts", () => {
-        try {
-            new UnstableValue(null!, null!);
-            // noinspection ExceptionCaughtLocallyJS
-            throw new Error("Failed to fail");
-        } catch (e) {
-            expect((<Error>e).message).toBe("One of stable or unstable values must be supplied");
-        }
+        expect(() => new UnstableValue(null!, null!)).toThrow("One of stable or unstable values must be supplied");
     });
 });
 
@@ -72,12 +66,6 @@ describe("UnstableValue", () => {
     });
 
     it("should not permit falsey unstable values", () => {
-        try {
-            new UnstableValue("stable", null!);
-            // noinspection ExceptionCaughtLocallyJS
-            throw new Error("Failed to fail");
-        } catch (e) {
-            expect((<Error>e).message).toBe("Unstable value must be supplied");
-        }
+        expect(() => new UnstableValue("stable", null!)).toThrow("Unstable value must be supplied");
     });
 });

spec/unit/crypto/CrossSigningInfo.spec.ts

@@ -102,9 +102,10 @@ describe("CrossSigningInfo.getCrossSigningKey", function () {
             const info = new CrossSigningInfo(userId, { getCrossSigningKey }, { getCrossSigningKeyCache });
             const [pubKey] = await info.getCrossSigningKey(type, masterKeyPub);
             expect(pubKey).toEqual(masterKeyPub);
-            expect(getCrossSigningKeyCache.mock.calls.length).toBe(shouldCache ? 1 : 0);
+            expect(getCrossSigningKeyCache).toHaveBeenCalledTimes(shouldCache ? 1 : 0);
             if (shouldCache) {
-                expect(getCrossSigningKeyCache.mock.calls[0][0]).toBe(type);
+                // eslint-disable-next-line jest/no-conditional-expect
+                expect(getCrossSigningKeyCache).toHaveBeenLastCalledWith(type, expect.any(String));
             }
         },
     );
@@ -115,10 +116,10 @@ describe("CrossSigningInfo.getCrossSigningKey", function () {
         const info = new CrossSigningInfo(userId, { getCrossSigningKey }, { storeCrossSigningKeyCache });
         const [pubKey] = await info.getCrossSigningKey(type, masterKeyPub);
         expect(pubKey).toEqual(masterKeyPub);
-        expect(storeCrossSigningKeyCache.mock.calls.length).toEqual(shouldCache ? 1 : 0);
+        expect(storeCrossSigningKeyCache).toHaveBeenCalledTimes(shouldCache ? 1 : 0);
         if (shouldCache) {
-            expect(storeCrossSigningKeyCache.mock.calls[0][0]).toBe(type);
-            expect(storeCrossSigningKeyCache.mock.calls[0][1]).toBe(testKey);
+            // eslint-disable-next-line jest/no-conditional-expect
+            expect(storeCrossSigningKeyCache).toHaveBeenLastCalledWith(type, testKey);
         }
     });
 

spec/unit/crypto/backup.spec.ts

@@ -456,6 +456,7 @@ describe("MegolmBackup", function () {
                     client.http.authedRequest = function (method, path, queryParams, data, opts): any {
                         ++numCalls;
                         expect(numCalls).toBeLessThanOrEqual(2);
+                        /* eslint-disable jest/no-conditional-expect */
                         if (numCalls === 1) {
                             expect(method).toBe("POST");
                             expect(path).toBe("/room_keys/version");
@@ -482,6 +483,7 @@ describe("MegolmBackup", function () {
                             reject(new Error("authedRequest called too many times"));
                             return Promise.resolve({});
                         }
+                        /* eslint-enable jest/no-conditional-expect */
                     };
                 }),
                 client.createKeyBackupVersion({

spec/unit/matrix-client.spec.ts

@@ -725,6 +725,7 @@ describe("MatrixClient", function () {
             getMyMembership: () => "join",
             currentState: {
                 getStateEvents: (eventType, stateKey) => {
+                    /* eslint-disable jest/no-conditional-expect */
                     if (eventType === EventType.RoomCreate) {
                         expect(stateKey).toEqual("");
                         return new MatrixEvent({
@@ -743,6 +744,7 @@ describe("MatrixClient", function () {
                     } else {
                         throw new Error("Unexpected event type or state key");
                     }
+                    /* eslint-enable jest/no-conditional-expect */
                 },
             } as Room["currentState"],
         } as unknown as Room;
@@ -785,6 +787,7 @@ describe("MatrixClient", function () {
             getMyMembership: () => "join",
             currentState: {
                 getStateEvents: (eventType, stateKey) => {
+                    /* eslint-disable jest/no-conditional-expect */
                     if (eventType === EventType.RoomCreate) {
                         expect(stateKey).toEqual("");
                         return new MatrixEvent({
@@ -803,6 +806,7 @@ describe("MatrixClient", function () {
                     } else {
                         throw new Error("Unexpected event type or state key");
                     }
+                    /* eslint-enable jest/no-conditional-expect */
                 },
             } as Room["currentState"],
         } as unknown as Room;
@@ -820,6 +824,7 @@ describe("MatrixClient", function () {
             getMyMembership: () => "join",
             currentState: {
                 getStateEvents: (eventType, stateKey) => {
+                    /* eslint-disable jest/no-conditional-expect */
                     if (eventType === EventType.RoomCreate) {
                         expect(stateKey).toEqual("");
                         return new MatrixEvent({
@@ -837,6 +842,7 @@ describe("MatrixClient", function () {
                     } else {
                         throw new Error("Unexpected event type or state key");
                     }
+                    /* eslint-enable jest/no-conditional-expect */
                 },
             } as Room["currentState"],
         } as unknown as Room;
@@ -858,6 +864,7 @@ describe("MatrixClient", function () {
         const syncPromise = new Promise<void>((resolve, reject) => {
             client.on(ClientEvent.Sync, function syncListener(state) {
                 if (state === "SYNCING") {
+                    // eslint-disable-next-line jest/no-conditional-expect
                     expect(httpLookups.length).toEqual(0);
                     client.removeListener(ClientEvent.Sync, syncListener);
                     resolve();
@@ -944,6 +951,7 @@ describe("MatrixClient", function () {
 
             const wasPreparedPromise = new Promise((resolve) => {
                 client.on(ClientEvent.Sync, function syncListener(state) {
+                    /* eslint-disable jest/no-conditional-expect */
                     if (state === "ERROR" && httpLookups.length > 0) {
                         expect(httpLookups.length).toEqual(2);
                         expect(client.retryImmediately()).toBe(true);
@@ -955,6 +963,7 @@ describe("MatrixClient", function () {
                         // unexpected state transition!
                         expect(state).toEqual(null);
                     }
+                    /* eslint-enable jest/no-conditional-expect */
                 });
             });
             await client.startClient();
@@ -976,8 +985,10 @@ describe("MatrixClient", function () {
             const isSyncingPromise = new Promise((resolve) => {
                 client.on(ClientEvent.Sync, function syncListener(state) {
                     if (state === "ERROR" && httpLookups.length > 0) {
+                        /* eslint-disable jest/no-conditional-expect */
                         expect(httpLookups.length).toEqual(1);
                         expect(client.retryImmediately()).toBe(true);
+                        /* eslint-enable jest/no-conditional-expect */
                         jest.advanceTimersByTime(1);
                     } else if (state === "RECONNECTING" && httpLookups.length > 0) {
                         jest.advanceTimersByTime(10000);
@@ -1004,6 +1015,7 @@ describe("MatrixClient", function () {
 
             const wasPreparedPromise = new Promise((resolve) => {
                 client.on(ClientEvent.Sync, function syncListener(state) {
+                    /* eslint-disable jest/no-conditional-expect */
                     if (state === "ERROR" && httpLookups.length > 0) {
                         expect(httpLookups.length).toEqual(3);
                         expect(client.retryImmediately()).toBe(true);
@@ -1015,6 +1027,7 @@ describe("MatrixClient", function () {
                         // unexpected state transition!
                         expect(state).toEqual(null);
                     }
+                    /* eslint-enable jest/no-conditional-expect */
                 });
             });
             await client.startClient();

spec/unit/models/MSC3089Branch.spec.ts

@@ -265,11 +265,13 @@ describe("MSC3089Branch", () => {
                 expect(eventType).toEqual(UNSTABLE_MSC3089_BRANCH.unstable); // test that we're definitely using the unstable value
                 expect(stateKey).toEqual(stateKeyOrder[stateFn.mock.calls.length - 1]);
                 if (stateKey === fileEventId) {
+                    // eslint-disable-next-line jest/no-conditional-expect
                     expect(content).toMatchObject({
                         retained: true, // canary for copying state
                         active: false,
                     });
                 } else if (stateKey === fileEventId2) {
+                    // eslint-disable-next-line jest/no-conditional-expect
                     expect(content).toMatchObject({
                         active: true,
                         version: 2,

spec/unit/models/MSC3089TreeSpace.spec.ts

@@ -130,14 +130,8 @@ describe("MSC3089TreeSpace", () => {
             return Promise.reject(new MatrixError({ errcode: "M_FORBIDDEN", error: "Sample Failure" }));
         });
         client.invite = fn;
-        try {
-            await tree.invite(target, false, false);
 
-            // noinspection ExceptionCaughtLocallyJS
-            throw new Error("Failed to fail");
-        } catch (e) {
-            expect((<MatrixError>e).errcode).toEqual("M_FORBIDDEN");
-        }
+        await expect(tree.invite(target, false, false)).rejects.toThrow("MatrixError: Sample Failure");
 
         expect(fn).toHaveBeenCalledTimes(1);
     });
@@ -357,13 +351,18 @@ describe("MSC3089TreeSpace", () => {
             .fn()
             .mockImplementation(async (roomId: string, eventType: EventType, content: any, stateKey: string) => {
                 expect([tree.roomId, subspaceId]).toContain(roomId);
+
+                let expectedType: string;
+                let expectedStateKey: string;
                 if (roomId === subspaceId) {
-                    expect(eventType).toEqual(EventType.SpaceParent);
-                    expect(stateKey).toEqual(tree.roomId);
+                    expectedType = EventType.SpaceParent;
+                    expectedStateKey = tree.roomId;
                 } else {
-                    expect(eventType).toEqual(EventType.SpaceChild);
-                    expect(stateKey).toEqual(subspaceId);
+                    expectedType = EventType.SpaceChild;
+                    expectedStateKey = subspaceId;
                 }
+                expect(eventType).toEqual(expectedType);
+                expect(stateKey).toEqual(expectedStateKey);
                 expect(content).toMatchObject({ via: [domain] });
 
                 // return value not used
@@ -629,15 +628,8 @@ describe("MSC3089TreeSpace", () => {
         });
 
         it("should throw when setting an order at the top level space", async () => {
-            try {
-                // The tree is what we've defined as top level, so it should work
-                await tree.setOrder(2);
-
-                // noinspection ExceptionCaughtLocallyJS
-                throw new Error("Failed to fail");
-            } catch (e) {
-                expect((<Error>e).message).toEqual("Cannot set order of top level spaces currently");
-            }
+            // The tree is what we've defined as top level, so it should work
+            await expect(tree.setOrder(2)).rejects.toThrow("Cannot set order of top level spaces currently");
         });
 
         it("should return a stable order for unordered children", () => {

spec/unit/pushprocessor.spec.ts

@@ -572,11 +572,7 @@ describe("NotificationService", function () {
             });
 
             const actions = pushProcessor.actionsForEvent(testEvent);
-            if (expected) {
-                expect(actions?.notify).toBeTruthy();
-            } else {
-                expect(actions?.notify).toBeFalsy();
-            }
+            expect(!!actions?.notify).toBe(expected);
         });
     });
 

spec/unit/room.spec.ts

@@ -390,36 +390,30 @@ describe("Room", function () {
             remoteEvent.event.unsigned = { transaction_id: "TXN_ID" };
             const remoteEventId = remoteEvent.getId();
 
-            let callCount = 0;
-            room.on(RoomEvent.LocalEchoUpdated, (event, emitRoom, oldEventId, oldStatus) => {
-                switch (callCount) {
-                    case 0:
-                        expect(event.getId()).toEqual(localEventId);
-                        expect(event.status).toEqual(EventStatus.SENDING);
-                        expect(emitRoom).toEqual(room);
-                        expect(oldEventId).toBeUndefined();
-                        expect(oldStatus).toBeUndefined();
-                        break;
-                    case 1:
-                        expect(event.getId()).toEqual(remoteEventId);
-                        expect(event.status).toBeNull();
-                        expect(emitRoom).toEqual(room);
-                        expect(oldEventId).toEqual(localEventId);
-                        expect(oldStatus).toBe(EventStatus.SENDING);
-                        break;
-                }
-                callCount += 1;
-            });
+            const stub = jest.fn();
+            room.on(RoomEvent.LocalEchoUpdated, stub);
 
             // first add the local echo
             room.addPendingEvent(localEvent, "TXN_ID");
             expect(room.timeline.length).toEqual(1);
 
+            expect(stub.mock.calls[0][0].getId()).toEqual(localEventId);
+            expect(stub.mock.calls[0][0].status).toEqual(EventStatus.SENDING);
+            expect(stub.mock.calls[0][1]).toEqual(room);
+            expect(stub.mock.calls[0][2]).toBeUndefined();
+            expect(stub.mock.calls[0][3]).toBeUndefined();
+
             // then the remoteEvent
             room.addLiveEvents([remoteEvent]);
             expect(room.timeline.length).toEqual(1);
 
-            expect(callCount).toEqual(2);
+            expect(stub).toHaveBeenCalledTimes(2);
+
+            expect(stub.mock.calls[1][0].getId()).toEqual(remoteEventId);
+            expect(stub.mock.calls[1][0].status).toBeNull();
+            expect(stub.mock.calls[1][1]).toEqual(room);
+            expect(stub.mock.calls[1][2]).toEqual(localEventId);
+            expect(stub.mock.calls[1][3]).toBe(EventStatus.SENDING);
         });
 
         it("should be able to update local echo without a txn ID (/send then /sync)", function () {

spec/unit/scheduler.spec.ts

@@ -58,10 +58,12 @@ describe("MatrixScheduler", function () {
         let yieldedA = false;
         scheduler.setProcessFunction(function (event) {
             if (yieldedA) {
+                // eslint-disable-next-line jest/no-conditional-expect
                 expect(event).toEqual(eventB);
                 return deferB.promise;
             } else {
                 yieldedA = true;
+                // eslint-disable-next-line jest/no-conditional-expect
                 expect(event).toEqual(eventA);
                 return deferA.promise;
             }
@@ -89,6 +91,7 @@ describe("MatrixScheduler", function () {
         scheduler.setProcessFunction(function (ev) {
             procCount += 1;
             if (procCount === 1) {
+                // eslint-disable-next-line jest/no-conditional-expect
                 expect(ev).toEqual(eventA);
                 return deferred.promise;
             } else if (procCount === 2) {
@@ -129,9 +132,11 @@ describe("MatrixScheduler", function () {
         scheduler.setProcessFunction(function (ev) {
             procCount += 1;
             if (procCount === 1) {
+                // eslint-disable-next-line jest/no-conditional-expect
                 expect(ev).toEqual(eventA);
                 return deferA.promise;
             } else if (procCount === 2) {
+                // eslint-disable-next-line jest/no-conditional-expect
                 expect(ev).toEqual(eventB);
                 return deferB.promise;
             }