Skip to content

test(browser): Remove getLocalTestPath in favor of getLocalTestUrl #14331

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 9 commits into from
Nov 19, 2024

Conversation

mydea
Copy link
Member

@mydea mydea commented Nov 18, 2024

Using a proper URL-based setup in browser integration tests instead of serving from a file makes the tests more realistic, as chrome behavior can differ slightly in those scenarios.

I refactored any tests that required actual changes in a previous PR, so this PR does:

  1. Search-replace all usages of getLocalTestPath with getLocalTestUrl
  2. Remove the getLocalTestPath test fixture

@mydea mydea self-assigned this Nov 18, 2024
@mydea mydea requested a review from a team as a code owner November 18, 2024 08:41
Copy link

codecov bot commented Nov 18, 2024

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
585 1 584 29
View the top 1 failed tests by shortest run time
replay/errors/errorMode/test.ts [error-mode] should start recording and switch to session mode once an error is thrown
Stack Traces | 1.66s run time
test.ts:18:11 [error-mode] should start recording and switch to session mode once an error is thrown

To view more test analytics, go to the Test Analytics Dashboard
Got feedback? Let us know on Github

@mydea mydea force-pushed the fn/getLocalTestUrl branch from 5de5673 to 2f3ad9f Compare November 18, 2024 08:46
Copy link
Member

@chargome chargome left a comment

Choose a reason for hiding this comment

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

LGTM!

Just one thing, getLocalTestUrl always adds routes for allowing lazy loading of feedback, maybe we want to make this opt-in? Not sure if it actually has any (performance) impact on our tests.

@mydea
Copy link
Member Author

mydea commented Nov 18, 2024

LGTM!

Just one thing, getLocalTestUrl always adds routes for allowing lazy loading of feedback, maybe we want to make this opt-in? Not sure if it actually has any (performance) impact on our tests.

Hmm we could make it opt-in I suppose! Will check it out.

@mydea mydea force-pushed the fn/getLocalTestUrl branch from 2f3ad9f to 04cdcc7 Compare November 18, 2024 11:23
@mydea mydea force-pushed the fn/getLocalTestUrl branch from 04cdcc7 to 3c9e578 Compare November 19, 2024 08:06
Copy link
Contributor

github-actions bot commented Nov 19, 2024

size-limit report 📦

Path Size % Change Change
@sentry/browser 22.77 KB - -
@sentry/browser - with treeshaking flags 21.53 KB - -
@sentry/browser (incl. Tracing) 35.27 KB - -
@sentry/browser (incl. Tracing, Replay) 72 KB - -
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 62.38 KB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 76.31 KB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 89.17 KB - -
@sentry/browser (incl. Feedback) 39.93 KB - -
@sentry/browser (incl. sendFeedback) 27.42 KB - -
@sentry/browser (incl. FeedbackAsync) 32.23 KB - -
@sentry/react 25.52 KB - -
@sentry/react (incl. Tracing) 38.23 KB - -
@sentry/vue 26.92 KB - -
@sentry/vue (incl. Tracing) 37.1 KB - -
@sentry/svelte 22.91 KB - -
CDN Bundle 24.13 KB - -
CDN Bundle (incl. Tracing) 37.05 KB - -
CDN Bundle (incl. Tracing, Replay) 71.72 KB - -
CDN Bundle (incl. Tracing, Replay, Feedback) 77.07 KB - -
CDN Bundle - uncompressed 70.73 KB - -
CDN Bundle (incl. Tracing) - uncompressed 109.94 KB - -
CDN Bundle (incl. Tracing, Replay) - uncompressed 222.46 KB - -
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 235.68 KB - -
@sentry/nextjs (client) 38.35 KB - -
@sentry/sveltekit (client) 35.85 KB - -
@sentry/node 134.33 KB - -
@sentry/node - without tracing 96.2 KB - -
@sentry/aws-serverless 106.48 KB - -

View base workflow run

@mydea mydea force-pushed the fn/getLocalTestUrl branch from 1c04dbf to de26ef5 Compare November 19, 2024 09:08
@mydea mydea force-pushed the fn/getLocalTestUrl branch from de26ef5 to 8cd4133 Compare November 19, 2024 12:48
@mydea mydea merged commit e99dd09 into develop Nov 19, 2024
52 checks passed
@mydea mydea deleted the fn/getLocalTestUrl branch November 19, 2024 14:26
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.

3 participants