Skip to content
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

fix(browser-integration-tests): Fix feedback addon CDN bundle integration test setup #13108

Merged
merged 1 commit into from
Jul 30, 2024

Conversation

Lms24
Copy link
Member

@Lms24 Lms24 commented Jul 30, 2024

This PR fixes a hard to catch problem in our browser integration test setup that caused our current release 8.21.0 to fail.

In #13081 we re-introduced testing against the feedback addon CDN bundles in our integration tests. This triggered a test fail on release branches but it remained undiscovered in develop and feature branches. Chain of events:

  • The addon feedback CDN bundle now correctly exports the async feedback integration
  • The async feedback integration lazy-loads the modal and screenshot integrations
  • To avoid actually lazy loading these via http requests to the CDN but to test against the locally built versions, we added a couple of request interceptors in our getLocalTestPath fixture setup function.
  • This interceptor however, would just silently fail if it didn't find the local version of the bundle [1] and forward the request.
  • So, in develop and feature branches, unknowingly actually took the bundles from our CDN.
  • On the release branch, we bump the version of the bundle we want to download from the CDN which does not exist
  • So the request obviously fails, causing the bundle not to be loaded, causing the feedback UI not to be available, finally causing the test to fail

[1] Now, why was the file not present all of a sudden? because we only linked these files locally when we tested the SDK CDN bundle which already includes feedback. Since we disabled the addon CDN tests (or didn't test them at all before?) we just didn't symlink the necessary bundles into the test directory.

So to fix this, this PR:

  • Adds the symlinks whenever we discover that feedbackIntegration is imported in a test app
  • Stops forwarding the CDN bundle request to the actual CDN but throws a hard error. We should have never forwarded here because we'd unknowingly test against already published artifacts instead of the locally built ones.

Copy link
Contributor

size-limit report 📦

Path Size
@sentry/browser 22.45 KB (0%)
@sentry/browser (incl. Tracing) 34.22 KB (0%)
@sentry/browser (incl. Tracing, Replay) 70.26 KB (0%)
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 63.59 KB (0%)
@sentry/browser (incl. Tracing, Replay with Canvas) 74.66 KB (0%)
@sentry/browser (incl. Tracing, Replay, Feedback) 87.24 KB (0%)
@sentry/browser (incl. Tracing, Replay, Feedback, metrics) 89.08 KB (0%)
@sentry/browser (incl. metrics) 26.75 KB (0%)
@sentry/browser (incl. Feedback) 39.37 KB (0%)
@sentry/browser (incl. sendFeedback) 27.06 KB (0%)
@sentry/browser (incl. FeedbackAsync) 31.7 KB (0%)
@sentry/react 25.22 KB (0%)
@sentry/react (incl. Tracing) 37.22 KB (0%)
@sentry/vue 26.6 KB (0%)
@sentry/vue (incl. Tracing) 36.06 KB (0%)
@sentry/svelte 22.58 KB (0%)
CDN Bundle 23.64 KB (0%)
CDN Bundle (incl. Tracing) 35.88 KB (0%)
CDN Bundle (incl. Tracing, Replay) 70.26 KB (0%)
CDN Bundle (incl. Tracing, Replay, Feedback) 75.53 KB (0%)
CDN Bundle - uncompressed 69.37 KB (0%)
CDN Bundle (incl. Tracing) - uncompressed 106.31 KB (0%)
CDN Bundle (incl. Tracing, Replay) - uncompressed 217.95 KB (0%)
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 230.78 KB (0%)
@sentry/nextjs (client) 37.08 KB (0%)
@sentry/sveltekit (client) 34.81 KB (0%)
@sentry/node 114.55 KB (+2.36% 🔺)
@sentry/node - without tracing 89.33 KB (-0.01% 🔽)
@sentry/aws-serverless 98.5 KB (-0.01% 🔽)

@Lms24 Lms24 requested review from mydea and lforst July 30, 2024 14:35
@Lms24 Lms24 merged commit d73a567 into develop Jul 30, 2024
112 checks passed
@Lms24 Lms24 deleted the lms/fix-feedback-integration-tests branch July 30, 2024 14:53
@Lms24 Lms24 self-assigned this Jul 30, 2024
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.

2 participants