Skip to content

test(replay): Fix flaky flush test #7268

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Feb 28, 2023
Merged

test(replay): Fix flaky flush test #7268

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
0c16c1e
test(replay): Potentially fix flaky flush test
Lms24 Feb 23, 2023
896f1bb
loosen time margin and ease up click events
Lms24 Feb 23, 2023
ae30188
click more often
Lms24 Feb 23, 2023
727c05f
click less often
Lms24 Feb 23, 2023
a381cb9
only click once 🥲
Lms24 Feb 24, 2023
436a4c9
avoid going to background
Lms24 Feb 24, 2023
2bba4b9
ease up time expectation
Lms24 Feb 27, 2023
3e74458
don't compression, change test expectation
Lms24 Feb 28, 2023
c567ade
add comment
Lms24 Feb 28, 2023
2ad89ab
cleanup
Lms24 Feb 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/integration-tests/suites/replay/flushing/init.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ window.Sentry = Sentry;
window.Replay = new Sentry.Replay({
flushMinDelay: 500,
flushMaxDelay: 500,
useCompression: false,
});

Sentry.init({
Expand Down
2 changes: 1 addition & 1 deletion packages/integration-tests/suites/replay/flushing/template.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
34 changes: 15 additions & 19 deletions packages/integration-tests/suites/replay/flushing/test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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
}
Expand All @@ -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);
});