test(replay): Fix flaky flush test (#7268)

Lms24 · web-flow · commit 4ae08fee7c2e · 2023-02-28T15:24:21.000+01:00
Fix the flakiness of our replay event flushing test by slightly changing the test requirements/expectations:

* Disabled compression to avoid potential timing problems
* Instead of asserting that a flush happens within a certain time frame from the last flush, we ease up this expectation so that we only expect _that_ another flush happens after the previous one, given that click events happened in between. 
* Instead of forcing a flush by triggering a visibility change (like we do a lot in other tests to avoid flakes), this test focuses on the max delay of flushing, thereby ensuring that our flushes are not debounced to eternity.
diff --git a/packages/integration-tests/suites/replay/flushing/init.js b/packages/integration-tests/suites/replay/flushing/init.js
@@ -4,6 +4,7 @@ window.Sentry = Sentry;
 window.Replay = new Sentry.Replay({
   flushMinDelay: 500,
   flushMaxDelay: 500,
+  useCompression: false,
 });
 
 Sentry.init({
diff --git a/packages/integration-tests/suites/replay/flushing/template.html b/packages/integration-tests/suites/replay/flushing/template.html
@@ -4,6 +4,6 @@
     <meta charset="utf-8" />
   </head>
   <body>
-    <button id="go-background">New Tab</button>
+    <button id="something">Do something</button>
   </body>
 </html>
diff --git a/packages/integration-tests/suites/replay/flushing/test.ts b/packages/integration-tests/suites/replay/flushing/test.ts
@@ -4,10 +4,15 @@ import { sentryTest } from '../../../utils/fixtures';
 import { getExpectedReplayEvent } from '../../../utils/replayEventTemplates';
 import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../utils/replayHelpers';
 
-// Sync this with init.js - not we take seconds here instead of ms
-const FLUSH_DELAY_SECONDS = 0.5;
-
-sentryTest('replay recording flushes every 5s', async ({ getLocalTestPath, page }) => {
+/*
+ * In this test we're explicitly not forcing a flush by triggering a visibility change.
+ * Instead, we want to verify that the `flushMaxDelay` works in the sense that eventually
+ * a flush is triggered if some events are in the buffer.
+ * Note: Due to timing problems and inconsistencies in Playwright/CI, we can't reliably
+ * assert on the flush timestamps. Therefore we only assert that events were eventually
+ * sent (i.e. flushed).
+ */
+sentryTest('replay events are flushed after max flush delay was reached', async ({ getLocalTestPath, page }) => {
   if (shouldSkipReplayTest()) {
     sentryTest.skip();
   }
@@ -30,17 +35,18 @@ sentryTest('replay recording flushes every 5s', async ({ getLocalTestPath, page
   const replayEvent0 = getReplayEvent(await reqPromise0);
   expect(replayEvent0).toEqual(getExpectedReplayEvent());
 
-  // trigger mouse click
-  void page.click('#go-background');
+  // trigger one mouse click
+  void page.click('#something');
 
+  // this must eventually lead to a flush after the max delay was reached
   const replayEvent1 = getReplayEvent(await reqPromise1);
   expect(replayEvent1).toEqual(getExpectedReplayEvent({ replay_start_timestamp: undefined, segment_id: 1, urls: [] }));
 
-  // trigger mouse click every 100ms, it should still flush after 5s even if clicks are ongoing
-  for (let i = 0; i < 70; i++) {
+  // trigger mouse click every 100ms, it should still flush after the max delay even if clicks are ongoing
+  for (let i = 0; i < 700; i++) {
     setTimeout(async () => {
       try {
-        await page.click('#go-background');
+        await page.click('#something');
       } catch {
         // ignore errors here, we don't care if the page is closed
       }
@@ -49,14 +55,4 @@ sentryTest('replay recording flushes every 5s', async ({ getLocalTestPath, page
 
   const replayEvent2 = getReplayEvent(await reqPromise2);
   expect(replayEvent2).toEqual(getExpectedReplayEvent({ replay_start_timestamp: undefined, segment_id: 2, urls: [] }));
-
-  // Ensure time diff is about 500ms between each event
-  const diff1 = replayEvent1.timestamp! - replayEvent0.timestamp!;
-  const diff2 = replayEvent2.timestamp! - replayEvent1.timestamp!;
-
-  // We want to check that the diff is between 0.1 and 0.9 seconds, to accomodate for some wiggle room
-  expect(diff1).toBeLessThan(FLUSH_DELAY_SECONDS + 0.4);
-  expect(diff1).toBeGreaterThanOrEqual(FLUSH_DELAY_SECONDS - 0.4);
-  expect(diff2).toBeLessThan(FLUSH_DELAY_SECONDS + 0.4);
-  expect(diff2).toBeGreaterThanOrEqual(FLUSH_DELAY_SECONDS - 0.4);
 });
