chore: Test improvements (#10945)

* Lazy queues, correctly closing Redis and server

* feedback

* fix: Tests not correctly split across matrix

.github/workflows/ci.yml

@@ -133,7 +133,7 @@ jobs:
       - run: yarn sequelize db:migrate
       - name: Run server tests
         run: |
-          TESTFILES=$(find . -name "*.test.ts" -path "*/server/*" | sort | split -n -d -l $(($(find . -name "*.test.ts" -path "*/server/*" | wc -l)/${{ matrix.shard }})) - | sed -n "${{ matrix.shard }}p")
+          TESTFILES=$(find . -name "*.test.ts" -path "*/server/*" | sort | awk "NR % 4 == (${{ matrix.shard }} - 1)")
           yarn test --maxWorkers=2 $TESTFILES
 
   bundle-size:

.jestconfig.json

@@ -10,7 +10,10 @@
         "^@server/(.*)$": "<rootDir>/server/$1",
         "^@shared/(.*)$": "<rootDir>/shared/$1"
       },
-      "setupFiles": ["<rootDir>/__mocks__/console.js"],
+      "setupFiles": [
+        "<rootDir>/__mocks__/console.js",
+        "<rootDir>/server/test/setupMocks.js"
+      ],
       "setupFilesAfterEnv": ["<rootDir>/server/test/setup.ts"],
       "globalTeardown": "<rootDir>/server/test/globalTeardown.js",
       "testEnvironment": "node"

package.json

@@ -28,7 +28,7 @@
     "db:rollback": "sequelize db:migrate:undo",
     "db:reset": "sequelize db:drop && sequelize db:create && sequelize db:migrate",
     "upgrade": "git fetch && git pull && yarn install && yarn heroku-postbuild",
-    "test": "TZ=UTC jest --config=.jestconfig.json --forceExit",
+    "test": "TZ=UTC jest --config=.jestconfig.json",
     "test:app": "TZ=UTC jest --config=.jestconfig.json --selectProjects app",
     "test:shared": "TZ=UTC jest --config=.jestconfig.json --selectProjects shared-node shared-jsdom",
     "test:server": "TZ=UTC jest --config=.jestconfig.json --selectProjects server",

server/emails/templates/BaseEmail.tsx

@@ -68,7 +68,7 @@ export default abstract class BaseEmail<
 
     // Ideally we'd use EmailTask.schedule here but importing creates a circular
     // dependency so we're pushing onto the task queue in the expected format
-    return taskQueue.add(
+    return taskQueue().add(
       {
         name: "EmailTask",
         props: {

server/models/Event.ts

@@ -88,11 +88,11 @@ class Event extends IdModel<
       // We want to use the parent transaction, otherwise the 'afterCommit' hook will never fire in this case.
       // See: https://github.com/sequelize/sequelize/issues/17452
       (options.transaction.parent || options.transaction).afterCommit(
-        () => void globalEventQueue.add(model)
+        () => void globalEventQueue().add(model)
       );
       return;
     }
-    void globalEventQueue.add(model);
+    void globalEventQueue().add(model);
   }
 
   // associations
@@ -138,7 +138,7 @@ class Event extends IdModel<
    */
   static schedule(event: Partial<Event>) {
     const now = new Date();
-    return globalEventQueue.add(
+    return globalEventQueue().add(
       this.build({
         createdAt: now,
         ...event,

server/queues/index.ts

@@ -1,30 +1,54 @@
 import { createQueue } from "@server/queues/queue";
 import { Second } from "@shared/utils/time";
 
-export const globalEventQueue = createQueue("globalEvents", {
-  attempts: 5,
-  backoff: {
-    type: "exponential",
-    delay: Second.ms,
-  },
-});
+let cachedGlobalEventQueue: ReturnType<typeof createQueue> | undefined;
+export const globalEventQueue = () => {
+  if (!cachedGlobalEventQueue) {
+    cachedGlobalEventQueue = createQueue("globalEvents", {
+      attempts: 5,
+      backoff: {
+        type: "exponential",
+        delay: Second.ms,
+      },
+    });
+  }
+  return cachedGlobalEventQueue;
+};
 
-export const processorEventQueue = createQueue("processorEvents", {
-  attempts: 5,
-  backoff: {
-    type: "exponential",
-    delay: 10 * Second.ms,
-  },
-});
+let cachedProcessorEventQueue: ReturnType<typeof createQueue> | undefined;
+export const processorEventQueue = () => {
+  if (!cachedProcessorEventQueue) {
+    cachedProcessorEventQueue = createQueue("processorEvents", {
+      attempts: 5,
+      backoff: {
+        type: "exponential",
+        delay: 10 * Second.ms,
+      },
+    });
+  }
+  return cachedProcessorEventQueue;
+};
 
-export const websocketQueue = createQueue("websockets", {
-  timeout: 10 * Second.ms,
-});
+let cachedWebsocketQueue: ReturnType<typeof createQueue> | undefined;
+export const websocketQueue = () => {
+  if (!cachedWebsocketQueue) {
+    cachedWebsocketQueue = createQueue("websockets", {
+      timeout: 10 * Second.ms,
+    });
+  }
+  return cachedWebsocketQueue;
+};
 
-export const taskQueue = createQueue("tasks", {
-  attempts: 5,
-  backoff: {
-    type: "exponential",
-    delay: 10 * Second.ms,
-  },
-});
+let cachedTaskQueue: ReturnType<typeof createQueue> | undefined;
+export const taskQueue = () => {
+  if (!cachedTaskQueue) {
+    cachedTaskQueue = createQueue("tasks", {
+      attempts: 5,
+      backoff: {
+        type: "exponential",
+        delay: 10 * Second.ms,
+      },
+    });
+  }
+  return cachedTaskQueue;
+};

server/queues/processors/DebounceProcessor.ts

@@ -13,7 +13,7 @@ export default class DebounceProcessor extends BaseProcessor {
   async perform(event: Event) {
     switch (event.name) {
       case "documents.update": {
-        await globalEventQueue.add(
+        await globalEventQueue().add(
           { ...event, name: "documents.update.delayed" },
           {
             // speed up revision creation in development, we don't have all the
@@ -41,7 +41,7 @@ export default class DebounceProcessor extends BaseProcessor {
           return;
         }
 
-        await globalEventQueue.add({
+        await globalEventQueue().add({
           ...event,
           name: "documents.update.debounced",
         });

server/queues/tasks/base/BaseTask.ts

@@ -17,7 +17,7 @@ export abstract class BaseTask<T extends Record<string, any>> {
    * @returns A promise that resolves once the job is placed on the task queue
    */
   public schedule(props: T, options?: JobOptions): Promise<Job> {
-    return taskQueue.add(
+    return taskQueue().add(
       {
         name: this.constructor.name,
         props,

server/services/admin.ts

@@ -13,10 +13,10 @@ export default function init(app: Koa) {
   const serverAdapter = new KoaAdapter();
   createBullBoard({
     queues: [
-      new BullAdapter(globalEventQueue),
-      new BullAdapter(processorEventQueue),
-      new BullAdapter(websocketQueue),
-      new BullAdapter(taskQueue),
+      new BullAdapter(globalEventQueue()),
+      new BullAdapter(processorEventQueue()),
+      new BullAdapter(websocketQueue()),
+      new BullAdapter(taskQueue()),
     ],
     serverAdapter,
   });

server/services/websockets.ts

@@ -142,7 +142,7 @@ export default function init(
 
   // Handle events from event queue that should be sent to the clients down ws
   const websockets = new WebsocketsProcessor();
-  websocketQueue
+  websocketQueue()
     .process(
       traceFunction({
         serviceName: "websockets",

server/services/worker.ts

@@ -18,7 +18,7 @@ export default async function init() {
   await initI18n();
 
   // This queue processes the global event bus
-  globalEventQueue
+  globalEventQueue()
     .process(
       env.WORKER_CONCURRENCY_EVENTS,
       traceFunction({
@@ -52,12 +52,12 @@ export default async function init() {
             if (name === "WebsocketsProcessor") {
               // websockets are a special case on their own queue because they must
               // only be consumed by the websockets service rather than workers.
-              await websocketQueue.add(job.data);
+              await websocketQueue().add(job.data);
             } else if (
               ProcessorClass.applicableEvents.includes(event.name) ||
               ProcessorClass.applicableEvents.includes("*")
             ) {
-              await processorEventQueue.add({ event, name });
+              await processorEventQueue().add({ event, name });
             }
           } catch (error) {
             Logger.error(
@@ -80,7 +80,7 @@ export default async function init() {
 
   // Jobs for individual processors are processed here. Only applicable events
   // as unapplicable events were filtered in the global event queue above.
-  processorEventQueue
+  processorEventQueue()
     .process(
       env.WORKER_CONCURRENCY_EVENTS,
       traceFunction({
@@ -131,7 +131,7 @@ export default async function init() {
     });
 
   // Jobs for async tasks are processed here.
-  taskQueue
+  taskQueue()
     .process(
       env.WORKER_CONCURRENCY_TASKS,
       traceFunction({
@@ -173,7 +173,7 @@ export default async function init() {
       Logger.fatal("Error starting taskQueue", err);
     });
 
-  HealthMonitor.start(globalEventQueue);
-  HealthMonitor.start(processorEventQueue);
-  HealthMonitor.start(taskQueue);
+  HealthMonitor.start(globalEventQueue());
+  HealthMonitor.start(processorEventQueue());
+  HealthMonitor.start(taskQueue());
 }

server/test/TestServer.ts

@@ -48,6 +48,7 @@ class TestServer {
   close() {
     this.listener = null;
     this.server.closeAllConnections();
+    this.server.close();
   }
 
   delete(path: string, options?: any) {

server/test/setup.ts

@@ -7,11 +7,6 @@ import { EventEmitter } from "events";
 // This needs to be done before any modules that use EventEmitter are loaded
 EventEmitter.defaultMaxListeners = 100;
 
-// Enable mocks for Redis-related modules
-jest.mock("ioredis", () => require("ioredis-mock"));
-jest.mock("@server/utils/MutexLock");
-jest.mock("@server/utils/CacheHelper");
-
 // Enable fetch mocks for testing
 require("jest-fetch-mock").enableMocks();
 fetchMock.dontMock();

server/test/setupMocks.js

@@ -0,0 +1,12 @@
+// This file runs before the test environment is set up to ensure mocks are registered early
+// It prevents real Redis clients from being initialized during module imports
+
+// Mock ioredis with ioredis-mock before any imports
+jest.mock("ioredis", () => require("ioredis-mock"));
+
+// Mock other Redis-dependent modules
+jest.mock("@server/utils/MutexLock");
+jest.mock("@server/utils/CacheHelper");
+
+// Mock AWS SDK signature module to prevent aws_logger open handle
+jest.mock("@aws-sdk/signature-v4-crt", () => ({}));