fix: tests were leaking EventEmitter handlers

Author: feywind

test/lease-manager.ts

@@ -108,6 +108,8 @@ describe('LeaseManager', () => {
   let LeaseManager: typeof leaseTypes.LeaseManager;
   let leaseManager: leaseTypes.LeaseManager;
 
+  let fakeLog: FakeLog | undefined;
+
   before(() => {
     LeaseManager = proxyquire('../src/lease-manager.js', {
       os: fakeos,
@@ -121,6 +123,7 @@ describe('LeaseManager', () => {
   });
 
   afterEach(() => {
+    fakeLog?.remove();
     leaseManager.clear();
     sandbox.restore();
   });
@@ -197,20 +200,20 @@ describe('LeaseManager', () => {
       fakeMessage.id = 'a';
       fakeMessage.ackId = 'b';
 
-      const fakeLog = new FakeLog(leaseTypes.logs.callbackDelivery);
+      fakeLog = new FakeLog(leaseTypes.logs.callbackDelivery);
 
       leaseManager.setOptions({
         allowExcessMessages: true,
       });
 
       subscriber.on('message', () => {
-        assert.strictEqual(fakeLog.called, true);
+        assert.strictEqual(fakeLog!.called, true);
         assert.strictEqual(
-          fakeLog.fields!.severity,
+          fakeLog!.fields!.severity,
           loggingUtils.LogSeverity.INFO,
         );
-        assert.strictEqual(fakeLog.args![1] as string, 'a');
-        assert.strictEqual(fakeLog.args![2] as string, 'b');
+        assert.strictEqual(fakeLog!.args![1] as string, 'a');
+        assert.strictEqual(fakeLog!.args![2] as string, 'b');
         done();
       });
 
@@ -222,7 +225,7 @@ describe('LeaseManager', () => {
       fakeMessage.id = 'a';
       fakeMessage.ackId = 'b';
 
-      const fakeLog = new FakeLog(leaseTypes.logs.callbackExceptions);
+      fakeLog = new FakeLog(leaseTypes.logs.callbackExceptions);
 
       leaseManager.setOptions({
         allowExcessMessages: true,
@@ -285,7 +288,7 @@ describe('LeaseManager', () => {
       const pendingStub = sandbox.stub(leaseManager, 'pending');
       pendingStub.get(() => 0);
       leaseManager.setOptions({allowExcessMessages: false});
-      const fakeLog = new FakeLog(leaseTypes.logs.subscriberFlowControl);
+      fakeLog = new FakeLog(leaseTypes.logs.subscriberFlowControl);
 
       leaseManager.add(fakeMessage);
       assert.strictEqual(fakeLog.called, true);
@@ -392,7 +395,7 @@ describe('LeaseManager', () => {
         const removeStub = sandbox.stub(leaseManager, 'remove');
         const modAckStub = sandbox.stub(goodMessage, 'modAck');
 
-        const fakeLog = new FakeLog(leaseTypes.logs.expiry);
+        fakeLog = new FakeLog(leaseTypes.logs.expiry);
 
         leaseManager.add(goodMessage as {} as Message);
         clock.tick(halfway);
@@ -474,7 +477,7 @@ describe('LeaseManager', () => {
     });
 
     it('should log if it was full and is now empty', () => {
-      const fakeLog = new FakeLog(leaseTypes.logs.subscriberFlowControl);
+      fakeLog = new FakeLog(leaseTypes.logs.subscriberFlowControl);
       const pendingStub = sandbox.stub(leaseManager, 'pending');
       pendingStub.get(() => 0);
       leaseManager.add(new FakeMessage() as {} as Message);
@@ -613,7 +616,7 @@ describe('LeaseManager', () => {
       const pending = new FakeMessage() as {} as Message;
 
       leaseManager.setOptions({allowExcessMessages: false, maxMessages: 1});
-      const fakeLog = new FakeLog(leaseTypes.logs.subscriberFlowControl);
+      fakeLog = new FakeLog(leaseTypes.logs.subscriberFlowControl);
 
       leaseManager.add(temp);
       leaseManager.add(pending);

test/message-queues.ts

@@ -415,6 +415,8 @@ describe('MessageQueues', () => {
       messages.forEach(message => ackQueue.add(message as Message));
       await ackQueue.flush('logtest');
 
+      fakeLog.remove();
+
       assert.strictEqual(fakeLog.called, true);
       assert.strictEqual(
         fakeLog.fields!.severity,
@@ -674,6 +676,8 @@ describe('MessageQueues', () => {
       messages.forEach(message => modAckQueue.add(message as Message));
       await modAckQueue.flush('logtest');
 
+      fakeLog.remove();
+
       assert.strictEqual(fakeLog.called, true);
       assert.strictEqual(
         fakeLog.fields!.severity,

test/publisher/message-queues.ts

@@ -184,6 +184,8 @@ describe('Message Queues', () => {
         sandbox.stub(topic, 'request');
         const fakeLog = new FakeLog(q.logs.publishBatch);
         void queue._publish(messages, callbacks, 0, 'test');
+        fakeLog.remove();
+
         assert.strictEqual(fakeLog.called, true);
         assert.strictEqual(
           fakeLog.fields!.severity,

test/subscriber.ts

@@ -206,6 +206,8 @@ describe('Subscriber', () => {
   let Subscriber: typeof s.Subscriber;
   let subscriber: s.Subscriber;
 
+  let fakeLog: FakeLog | undefined;
+
   beforeEach(() => {
     sandbox = sinon.createSandbox();
     fakeProjectify = {
@@ -243,6 +245,8 @@ describe('Subscriber', () => {
   });
 
   afterEach(async () => {
+    fakeLog?.remove();
+    fakeLog = undefined;
     sandbox.restore();
     await subscriber.close();
     tracing.setGloballyEnabled(false);
@@ -441,7 +445,7 @@ describe('Subscriber', () => {
     });
 
     it('should log on ack completion', async () => {
-      const fakeLog = new FakeLog(s.logs.ackNack);
+      fakeLog = new FakeLog(s.logs.ackNack);
 
       await subscriber.ack(message);
 
@@ -459,7 +463,7 @@ describe('Subscriber', () => {
 
       message.received = 0;
       sandbox.stub(histogram, 'percentile').withArgs(99).returns(10);
-      const fakeLog = new FakeLog(s.logs.slowAck);
+      fakeLog = new FakeLog(s.logs.slowAck);
 
       await subscriber.ack(message);
 
@@ -926,7 +930,7 @@ describe('Subscriber', () => {
     });
 
     it('should log on ack completion', async () => {
-      const fakeLog = new FakeLog(s.logs.ackNack);
+      fakeLog = new FakeLog(s.logs.ackNack);
 
       await subscriber.nack(message);
 

test/test-utils.ts

@@ -63,12 +63,23 @@ export class FakeLog {
   fields?: loggingUtils.LogFields;
   args?: unknown[];
   called = false;
+  log: loggingUtils.AdhocDebugLogFunction;
+  listener: (lf: loggingUtils.LogFields, a: unknown[]) => void;
 
   constructor(log: loggingUtils.AdhocDebugLogFunction) {
-    log.on('log', (lf, a) => {
+    this.log = log;
+    this.listener = (lf: loggingUtils.LogFields, a: unknown[]) => {
       this.fields = lf;
       this.args = a;
       this.called = true;
-    });
+    };
+    this.log.on('log', this.listener);
+  }
+
+  remove() {
+    // This really ought to be properly exposed, but since it's not, we'll
+    // do this for now to keep the tests from being leaky.
+    const instance = (this.log as loggingUtils.AdhocDebugLogFunction).instance;
+    instance.off('log', this.listener);
   }
 }