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.

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

Conversation

Lms24
Copy link
Member

@Lms24 Lms24 commented Feb 23, 2023

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

@Lms24 Lms24 changed the title test(replay): Potentially fix flaky flush test test(replay): Potentially fix flakey flush test Feb 23, 2023
@Lms24 Lms24 changed the title test(replay): Potentially fix flakey flush test test(replay): Potentially fix flaky flush test Feb 23, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Feb 23, 2023

size-limit report 📦

Path Size
@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
Member Author

Lms24 commented Feb 23, 2023

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
Member Author

Lms24 commented Feb 23, 2023

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 Lms24 force-pushed the lms/replay-pw-fix-flake-flushtime branch from 90d5f6b to c567ade Compare February 28, 2023 09:04
@Lms24 Lms24 changed the title test(replay): Potentially fix flaky flush test test(replay): Fix flaky flush test Feb 28, 2023
@Lms24 Lms24 marked this pull request as ready for review February 28, 2023 10:39
@Lms24 Lms24 requested a review from mydea February 28, 2023 10:39
@Lms24 Lms24 merged commit 4ae08fe into develop Feb 28, 2023
@Lms24 Lms24 deleted the lms/replay-pw-fix-flake-flushtime branch February 28, 2023 14:24
Sign up for free to 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