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

test(nuxt): Use sentry.server.config.ts in E2E tests #13999

Merged
merged 5 commits into from
Oct 16, 2024

Conversation

s1gr1d
Copy link
Member

@s1gr1d s1gr1d commented Oct 16, 2024

The E2E tests still used an instrument file in the public folder (which is not the way to setup the SDK anymore) because import-in-the-middle/hook.mjs was not available and threw an error. To be able to properly test the setup with sentry.server.config.ts, a function was added that copies the import-in-the-middle folder to the build output to include all files.

@s1gr1d s1gr1d self-assigned this Oct 16, 2024
@s1gr1d s1gr1d added the Package: nuxt Issues related to the Sentry Nuxt SDK label Oct 16, 2024
Copy link

codecov bot commented Oct 16, 2024

❌ 5 Tests Failed:

Tests completed Failed Passed Skipped
637 5 632 28
View the top 3 failed tests by shortest run time
tracing.server.test.ts does not send transactions for build asset folder "_nuxt"
Stack Traces | 30s run time
tracing.server.test.ts:24:1 does not send transactions for build asset folder "_nuxt"
tracing.test.ts distributed tracing › capture a distributed pageload trace
Stack Traces | 30s run time
tracing.test.ts:7:3 capture a distributed pageload trace
errors.server.test.ts server-side errors › captures api fetch error (fetched on click)
Stack Traces | 30s run time
errors.server.test.ts:5:3 captures api fetch error (fetched on click)

To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard

Copy link
Member

@andreiborza andreiborza left a comment

Choose a reason for hiding this comment

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

Great findings, glad it works now.

Comment on lines 4 to 5
/* This script copies the `import-in-the-middle` content of the E2E test project root `node_modules` to the build output `node_modules`
For some reason, some files are missing in the output (like `hook.mjs`) and this is not reproducible in external, standalone projects.
Copy link
Member

Choose a reason for hiding this comment

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

m: Could we not just use cp inside the build script in package.json instead of writing code to copy this over?

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed it, so much cleaner ✨
Thanks for the input!

Copy link
Member

@andreiborza andreiborza left a comment

Choose a reason for hiding this comment

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

Nice job!

@s1gr1d s1gr1d merged commit 8782af8 into develop Oct 16, 2024
118 of 119 checks passed
@s1gr1d s1gr1d deleted the sig/fix-nuxt-e2e-test branch October 16, 2024 15:07
billyvg pushed a commit that referenced this pull request Oct 17, 2024
The E2E tests still used an `instrument` file in the `public` folder
(which is not the way to setup the SDK anymore) because
`import-in-the-middle/hook.mjs` was not available and threw an error. To
be able to properly test the setup with `sentry.server.config.ts`, a
function was added that copies the `import-in-the-middle` folder to the
build output to include all files.
billyvg pushed a commit that referenced this pull request Oct 17, 2024
The E2E tests still used an `instrument` file in the `public` folder
(which is not the way to setup the SDK anymore) because
`import-in-the-middle/hook.mjs` was not available and threw an error. To
be able to properly test the setup with `sentry.server.config.ts`, a
function was added that copies the `import-in-the-middle` folder to the
build output to include all files.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: nuxt Issues related to the Sentry Nuxt SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants