Skip to content

fix(solidstart): Use production server for e2e tests #14033

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 1 commit into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 5 additions & 13 deletions dev-packages/e2e-tests/test-applications/solidstart/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,9 @@
"version": "0.0.0",
"scripts": {
"clean": "pnpx rimraf node_modules pnpm-lock.yaml .vinxi .output",
"clean:build": "pnpx rimraf .vinxi .output",
"dev": "NODE_OPTIONS='--import ./src/instrument.server.mjs' vinxi dev",
"build": "vinxi build",
"//": [
"We are using `vinxi dev` to start the server because `vinxi start` is experimental and ",
"doesn't correctly resolve modules for @sentry/solidstart/solidrouter.",
"This is currently not an issue outside of our repo. See: https://github.com/nksaraf/vinxi/issues/177",
"We run the build command to ensure building succeeds. However, keeping",
"build output around slows down the vite dev server when using `@sentry/vite-plugin` so we clear it out",
"before actually running the tests.",
"Cleaning the build output should be removed once we can use `vinxi start`."
],
"preview": "pnpm clean:build && HOST=localhost PORT=3030 NODE_OPTIONS='--import ./src/instrument.server.mjs' vinxi dev",
"start": "HOST=localhost PORT=3030 NODE_OPTIONS='--import ./src/instrument.server.mjs' vinxi start",
Copy link
Member

Choose a reason for hiding this comment

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

we don't need the start command anymore?

Copy link
Member

Choose a reason for hiding this comment

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

I see that the playwright config has pnpm preview as a start command, so this is probably not necessary.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, what @s1gr1d said. I had the start in there just for local testing but it's not necessary anymore.

"build": "vinxi build && sh ./post_build.sh",
"preview": "HOST=localhost PORT=3030 NODE_OPTIONS='--import ./src/instrument.server.mjs' vinxi start",
"test:prod": "TEST_ENV=production playwright test",
"test:build": "pnpm install && npx playwright install && pnpm build",
"test:assert": "pnpm test:prod"
Expand All @@ -41,5 +30,8 @@
"vite": "^5.2.8",
"vite-plugin-solid": "^2.10.2",
"vitest": "^1.5.0"
},
"overrides": {
"@vercel/nft": "0.27.4"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# TODO: Investigate the need for this script periodically and remove once these modules are correctly resolved.

# This script copies `import-in-the-middle` and `@sentry/solidstart` from the E2E test project root `node_modules`
# to the nitro server build output `node_modules` as these are not properly resolved in our yarn workspace/pnpm
# e2e structure. Some files like `hook.mjs` and `@sentry/solidstart/solidrouter.server.js` are missing. This is
# not reproducible in an external project (when pinning `@vercel/nft` to `v0.27.0` and higher).
cp -r node_modules/.pnpm/import-in-the-middle@1.*/node_modules/import-in-the-middle .output/server/node_modules
cp -rL node_modules/@sentry/solidstart .output/server/node_modules/@sentry
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ test('captures an exception', async ({ page }) => {
);
});

await page.goto('/error-boundary');
await page.goto('/error-boundary');
await page.locator('#caughtErrorBtn').click();
const errorEvent = await errorEventPromise;
Expand Down Expand Up @@ -41,7 +40,6 @@ test('captures a second exception after resetting the boundary', async ({ page }
);
});

await page.goto('/error-boundary');
await page.goto('/error-boundary');
await page.locator('#caughtErrorBtn').click();
const firstErrorEvent = await firstErrorEventPromise;
Expand Down
Loading