Feat [ Test ]: migrate from redis-mock to redis testcontainer for tes…

…ts (#336) * set up redis testcontainert instead of redis-mock * cherry-pick tests refactoring * rm redundant file * refactor grouper tests * lint fix * lint fix
codex-team · Dec 25, 2024 · 4d1973c · 4d1973c
1 parent 1322621
commit 4d1973c
Show file tree

Hide file tree

Showing 11 changed files with 411 additions and 108 deletions.
diff --git a/jest.setup.redis-mock.js b/jest.setup.redis-mock.js
@@ -1,4 +1,25 @@
+const { GenericContainer } = require('testcontainers');
+
+let redisTestContainer;
+
 /**
- * Mock redis library
+ * Create test container with Redis, which could be used in tests
  */
-jest.mock('redis', () => jest.requireActual('redis-mock'));
+beforeAll(async () => {
+  redisTestContainer = await new GenericContainer('redis')
+    .withExposedPorts(6379)
+    .start();
+
+  const port = redisTestContainer.getMappedPort(6379);
+  const host = redisTestContainer.getContainerIpAddress();
+
+  /**
+   * Set environment variable for redisHelper to connect to redis container
+   */
+  process.env.REDIS_URL = `redis://${host}:${port}`;
+}
+);
+
+afterAll(async () => {
+  await redisTestContainer.stop();
+});
diff --git a/package.json b/package.json
@@ -67,16 +67,17 @@
         "typescript": "^3.8.3",
         "uuid": "^8.3.0",
         "winston": "^3.2.1",
-        "yup": "^0.28.5"
+        "yup": "^0.28.5",
+        "redis": "^4.7.0"
     },
     "devDependencies": {
         "@shelf/jest-mongodb": "^1.2.3",
+        "testcontainers": "^3.0.0",
         "eslint": "^7.14.0",
         "eslint-config-codex": "^1.6.1",
         "jest": "25.5.4",
         "nodemon": "^2.0.3",
         "random-words": "^1.1.1",
-        "redis-mock": "^0.56.3",
         "ts-jest": "25.4.0",
         "wait-for-expect": "^3.0.2",
         "webpack": "^4.43.0",

diff --git a/workers/grouper/package.json b/workers/grouper/package.json
@@ -10,7 +10,6 @@
   "workerType": "grouper",
   "dependencies": {
     "@types/redis": "^2.8.28",
-    "js-levenshtein": "^1.1.6",
-    "redis": "^3.1.1"
+    "js-levenshtein": "^1.1.6"
   }
 }
diff --git a/workers/grouper/src/index.ts b/workers/grouper/src/index.ts
@@ -51,6 +51,8 @@ export default class GrouperWorker extends Worker {
   public async start(): Promise<void> {
     await this.db.connect();
     this.prepareCache();
+    await this.redis.initialize();
+
     await super.start();
   }
 
@@ -61,6 +63,7 @@ export default class GrouperWorker extends Worker {
     await super.finish();
     this.prepareCache();
     await this.db.close();
+    await this.redis.close();
   }
 
   /**

diff --git a/workers/grouper/src/redisHelper.ts b/workers/grouper/src/redisHelper.ts
@@ -1,5 +1,5 @@
 import HawkCatcher from '@hawk.so/nodejs';
-import redis from 'redis';
+import { createClient, RedisClientType } from 'redis';
 import createLogger from '../../../lib/logger';
 
 /**
@@ -14,27 +14,70 @@ export default class RedisHelper {
   /**
    * Redis client for making queries
    */
-  private readonly redisClient = redis.createClient({ url: process.env.REDIS_URL });
+  private readonly redisClient: RedisClientType;
 
   /**
    * Logger instance
    * (default level='info')
    */
   private logger = createLogger();
 
+  /**
+   * Constructor of the Redis helper class
+   * Initializes the Redis client and sets up error handling
+   */
+  constructor() {
+    this.redisClient = createClient({ url: process.env.REDIS_URL });
+
+    this.redisClient.on('error', (error) => {
+      if (error) {
+        this.logger.error(error);
+        HawkCatcher.send(error);
+      }
+    });
+  }
+
+  /**
+   * Connect to redis client
+   */
+  public async initialize(): Promise<void> {
+    try {
+      await this.redisClient.connect();
+    } catch (error) {
+      console.error('Error connecting to redis', error);
+    }
+  }
+
+  /**
+   * Close redis client
+   */
+  public async close(): Promise<void> {
+    if (this.redisClient.isOpen) {
+      await this.redisClient.quit();
+    }
+  }
+
   /**
    * Checks if a lock exists on the given group hash and identifier pair. If it does not exist, creates a lock.
    * Returns true if lock exists
    *
    * @param groupHash - event group hash
    * @param userId - event user id
    */
-  public checkOrSetEventLock(groupHash: string, userId: string): Promise<boolean> {
-    return new Promise((resolve, reject) => {
-      const callback = this.createCallback(resolve, reject);
+  public async checkOrSetEventLock(groupHash: string, userId: string): Promise<boolean> {
+    const result = await this.redisClient.set(
+      `${groupHash}:${userId}`,
+      '1',
+      {
+        EX: RedisHelper.LOCK_TTL,
+        NX: true,
+      } as const
+    );
 
-      this.redisClient.set(`${groupHash}:${userId}`, '1', 'EX', RedisHelper.LOCK_TTL, 'NX', callback);
-    });
+    /**
+     * Result would be null if lock already exists, false otherwise
+     */
+    return result === null;
   }
 
   /**

diff --git a/workers/grouper/tests/index.test.ts b/workers/grouper/tests/index.test.ts
@@ -1,7 +1,7 @@
 import '../../../env-test';
 import GrouperWorker from '../src';
 import { GroupWorkerTask } from '../types/group-worker-task';
-import redis from 'redis';
+import { createClient, RedisClientType } from 'redis';
 import { Collection, MongoClient } from 'mongodb';
 import { EventAddons, EventDataAccepted } from '@hawk.so/types';
 
@@ -74,14 +74,16 @@ function generateTask(event: Partial<EventDataAccepted<EventAddons>> = undefined
 }
 
 describe('GrouperWorker', () => {
-  const worker = new GrouperWorker();
   let connection: MongoClient;
   let eventsCollection: Collection;
   let dailyEventsCollection: Collection;
   let repetitionsCollection: Collection;
-  let redisClient;
+  let redisClient: RedisClientType;
+  let worker: GrouperWorker;
 
   beforeAll(async () => {
+    worker = new GrouperWorker();
+
     await worker.start();
     connection = await MongoClient.connect(process.env.MONGO_EVENTS_DATABASE_URI, {
       useNewUrlParser: true,
@@ -90,7 +92,10 @@ describe('GrouperWorker', () => {
     eventsCollection = connection.db().collection('events:' + projectIdMock);
     dailyEventsCollection = connection.db().collection('dailyEvents:' + projectIdMock);
     repetitionsCollection = connection.db().collection('repetitions:' + projectIdMock);
-    redisClient = redis.createClient({ url: process.env.REDIS_URL });
+
+    redisClient = createClient({ url: process.env.REDIS_URL });
+    await redisClient.connect();
+
     jest.resetAllMocks();
   });
 
@@ -104,8 +109,8 @@ describe('GrouperWorker', () => {
     await repetitionsCollection.deleteMany({});
   });
 
-  afterEach((done) => {
-    redisClient.flushall(done);
+  afterEach(async () => {
+    await redisClient.flushAll();
   });
 
   describe('Saving events', () => {
@@ -299,6 +304,7 @@ describe('GrouperWorker', () => {
   });
 
   afterAll(async () => {
+    await redisClient.quit();
     await worker.finish();
     await connection.close();
   });

diff --git a/workers/limiter/package.json b/workers/limiter/package.json
@@ -10,7 +10,6 @@
     "workerType": "cron-tasks/limiter",
     "dependencies": {
         "@types/redis": "^2.8.28",
-        "axios": "^0.21.2",
-        "redis": "^3.1.1"
+        "axios": "^0.21.2"
     }
 }
diff --git a/workers/limiter/src/index.ts b/workers/limiter/src/index.ts
@@ -81,6 +81,9 @@ export default class LimiterWorker extends Worker {
 
     this.projectsCollection = accountDbConnection.collection<ProjectDBScheme>('projects');
     this.workspacesCollection = accountDbConnection.collection<WorkspaceDBScheme>('workspaces');
+
+    await this.redis.initialize();
+
     await super.start();
   }
 
@@ -91,6 +94,7 @@ export default class LimiterWorker extends Worker {
     await super.finish();
     await this.eventsDb.close();
     await this.accountsDb.close();
+    await this.redis.close();
   }
 
   /**

diff --git a/workers/limiter/src/redisHelper.ts b/workers/limiter/src/redisHelper.ts
@@ -1,5 +1,5 @@
 import HawkCatcher from '@hawk.so/nodejs';
-import redis from 'redis';
+import { createClient, RedisClientType } from 'redis';
 import createLogger from '../../../lib/logger';
 
 /**
@@ -9,7 +9,8 @@ export default class RedisHelper {
   /**
    * Redis client for making queries
    */
-  private readonly redisClient = redis.createClient({ url: process.env.REDIS_URL });
+  // private readonly redisClient = redis.createClient({ url: process.env.REDIS_URL });
+  private readonly redisClient: RedisClientType;
 
   /**
    * Logger instance
@@ -22,6 +23,35 @@ export default class RedisHelper {
    */
   private readonly redisDisabledProjectsKey = 'DisabledProjectsSet';
 
+  /**
+   * Constructor of the Redis helper class
+   * Initializes the Redis client and sets up error handling
+   */
+  constructor() {
+    this.redisClient = createClient({ url: process.env.REDIS_URL });
+
+    this.redisClient.on('error', (error) => {
+      this.logger.error(error);
+      HawkCatcher.send(error);
+    });
+  }
+
+  /**
+   * Connect to redis client
+   */
+  public async initialize(): Promise<void> {
+    await this.redisClient.connect();
+  }
+
+  /**
+   * Close redis client
+   */
+  public async close(): Promise<void> {
+    if (this.redisClient.isOpen) {
+      await this.redisClient.quit();
+    }
+  }
+
   /**
    * Saves banned project ids to redis
    * If there is no projects, then previous data in Redis will be erased
@@ -33,12 +63,26 @@ export default class RedisHelper {
       const callback = this.createCallback(resolve, reject);
 
       if (projectIdsToBan.length) {
-        this.redisClient.multi()
-          .del(this.redisDisabledProjectsKey)
-          .sadd(this.redisDisabledProjectsKey, projectIdsToBan)
-          .exec(callback);
+        const pipeline = this.redisClient.multi();
+
+        pipeline.del(this.redisDisabledProjectsKey);
+
+        pipeline.sAdd(this.redisDisabledProjectsKey, projectIdsToBan);
+
+        try {
+          pipeline.exec();
+          callback(null);
+        } catch (err) {
+          callback(err);
+        }
       } else {
-        this.redisClient.del(this.redisDisabledProjectsKey, callback);
+        this.redisClient.del(this.redisDisabledProjectsKey)
+          .then(() => {
+            callback(null);
+          })
+          .catch((err) => {
+            callback(err);
+          });
       }
     });
   }
@@ -53,7 +97,9 @@ export default class RedisHelper {
       const callback = this.createCallback(resolve, reject);
 
       if (projectIds.length) {
-        this.redisClient.sadd(this.redisDisabledProjectsKey, projectIds, callback);
+        this.redisClient.sAdd(this.redisDisabledProjectsKey, projectIds)
+          .then(() => callback(null))
+          .catch((err) => callback(err));
       } else {
         resolve();
       }
@@ -70,7 +116,9 @@ export default class RedisHelper {
       const callback = this.createCallback(resolve, reject);
 
       if (projectIds.length) {
-        this.redisClient.srem(this.redisDisabledProjectsKey, projectIds, callback);
+        this.redisClient.sRem(this.redisDisabledProjectsKey, projectIds)
+          .then(() => callback(null))
+          .catch((err) => callback(err));
       } else {
         resolve();
       }