Skip to content

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

Merged
Lms24 merged 10 commits into
developfrom
lms/replay-pw-fix-flake-flushtime
Feb 28, 2023
Merged

test(replay): Fix flaky flush test#7268
Lms24 merged 10 commits into
developfrom
lms/replay-pw-fix-flake-flushtime

Conversation

@Lms24

@Lms24Lms24 commented Feb 23, 2023

Copy link
Copy Markdown
Member

This PR fixes 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.

Note: I tested that flakes were gone by running this test 75x per CI job for multiple times and didn't see this test failing anymore.

closes#7266

@Lms24Lms24 changed the title test(replay): Potentially fix flaky flush testtest(replay): Potentially fix flakey flush testFeb 23, 2023
@Lms24Lms24 changed the title test(replay): Potentially fix flakey flush testtest(replay): Potentially fix flaky flush testFeb 23, 2023
@github-actions

github-actionsBot commented Feb 23, 2023

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
@sentry/browser - ES5 CDN Bundle (gzipped + minified)20.11 KB (0%)
@sentry/browser - ES5 CDN Bundle (minified)62.49 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped + minified)18.74 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified)55.5 KB (0%)
@sentry/browser - Webpack (gzipped + minified)20.48 KB (-0.01% 🔽)
@sentry/browser - Webpack (minified)66.94 KB (0%)
@sentry/react - Webpack (gzipped + minified)20.51 KB (0%)
@sentry/nextjs Client - Webpack (gzipped + minified)48.04 KB (-0.01% 🔽)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified)27.04 KB (0%)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified)25.29 KB (-0.01% 🔽)
@sentry/replay ES6 CDN Bundle (gzipped + minified)42.86 KB (+0.2% 🔺)
@sentry/replay - Webpack (gzipped + minified)36.94 KB (+0.32% 🔺)
@sentry/browser + @sentry/tracing + @sentry/replay - ES6 CDN Bundle (gzipped + minified)60.45 KB (+0.18% 🔺)
@sentry/browser + @sentry/replay - ES6 CDN Bundle (gzipped + minified)53.99 KB (+0.17% 🔺)

@Lms24

Copy link
Copy Markdown
MemberAuthor

I'm actually not sure if there isn't some sort of problem with our flushing implementation rather than with test flakiness. We set maxFlushDelay to 500ms in init.js but very frequently, times between flushes seem to be closer to 1000ms. Sure, I guess there could be some wiggle room because of the event loop when the max timeout of the debounced flush resolves but almost double the actual time seems a little strange to me.

@Lms24

Copy link
Copy Markdown
MemberAuthor

Hmm we're even getting outliers with delays >2s: https://github.com/getsentry/sentry-javascript/actions/runs/4254985922/jobs/7402098735#step:11:264

It might just be that the runners are so slow that this just keeps on happening despite the maxdelay we set 🤔

@Lms24
Lms24force-pushed the lms/replay-pw-fix-flake-flushtime branch from 90d5f6b to c567adeCompareFebruary 28, 2023 09:04
@Lms24Lms24 changed the title test(replay): Potentially fix flaky flush testtest(replay): Fix flaky flush testFeb 28, 2023
@Lms24
Lms24 marked this pull request as ready for review February 28, 2023 10:39
@Lms24
Lms24 requested a review from mydeaFebruary 28, 2023 10:39
@Lms24
Lms24 merged commit 4ae08fe into developFeb 28, 2023
@Lms24
Lms24 deleted the lms/replay-pw-fix-flake-flushtime branch February 28, 2023 14:24
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Flaky CI]: Playwright replay flushing test

2 participants

@Lms24@mydea