From dfdb6e9ece272580f6d34a67ccae71d81b83eb95 Mon Sep 17 00:00:00 2001
From: Harrison Healey <harrisonmhealey@gmail.com>
Date: Tue, 11 Jun 2024 17:05:29 -0400
Subject: [PATCH] Improve isolation of PerformanceReporter tests to increase
 their stability (#27275)

* Improve isolation of PerformanceReporter tests to increase their stability

* Change how PerformanceReporter.sendBeacon is defined
---
 .../performance_telemetry/reporter.test.ts    | 32 +++++++++++--------
 .../utils/performance_telemetry/reporter.ts   |  7 +++-
 2 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/webapp/channels/src/utils/performance_telemetry/reporter.test.ts b/webapp/channels/src/utils/performance_telemetry/reporter.test.ts
index da3813d931..ab84e2e96d 100644
--- a/webapp/channels/src/utils/performance_telemetry/reporter.test.ts
+++ b/webapp/channels/src/utils/performance_telemetry/reporter.test.ts
@@ -17,9 +17,6 @@ import {markAndReport, measureAndReport} from '.';
 
 jest.mock('web-vitals');
 
-const sendBeacon = jest.fn().mockReturnValue(true);
-navigator.sendBeacon = sendBeacon;
-
 const siteUrl = 'http://localhost:8065';
 
 describe('PerformanceReporter', () => {
@@ -29,7 +26,7 @@ describe('PerformanceReporter', () => {
     });
 
     test('should report measurements to the server as histograms', async () => {
-        const {reporter} = newTestReporter();
+        const {reporter, sendBeacon} = newTestReporter();
         reporter.observe();
 
         expect(sendBeacon).not.toHaveBeenCalled();
@@ -77,7 +74,7 @@ describe('PerformanceReporter', () => {
     });
 
     test('should report some marks to the server as counters', async () => {
-        const {reporter} = newTestReporter();
+        const {reporter, sendBeacon} = newTestReporter();
         reporter.observe();
 
         expect(sendBeacon).not.toHaveBeenCalled();
@@ -121,7 +118,7 @@ describe('PerformanceReporter', () => {
     });
 
     test('should report longtasks to the server as counters', async () => {
-        const {reporter} = newTestReporter();
+        const {reporter, sendBeacon} = newTestReporter();
         reporter.observe();
 
         expect(sendBeacon).not.toHaveBeenCalled();
@@ -166,7 +163,7 @@ describe('PerformanceReporter', () => {
     });
 
     test('should report web vitals to the server as histograms', async () => {
-        const {reporter} = newTestReporter();
+        const {reporter, sendBeacon} = newTestReporter();
         reporter.observe();
 
         expect(sendBeacon).not.toHaveBeenCalled();
@@ -229,7 +226,7 @@ describe('PerformanceReporter', () => {
     });
 
     test('should not report anything there is no data to report', async () => {
-        const {reporter} = newTestReporter();
+        const {reporter, sendBeacon} = newTestReporter();
         reporter.observe();
 
         expect(sendBeacon).not.toHaveBeenCalled();
@@ -247,7 +244,7 @@ describe('PerformanceReporter', () => {
     });
 
     test('should not report anything if EnableClientMetrics is false', async () => {
-        const {reporter} = newTestReporter(false);
+        const {reporter, sendBeacon} = newTestReporter(false);
         reporter.observe();
 
         expect(sendBeacon).not.toHaveBeenCalled();
@@ -267,7 +264,7 @@ describe('PerformanceReporter', () => {
     });
 
     test('should not report anything if the user is not logged in', async () => {
-        const {reporter} = newTestReporter(true, false);
+        const {reporter, sendBeacon} = newTestReporter(true, false);
         reporter.observe();
 
         expect(sendBeacon).not.toHaveBeenCalled();
@@ -290,7 +287,7 @@ describe('PerformanceReporter', () => {
         setPlatform('MacIntel');
         setUserAgent('Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:124.0) Gecko/20100101 Firefox/124.0');
 
-        const {reporter} = newTestReporter();
+        const {reporter, sendBeacon} = newTestReporter();
         reporter.observe();
 
         markAndReport('reportedA');
@@ -317,7 +314,11 @@ describe('PerformanceReporter', () => {
     });
 
     test('should fall back to making a fetch request if a beacon cannot be sent', async () => {
-        const {client, reporter} = newTestReporter();
+        const {
+            client,
+            reporter,
+            sendBeacon,
+        } = newTestReporter();
         reporter.observe();
 
         sendBeacon.mockReturnValue(false);
@@ -344,6 +345,7 @@ describe('PerformanceReporter', () => {
 });
 
 class TestPerformanceReporter extends PerformanceReporter {
+    public sendBeacon: jest.Mock = jest.fn(() => true);
     public reportPeriodBase = 10;
     public reportPeriodJitter = 0;
 
@@ -371,7 +373,11 @@ function newTestReporter(telemetryEnabled = true, loggedIn = true) {
         },
     }));
 
-    return {client, reporter};
+    return {
+        client,
+        reporter,
+        sendBeacon: reporter.sendBeacon,
+    };
 }
 
 function waitForReport() {
diff --git a/webapp/channels/src/utils/performance_telemetry/reporter.ts b/webapp/channels/src/utils/performance_telemetry/reporter.ts
index 87a47067ac..6a003a5e86 100644
--- a/webapp/channels/src/utils/performance_telemetry/reporter.ts
+++ b/webapp/channels/src/utils/performance_telemetry/reporter.ts
@@ -67,6 +67,7 @@ export default class PerformanceReporter {
     private observer: PerformanceObserver;
     private reportTimeout: number | undefined;
 
+    // These values are protected instead of private so that they can be modified by unit tests
     protected reportPeriodBase = 60 * 1000;
     protected reportPeriodJitter = 15 * 1000;
 
@@ -298,13 +299,17 @@ export default class PerformanceReporter {
         const url = this.client.getClientMetricsRoute();
         const data = JSON.stringify(report);
 
-        const beaconSent = navigator.sendBeacon(url, data);
+        const beaconSent = this.sendBeacon(url, data);
 
         if (!beaconSent) {
             // The data couldn't be queued as a beacon for some reason, so fall back to sending an immediate fetch
             fetch(url, {method: 'POST', body: data});
         }
     }
+
+    protected sendBeacon(url: string | URL, data?: BodyInit | null | undefined): boolean {
+        return navigator.sendBeacon(url, data);
+    }
 }
 
 function isPerformanceLongTask(entry: PerformanceEntry): entry is PerformanceLongTaskTiming {