Add matrix-org/jest linting (#2973)

Author: MadLittleMods

.eslintrc.js

@@ -1,6 +1,6 @@
 module.exports = {
     plugins: ["matrix-org", "import", "jsdoc"],
-    extends: ["plugin:matrix-org/babel", "plugin:import/typescript"],
+    extends: ["plugin:matrix-org/babel", "plugin:matrix-org/jest", "plugin:import/typescript"],
     parserOptions: {
         project: ["./tsconfig.json"],
     },
@@ -63,6 +63,23 @@ module.exports = {
                 ],
             },
         ],
+        // Disabled tests are a reality for now but as soon as all of the xits are
+        // eliminated, we should enforce this.
+        "jest/no-disabled-tests": "off",
+        // 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": [
+            "error",
+            {
+                additionalTestBlockFunctions: ["beforeAll", "beforeEach", "oldBackendOnly"],
+            },
+        ],
     },
     overrides: [
         {

package.json

@@ -103,8 +103,9 @@
         "eslint-config-prettier": "^8.5.0",
         "eslint-import-resolver-typescript": "^3.5.1",
         "eslint-plugin-import": "^2.26.0",
+        "eslint-plugin-jest": "^27.1.6",
         "eslint-plugin-jsdoc": "^39.6.4",
-        "eslint-plugin-matrix-org": "^0.10.0",
+        "eslint-plugin-matrix-org": "^1.0.0",
         "eslint-plugin-tsdoc": "^0.2.17",
         "eslint-plugin-unicorn": "^45.0.0",
         "exorcist": "^2.0.0",

spec/TestClient.ts

@@ -16,6 +16,9 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+// `expect` is allowed in helper functions which are called within `test`/`it` blocks
+/* eslint-disable jest/no-standalone-expect */
+
 // load olm before the sdk if possible
 import "./olm-loader";
 

spec/integ/crypto.spec.ts

@@ -743,10 +743,8 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("megolm (%s)", (backend: string,
     describe("get|setGlobalErrorOnUnknownDevices", () => {
         it("should raise an error if crypto is disabled", () => {
             aliceTestClient.client["cryptoBackend"] = undefined;
-            expect(() => aliceTestClient.client.setGlobalErrorOnUnknownDevices(true)).toThrowError(
-                "encryption disabled",
-            );
-            expect(() => aliceTestClient.client.getGlobalErrorOnUnknownDevices()).toThrowError("encryption disabled");
+            expect(() => aliceTestClient.client.setGlobalErrorOnUnknownDevices(true)).toThrow("encryption disabled");
+            expect(() => aliceTestClient.client.getGlobalErrorOnUnknownDevices()).toThrow("encryption disabled");
         });
 
         oldBackendOnly("should permit sending to unknown devices", async () => {
@@ -799,12 +797,10 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("megolm (%s)", (backend: string,
     describe("get|setGlobalBlacklistUnverifiedDevices", () => {
         it("should raise an error if crypto is disabled", () => {
             aliceTestClient.client["cryptoBackend"] = undefined;
-            expect(() => aliceTestClient.client.setGlobalBlacklistUnverifiedDevices(true)).toThrowError(
-                "encryption disabled",
-            );
-            expect(() => aliceTestClient.client.getGlobalBlacklistUnverifiedDevices()).toThrowError(
+            expect(() => aliceTestClient.client.setGlobalBlacklistUnverifiedDevices(true)).toThrow(
                 "encryption disabled",
             );
+            expect(() => aliceTestClient.client.getGlobalBlacklistUnverifiedDevices()).toThrow("encryption disabled");
         });
 
         oldBackendOnly("should disable sending to unverified devices", async () => {

spec/integ/matrix-client-event-emitter.spec.ts

@@ -175,7 +175,7 @@ describe("MatrixClient events", function () {
             });
         });
 
-        it("should emit User events", function (done) {
+        it("should emit User events", async () => {
             httpBackend!.when("GET", "/sync").respond(200, SYNC_DATA);
             httpBackend!.when("GET", "/sync").respond(200, NEXT_SYNC_DATA);
             let fired = false;
@@ -192,10 +192,8 @@ describe("MatrixClient events", function () {
             });
             client!.startClient();
 
-            httpBackend!.flushAllExpected().then(function () {
-                expect(fired).toBe(true);
-                done();
-            });
+            await httpBackend!.flushAllExpected();
+            expect(fired).toBe(true);
         });
 
         it("should emit Room events", function () {

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

@@ -205,19 +205,17 @@ describe("MatrixClient", function () {
     describe("getFilter", function () {
         const filterId = "f1lt3r1d";
 
-        it("should return a filter from the store if allowCached", function (done) {
+        it("should return a filter from the store if allowCached", async () => {
             const filter = Filter.fromJson(userId, filterId, {
                 event_format: "client",
             });
             store!.storeFilter(filter);
-            client!.getFilter(userId, filterId, true).then(function (gotFilter) {
-                expect(gotFilter).toEqual(filter);
-                done();
-            });
+            const gotFilter = await client!.getFilter(userId, filterId, true);
+            expect(gotFilter).toEqual(filter);
             httpBackend!.verifyNoOutstandingRequests();
         });
 
-        it("should do an HTTP request if !allowCached even if one exists", function (done) {
+        it("should do an HTTP request if !allowCached even if one exists", async () => {
             const httpFilterDefinition = {
                 event_format: "federation",
             };
@@ -230,15 +228,11 @@ describe("MatrixClient", function () {
                 event_format: "client",
             });
             store!.storeFilter(storeFilter);
-            client!.getFilter(userId, filterId, false).then(function (gotFilter) {
-                expect(gotFilter.getDefinition()).toEqual(httpFilterDefinition);
-                done();
-            });
-
-            httpBackend!.flush("");
+            const [gotFilter] = await Promise.all([client!.getFilter(userId, filterId, false), httpBackend!.flush("")]);
+            expect(gotFilter.getDefinition()).toEqual(httpFilterDefinition);
         });
 
-        it("should do an HTTP request if nothing is in the cache and then store it", function (done) {
+        it("should do an HTTP request if nothing is in the cache and then store it", async () => {
             const httpFilterDefinition = {
                 event_format: "federation",
             };
@@ -247,20 +241,16 @@ describe("MatrixClient", function () {
             httpBackend!
                 .when("GET", "/user/" + encodeURIComponent(userId) + "/filter/" + filterId)
                 .respond(200, httpFilterDefinition);
-            client!.getFilter(userId, filterId, true).then(function (gotFilter) {
-                expect(gotFilter.getDefinition()).toEqual(httpFilterDefinition);
-                expect(store!.getFilter(userId, filterId)).toBeTruthy();
-                done();
-            });
-
-            httpBackend!.flush("");
+            const [gotFilter] = await Promise.all([client!.getFilter(userId, filterId, true), httpBackend!.flush("")]);
+            expect(gotFilter.getDefinition()).toEqual(httpFilterDefinition);
+            expect(store!.getFilter(userId, filterId)).toBeTruthy();
         });
     });
 
     describe("createFilter", function () {
         const filterId = "f1llllllerid";
 
-        it("should do an HTTP request and then store the filter", function (done) {
+        it("should do an HTTP request and then store the filter", async () => {
             expect(store!.getFilter(userId, filterId)).toBe(null);
 
             const filterDefinition = {
@@ -276,13 +266,9 @@ describe("MatrixClient", function () {
                     filter_id: filterId,
                 });
 
-            client!.createFilter(filterDefinition).then(function (gotFilter) {
-                expect(gotFilter.getDefinition()).toEqual(filterDefinition);
-                expect(store!.getFilter(userId, filterId)).toEqual(gotFilter);
-                done();
-            });
-
-            httpBackend!.flush("");
+            const [gotFilter] = await Promise.all([client!.createFilter(filterDefinition), httpBackend!.flush("")]);
+            expect(gotFilter.getDefinition()).toEqual(filterDefinition);
+            expect(store!.getFilter(userId, filterId)).toEqual(gotFilter);
         });
     });
 

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

@@ -94,16 +94,16 @@ describe("MatrixClient opts", function () {
             client.stopClient();
         });
 
-        it("should be able to send messages", function (done) {
+        it("should be able to send messages", async () => {
             const eventId = "$flibble:wibble";
             httpBackend.when("PUT", "/txn1").respond(200, {
                 event_id: eventId,
             });
-            client.sendTextMessage("!foo:bar", "a body", "txn1").then(function (res) {
-                expect(res.event_id).toEqual(eventId);
-                done();
-            });
-            httpBackend.flush("/txn1", 1);
+            const [res] = await Promise.all([
+                client.sendTextMessage("!foo:bar", "a body", "txn1"),
+                httpBackend.flush("/txn1", 1),
+            ]);
+            expect(res.event_id).toEqual(eventId);
         });
 
         it("should be able to sync / get new events", async function () {
@@ -149,27 +149,25 @@ describe("MatrixClient opts", function () {
             client.stopClient();
         });
 
-        it("shouldn't retry sending events", function (done) {
+        it("shouldn't retry sending events", async () => {
             httpBackend.when("PUT", "/txn1").respond(
                 500,
                 new MatrixError({
                     errcode: "M_SOMETHING",
                     error: "Ruh roh",
                 }),
             );
-            client.sendTextMessage("!foo:bar", "a body", "txn1").then(
-                function (res) {
-                    expect(false).toBe(true);
-                },
-                function (err) {
-                    expect(err.errcode).toEqual("M_SOMETHING");
-                    done();
-                },
-            );
-            httpBackend.flush("/txn1", 1);
+            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");
+            }
         });
 
-        it("shouldn't queue events", function (done) {
+        it("shouldn't queue events", async () => {
             httpBackend.when("PUT", "/txn1").respond(200, {
                 event_id: "AAA",
             });
@@ -178,30 +176,38 @@ describe("MatrixClient opts", function () {
             });
             let sentA = false;
             let sentB = false;
-            client.sendTextMessage("!foo:bar", "a body", "txn1").then(function (res) {
+            const messageASendPromise = client.sendTextMessage("!foo:bar", "a body", "txn1").then(function (res) {
                 sentA = true;
+                // We expect messageB to be sent before messageA to ensure as we're
+                // testing that there is no queueing that blocks each other
                 expect(sentB).toBe(true);
             });
-            client.sendTextMessage("!foo:bar", "b body", "txn2").then(function (res) {
+            const messageBSendPromise = client.sendTextMessage("!foo:bar", "b body", "txn2").then(function (res) {
                 sentB = true;
+                // We expect messageB to be sent before messageA to ensure as we're
+                // testing that there is no queueing that blocks each other
                 expect(sentA).toBe(false);
             });
-            httpBackend.flush("/txn2", 1).then(function () {
-                httpBackend.flush("/txn1", 1).then(function () {
-                    done();
-                });
-            });
+            // Allow messageB to succeed first
+            await httpBackend.flush("/txn2", 1);
+            // Then allow messageA to succeed
+            await httpBackend.flush("/txn1", 1);
+
+            // Now await the message send promises to
+            await messageBSendPromise;
+            await messageASendPromise;
         });
 
-        it("should be able to send messages", function (done) {
+        it("should be able to send messages", async () => {
             httpBackend.when("PUT", "/txn1").respond(200, {
                 event_id: "foo",
             });
-            client.sendTextMessage("!foo:bar", "a body", "txn1").then(function (res) {
-                expect(res.event_id).toEqual("foo");
-                done();
-            });
-            httpBackend.flush("/txn1", 1);
+            const [res] = await Promise.all([
+                client.sendTextMessage("!foo:bar", "a body", "txn1"),
+                httpBackend.flush("/txn1", 1),
+            ]);
+
+            expect(res.event_id).toEqual("foo");
         });
     });
 });

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

@@ -48,13 +48,13 @@ describe("MatrixClient retrying", function () {
         return httpBackend!.stop();
     });
 
-    xit("should retry according to MatrixScheduler.retryFn", function () {});
+    it.skip("should retry according to MatrixScheduler.retryFn", function () {});
 
-    xit("should queue according to MatrixScheduler.queueFn", function () {});
+    it.skip("should queue according to MatrixScheduler.queueFn", function () {});
 
-    xit("should mark events as EventStatus.NOT_SENT when giving up", function () {});
+    it.skip("should mark events as EventStatus.NOT_SENT when giving up", function () {});
 
-    xit("should mark events as EventStatus.QUEUED when queued", function () {});
+    it.skip("should mark events as EventStatus.QUEUED when queued", function () {});
 
     it("should mark events as EventStatus.CANCELLED when cancelled", function () {
         // send a couple of events; the second will be queued
@@ -130,7 +130,7 @@ describe("MatrixClient retrying", function () {
     });
 
     describe("resending", function () {
-        xit("should be able to resend a NOT_SENT event", function () {});
-        xit("should be able to resend a sent event", function () {});
+        it.skip("should be able to resend a NOT_SENT event", function () {});
+        it.skip("should be able to resend a sent event", function () {});
     });
 });