Add unit tests for onWorkerUnavailable

app/gen-server/lib/DocWorkerMap.ts

@@ -181,9 +181,15 @@ class DummyDocWorkerMap implements IDocWorkerMap {
  * "default".
  */
 export class DocWorkerMap implements IDocWorkerMap {
+  public static get MONITOR_AVAILABILITY_INTERVAL() {
+    return 30_000;  // 30 seconds
+  }
+
   private _client: RedisClient;
   private _clients: RedisClient[];
   private _redlock: Redlock;
+  // Required for testing, so we can stub it.
+  private _log = log;
 
   // Optional deploymentKey argument supplies a key unique to the deployment (this is important
   // for maintaining groups across redeployments only)
@@ -310,34 +316,35 @@ export class DocWorkerMap implements IDocWorkerMap {
     }
   }
 
+  /**
+   * Monitor the availability of a worker.  If the worker is no longer available,
+   * call the callback.
+   *
+   * @param workerInfo The worker we are checking for presence.
+   * @param cb A callback to be called when the worker is no longer available.
+   * @returns A function to stop monitoring the worker.
+   */
   public onWorkerUnavailable(workerInfo: DocWorkerInfo, cb: () => void): () => void {
     let timeout: NodeJS.Timeout;
     const group = workerInfo.group || 'default';
     const workerId = workerInfo.id;
 
-    (function recursiveTimeout(self, nbFailures = 0) {
+    (function recursiveTimeout(this: DocWorkerMap) {
       timeout = setTimeout(async () => {
-        if (nbFailures >= 3) {
-          log.error(
-            'DocWorkerMap: Presence checker failed 3 times, considering the worker "%s" is not available anymore',
-            workerId
-          );
-          return cb();
-        }
         try {
-          if (!await self._client.sismemberAsync(`workers-available-${group}`, workerId)) {
-            log.error('DocWorkerMap: Worker "%s" has been marked as unavailable in Redis', workerId);
+          if (!await this._client.sismemberAsync(`workers-available-${group}`, workerId)) {
+            this._log.error('DocWorkerMap: Worker "%s" has been marked as unavailable in Redis', workerId);
             return cb();
           }
-          return recursiveTimeout(self);
         } catch (err) {
-          log.error('DocWorkerMap: Presence checker failed for worker "%s"', workerId, err);
-          return recursiveTimeout(self, nbFailures + 1);
+          this._log.error('DocWorkerMap: Presence checker failed for worker "%s"', workerId, err);
         }
-      }, 30_000);
-    })(this);
+        return recursiveTimeout.call(this);
+      }, DocWorkerMap.MONITOR_AVAILABILITY_INTERVAL);
+    }).call(this);
+
     return () => {
-      log.info('DocWorkerMap: Clearing presence checker for worker "%s"', workerId);
+      this._log.info('DocWorkerMap: Clearing presence checker for worker "%s"', workerId);
       clearTimeout(timeout);
     };
   }

test/gen-server/lib/DocWorkerMap.ts

@@ -0,0 +1,111 @@
+// Test for DocWorkerMap.ts
+
+import { DocWorkerMap } from 'app/gen-server/lib/DocWorkerMap';
+import { DocWorkerInfo } from 'app/server/lib/DocWorkerMap';
+import {expect} from 'chai';
+import sinon from 'sinon';
+
+describe('DocWorkerMap', () => {
+  const sandbox = sinon.createSandbox();
+  afterEach(() => {
+    sandbox.restore();
+  });
+  describe('onWorkerUnavailable', () => {
+    const baseWorkerInfo: DocWorkerInfo = {
+      id: 'workerId',
+      internalUrl: 'internalUrl',
+      publicUrl: 'publicUrl',
+      group: undefined
+    };
+    let unsubscribe: () => void | undefined;
+    function fixtures() {
+      const sismemberAsyncStub = sinon.stub();
+      const stubDocWorkerMap = {
+        _log: { error: sinon.stub(), info: sinon.stub() },
+        _client: { sismemberAsync: sismemberAsyncStub }
+      };
+      const onUnavailabilitySpy = sinon.spy();
+      return { onUnavailabilitySpy, stubDocWorkerMap, sismemberAsyncStub };
+    }
+    afterEach(() => unsubscribe?.());
+
+    it('should monitor worker availability regularly', async () => {
+      sandbox.useFakeTimers();
+      const { stubDocWorkerMap, sismemberAsyncStub, onUnavailabilitySpy } = fixtures();
+      sismemberAsyncStub.resolves(true);
+      unsubscribe = DocWorkerMap.prototype.onWorkerUnavailable.call(
+        stubDocWorkerMap, baseWorkerInfo, onUnavailabilitySpy
+      );
+      expect(sismemberAsyncStub.called).to.equal(false);
+
+      await sandbox.clock.tickAsync(DocWorkerMap.MONITOR_AVAILABILITY_INTERVAL + 1);
+      expect(sismemberAsyncStub.calledOnceWith('workers-available-default', baseWorkerInfo.id)).to.equal(true);
+
+      await sandbox.clock.tickAsync(DocWorkerMap.MONITOR_AVAILABILITY_INTERVAL + 1);
+      expect(sismemberAsyncStub.callCount).to.equal(2);
+
+      expect(onUnavailabilitySpy.called).to.equal(false);
+    });
+
+    it('should log when it cannot join Redis', async () => {
+      sandbox.useFakeTimers();
+      const err = new Error('Cannot join Redis');
+      const { stubDocWorkerMap, sismemberAsyncStub, onUnavailabilitySpy } = fixtures();
+
+      sismemberAsyncStub.rejects(err);
+      unsubscribe = DocWorkerMap.prototype.onWorkerUnavailable.call(
+        stubDocWorkerMap, baseWorkerInfo, onUnavailabilitySpy
+      );
+
+      await sandbox.clock.tickAsync(DocWorkerMap.MONITOR_AVAILABILITY_INTERVAL + 1);
+
+      expect(
+        stubDocWorkerMap._log.error.calledWithMatch(/Presence checker failed/, baseWorkerInfo.id, err)
+      ).to.equal(true);
+    });
+
+    it('should trigger callback when the worker is not available anymore', async () => {
+      sandbox.useFakeTimers();
+      const { stubDocWorkerMap, sismemberAsyncStub, onUnavailabilitySpy } = fixtures();
+      sismemberAsyncStub.resolves(true);
+      unsubscribe = DocWorkerMap.prototype.onWorkerUnavailable.call(
+        stubDocWorkerMap, baseWorkerInfo, onUnavailabilitySpy
+      );
+
+      await sandbox.clock.tickAsync(DocWorkerMap.MONITOR_AVAILABILITY_INTERVAL + 1);
+
+      expect(onUnavailabilitySpy.called).to.equal(false);
+
+      sismemberAsyncStub.resolves(false);
+      await sandbox.clock.tickAsync(DocWorkerMap.MONITOR_AVAILABILITY_INTERVAL + 1);
+
+      expect(onUnavailabilitySpy.calledOnce).to.equal(true);
+    });
+
+    it('should not run watch for unavailability after unsubscription', async () => {
+      sandbox.useFakeTimers();
+      const { stubDocWorkerMap, sismemberAsyncStub, onUnavailabilitySpy } = fixtures();
+      sismemberAsyncStub.resolves(true);
+
+      unsubscribe = DocWorkerMap.prototype.onWorkerUnavailable.call(
+        stubDocWorkerMap, baseWorkerInfo, onUnavailabilitySpy
+      );
+
+      await sandbox.clock.tickAsync(DocWorkerMap.MONITOR_AVAILABILITY_INTERVAL + 1);
+
+      expect(
+        stubDocWorkerMap._client.sismemberAsync.calledOnce
+      ).to.equal(true);
+      expect(stubDocWorkerMap._log.info.called).to.equal(false);
+
+      unsubscribe();
+      expect(stubDocWorkerMap._log.info.calledWithMatch(/Clearing presence checker/, baseWorkerInfo.id)).to.equal(true);
+      stubDocWorkerMap._client.sismemberAsync.resetHistory();
+
+      await sandbox.clock.tickAsync(DocWorkerMap.MONITOR_AVAILABILITY_INTERVAL + 1);
+      expect(
+        stubDocWorkerMap._client.sismemberAsync.called
+      ).to.equal(false);
+    });
+  });
+});