Skip to content

test(replay): Fix flaky xhr/fetch request tests #7270

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 2 commits into from
Feb 28, 2023

Conversation

Lms24
Copy link
Member

@Lms24 Lms24 commented Feb 23, 2023

As reported in #7267, a newly added replay test (#7044) for checking for the presence of XHR/fetch request spans was flaky on CI. I pinned this down to two reasons which this PR fixes:

  1. We didn't explicitly await for the fetch/xhr requests to finish before waiting for the replay event which we expected to contain the span. With this PR we now do wait that the browser actually received the response.
  2. The tests were only flaky on Firefox. After some research (this seems to be somewhat of a known problem in some situations) I decided to simply skip for FF. I think this is reasonably pragmatic as we're still testing on Chromium and Webkit.

Note: I ran these tests multiple times for 150 times per CI job and haven't observed flakes anymore.

closes #7267

@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.01% 🔺)
@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 Lms24 force-pushed the lms/replay-pw-fix-flake-requests branch 2 times, most recently from c6bd979 to 91fe0dd Compare February 27, 2023 13:46
change order of actions and awaiting requests

use Promise.all in xhr test

reduce flush mindelay, increase maxdelay

explicitly await request response

simplify waitForResponse

skip tests on firefox

cleanup test
@Lms24 Lms24 force-pushed the lms/replay-pw-fix-flake-requests branch from 6af894b to b10e550 Compare February 28, 2023 09:08
@Lms24 Lms24 marked this pull request as ready for review February 28, 2023 09:16
@Lms24 Lms24 requested a review from mydea February 28, 2023 09:16
flushMinDelay: 500,
flushMaxDelay: 500,
useCompression: true,
flushMinDelay: 200,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we change these on purpose? 🤔 IMHO it would be good to leave this at the same value to better replicate the regular "default" experience, wdyt?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh I think this is just a leftover from experimenting. Will change, thanks!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I'll test this with another 150x run just to be sure 😅

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, still seems to work 👍

@Lms24 Lms24 changed the title test(replay): Potentially fix flaky xhr/fetch request tests test(replay): Fix flaky xhr/fetch request tests Feb 28, 2023
@Lms24 Lms24 force-pushed the lms/replay-pw-fix-flake-requests branch from 28ec9fc to 44468e0 Compare February 28, 2023 09:58
@Lms24 Lms24 requested a review from mydea February 28, 2023 09:58
@Lms24 Lms24 merged commit 67b0684 into develop Feb 28, 2023
@Lms24 Lms24 deleted the lms/replay-pw-fix-flake-requests branch February 28, 2023 10:32
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 fetch/xhr breadcrumbs test
2 participants