From 506f61838a725bfa880560126ae30686f388d08e Mon Sep 17 00:00:00 2001
From: jarek <jaroslaw.sadzinski@gmail.com>
Date: Thu, 22 Dec 2022 18:15:06 +0100
Subject: [PATCH] Fixing time bug in webhook tests (#383)

Webhook tests were reusing date in logs, which caused a random failure in tests that checked updatedTime.
---
 app/server/lib/Triggers.ts | 11 ++++-------
 test/server/lib/DocApi.ts  | 13 ++++++-------
 2 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/app/server/lib/Triggers.ts b/app/server/lib/Triggers.ts
index e20114ab..e4f272d1 100644
--- a/app/server/lib/Triggers.ts
+++ b/app/server/lib/Triggers.ts
@@ -655,7 +655,6 @@ export class DocTriggers {
   private async _sendWebhookWithRetries(id: string, url: string, body: string, size: number, signal: AbortSignal) {
     const maxWait = 64;
     let wait = 1;
-    let now = Date.now();
     for (let attempt = 0; attempt < this._maxWebhookAttempts; attempt++) {
       if (this._shuttingDown) {
         return false;
@@ -672,12 +671,11 @@ export class DocTriggers {
           },
           signal,
         });
-        now = Date.now();
         if (response.status === 200) {
-          await this._stats.logBatch(id, 'success', now, { size, httpStatus: 200, error: null, attempts: attempt + 1 });
+          await this._stats.logBatch(id, 'success', { size, httpStatus: 200, error: null, attempts: attempt + 1 });
           return true;
         }
-        await this._stats.logBatch(id, 'failure', now, {
+        await this._stats.logBatch(id, 'failure', {
           httpStatus: response.status,
           error: await response.text(),
           attempts: attempt + 1,
@@ -685,7 +683,7 @@ export class DocTriggers {
         });
         this._log(`Webhook responded with non-200 status`, {level: 'warn', status: response.status, attempt});
       } catch (e) {
-        await this._stats.logBatch(id, 'failure', now, {
+        await this._stats.logBatch(id, 'failure', {
           httpStatus: null,
           error: (e.message || 'Unrecognized error during fetch'),
           attempts: attempt + 1,
@@ -873,7 +871,6 @@ class WebhookStatistics extends PersistedStore<StatsKey> {
   public async logBatch(
     id: string,
     status: WebhookBatchStatus,
-    now?: number|null,
     stats?: {
       httpStatus?: number|null,
       error?: string|null,
@@ -881,7 +878,7 @@ class WebhookStatistics extends PersistedStore<StatsKey> {
       attempts?: number|null,
     }
   ) {
-    now ??= Date.now();
+    const now = Date.now();
 
     // Update batchStats.
     const batchStats: [StatsKey, string][] = [
diff --git a/test/server/lib/DocApi.ts b/test/server/lib/DocApi.ts
index ac4890c6..8b20861a 100644
--- a/test/server/lib/DocApi.ts
+++ b/test/server/lib/DocApi.ts
@@ -3534,7 +3534,6 @@ function testDocApi() {
         // Ok, we can return success now.
         probeStatus = 200;
         controller.abort();
-        now = Date.now();
         await longFinished.waitAndReset();
         // After releasing the hook, we are not 100% sure stats are updated, so we will wait a bit.
         // If we are checking stats while we are holding the hook (in the probe endpoint) it is safe
@@ -3545,10 +3544,10 @@ function testDocApi() {
         }, 1000, 200);
         assert.equal(stats[0].usage?.numWaiting, 0);
         assert.equal(stats[0].usage?.status, 'idle');
-        assert.isAbove(stats[0].usage?.updatedTime ?? 0, now);
+        assert.isAtLeast(stats[0].usage?.updatedTime ?? 0, now);
         assert.isNull(stats[0].usage?.lastErrorMessage);
         assert.isNull(stats[0].usage?.lastFailureTime);
-        assert.isAbove(stats[0].usage?.lastSuccessTime ?? 0, now);
+        assert.isAtLeast(stats[0].usage?.lastSuccessTime ?? 0, now);
         assert.equal(stats[0].usage?.lastHttpStatus, 200);
         assert.deepEqual(stats[0].usage?.lastEventBatch, {
           status: 'success',
@@ -3575,11 +3574,11 @@ function testDocApi() {
         stats = await readStats(docId);
         assert.equal(stats[0].usage?.numWaiting, 1);
         assert.equal(stats[0].usage?.status, 'retrying');
-        assert.isAbove(stats[0].usage?.updatedTime ?? 0, now);
+        assert.isAtLeast(stats[0].usage?.updatedTime ?? 0, now);
         // There was no body in the response yet.
         assert.isNull(stats[0].usage?.lastErrorMessage);
         // Now we have a failure, and the success was before.
-        assert.isAbove(stats[0].usage?.lastFailureTime ?? 0, now);
+        assert.isAtLeast(stats[0].usage?.lastFailureTime ?? 0, now);
         assert.isBelow(stats[0].usage?.lastSuccessTime ?? 0, now);
         assert.equal(stats[0].usage?.lastHttpStatus, 404);
         // Batch contains info about last attempt.
@@ -3623,8 +3622,8 @@ function testDocApi() {
         assert.equal(stats[0].usage?.status, 'idle');
         assert.equal(stats[0].usage?.lastHttpStatus, 200);
         assert.equal(stats[0].usage?.lastErrorMessage, probeMessage);
-        assert.isAbove(stats[0].usage?.lastFailureTime ?? 0, now);
-        assert.isAbove(stats[0].usage?.lastSuccessTime ?? 0, now);
+        assert.isAtLeast(stats[0].usage?.lastFailureTime ?? 0, now);
+        assert.isAtLeast(stats[0].usage?.lastSuccessTime ?? 0, now);
         assert.deepEqual(stats[0].usage?.lastEventBatch, {
           status: 'success',
           attempts: 3,