test: Run E2E tests in isolated tmp directory #16783
Open
+811
−695
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
size-limit report 📦
|
mydea
force-pushed
the
fn/e2e-test-isolated
branch
from
12cebf0 to
a6a6c3a
Compare
mydea
added a commit
that referenced
this pull request
Jul 2, 2025
) Not sure why we even have this, but this somehow breaks cloudflare-pages E2E tests in some scenarios. It also seems simply unnecessary - or is there a reason we have this? 🤔 Extracted this out from #16783
mydea
force-pushed
the
fn/e2e-test-isolated
branch
from
a6a6c3a to
5cbdd25
Compare
mydea
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
s1gr1d
approved these changes
Jul 2, 2025
| @@ -17,14 +17,23 @@ | |||
| }, | |||
| "dependencies": { | |||
| "@sentry/nuxt": "latest || *", | |||
| "nuxt": "3.7.0" | |||
| "@sentry/core": "latest || *", | |||
There was a problem hiding this comment.
This is only installed because semantic attributes are imported from @sentry/core in the tests -> could it be that those are also exported from @sentry/nuxt?
andreiborza
approved these changes
Jul 2, 2025
| @@ -63,7 +63,7 @@ | |||
| "devDependencies": { | |||
| "@cloudflare/workers-types": "4.20250620.0", | |||
| "@types/node": "^18.19.1", | |||
| "wrangler": "^3.67.1" | |||
| "wrangler": "4.22.0" | |||
There was a problem hiding this comment.
Hard pinning is fine, we'll revisit soon anyway
mydea
added a commit
that referenced
this pull request
Jul 3, 2025
Noticed #16783 in the nuxt-3-min test, that the plugins are not consistently run apparently. In that test, the client integrations plugin was run before the client config plugin, leading to the client not existing yet, and thus not adding the browser tracing integration. This PR changes this to ensure we have a consistent order of these plugins.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
mydea
added a commit
that referenced
this pull request
Jul 3, 2025
We used to rely on a somewhat complex heuristic to determine if a router change is a pageload or not. This somehow did not work anymore here: #16783 in nuxt-3-min. Likely some vue router difference... However, I think this can be simplified anyhow, by just checking if we have an active pageload span. That seems to work reliably enough.
mydea
added a commit
that referenced
this pull request
Jul 3, 2025
We used to rely on a somewhat complex heuristic to determine if a router change is a pageload or not. This somehow did not work anymore here: #16783 in nuxt-3-min. Likely some vue router difference... However, I think this can be simplified anyhow, by just checking if we have an active pageload span. That seems to work reliably enough.
mydea
added a commit
that referenced
this pull request
Jul 3, 2025
Noticed #16783 in the nuxt-3-min test, that the plugins are not consistently run apparently. In that test, the client integrations plugin was run before the client config plugin, leading to the client not existing yet, and thus not adding the browser tracing integration. This PR changes this to ensure we have a consistent order of these plugins.
mydea
force-pushed
the
fn/e2e-test-isolated
branch
from
c059725 to
efd9b27
Compare
mydea
added a commit
that referenced
this pull request
Jul 3, 2025
Noticed #16783 in the nuxt-3-min test, that the plugins are not consistently run apparently. In that test, the client integrations plugin was run before the client config plugin, leading to the client not existing yet, and thus not adding the browser tracing integration. This PR changes this to ensure we have a consistent order of these plugins.
mydea
force-pushed
the
fn/e2e-test-isolated
branch
from
efd9b27 to
65a6c9f
Compare
|
Also fixes #16796 |
andreiborza
approved these changes
Jul 3, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR updates our E2E test runner to run the apps from an isolated tmp directory, instead of running them inside the monorepo.
Depends on:
The reason to do this is that running them inside the monorepo leads to slightly different behavior, as dependencies can be looked up from parent node_modules folders. This leads to behavior that differs from actual standalone apps.
Now, the whole app is moved into a folder in the system tmp directory. The package.json is adjusted to make it work there (e.g. rewriting volta
extendsfile paths etc), then normally run from there.Some things had to be changed/fixed to make tests work here properly:
@sentry/corein tests but did not have it as dependency.@sentry/corein tests