-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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(environment): add environment playground #16299
test(environment): add environment playground #16299
Conversation
Run & review this pull request in StackBlitz Codeflow. |
}, | ||
}, | ||
|
||
// [feedback] should preview automatically pick up environments.client.build.outDir? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We didn't work yet on the preview side, but I was thinking we should have a PreviewEnvironment
and make preview work for the whole app in the same way that we are now letting vite build
build the whole app. This is something that was requested by framework authors before and we have been slowly making Preview work closer to the dev server. I think vite should work at the app level in all commands.
This is great! I think we should merge it as it will be really helpful. But first, I think it is worth refactoring the test to use our regular e2e test infra. |
Thanks for the review! Yeah, I'll replace playwright with vitest e2e. For the other PR #16301, I just removed the playground part. |
I rewrote it in vitest e2e, but maybe this might be a little unusual setup. A few ideas I have is:
To me, this approach feels right as e2e, but probably this is a bit opinionated. Please let me know if it should be further aligned with existing e2e tests. Also it looks like typecheck is failing on CI, but not sure what's wrong. When I run $ pnpm run typecheck
> @vitejs/vite-monorepo@ typecheck /home/hiroshi/code/others/vite
> tsc -p scripts --noEmit && pnpm -r --parallel run typecheck
Scope: 212 of 213 workspace projects
packages/create-vite typecheck$ tsc --noEmit
playground typecheck$ tsc --noEmit
packages/vite typecheck$ tsc --noEmit
packages/create-vite typecheck: Done
playground typecheck: Done
packages/vite typecheck: Done |
The setup looks good to me, but I think we should align with the rest. And we can discuss later about why you think this is better and maybe move all the playgrounds to implement some of these ideas. Except if there is a very good reason that prevents you to aligning. |
Actually The reason why I like this setup is it tests the same layer as what users would use from cli. But, non-programmatic usage cannot directly assert things only accessible programmatically like Should I try programmatic usage of vite builder? |
Yes, I think programatic usage of vite builder would be good here. That is a good test too. |
OK, actually it was simpler than how I struggled with custom |
package.json
Outdated
@@ -42,6 +42,7 @@ | |||
"devDependencies": { | |||
"@babel/types": "^7.24.0", | |||
"@eslint-types/typescript-eslint": "^7.2.0", | |||
"@playwright/test": "^1.42.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can remove this one now, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be a separate discussion too, but I think we could consider using expect
from @playwright/test
where it's appropriate since it has some neat feature to write robust assertions.
For now, I'll revert this back too. Let me check.
Awesome! Thanks for adapting the test 🙏🏼 |
@@ -1,7 +1,7 @@ | |||
import { resolve } from 'node:path' | |||
import { defineConfig } from 'vitest/config' | |||
|
|||
const timeout = process.env.CI ? 50000 : 30000 | |||
const timeout = process.env.PWDEBUG ? Infinity : process.env.CI ? 50000 : 30000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also added this to allow using playwright inspector without timeout.
For example, running this command will break at the first page.goto(..)
and you can step debug and record with playwright browser window.
PWDEBUG=1 pnpm test-build playground/environment-react-ssr
This is great! |
Description
I added a minimal ssr environment example including my normal e2e testing setup.
Probably it's not ideal to have react specific playground (even though it doesn't use react plugin) and also playwright based testing, but I thought it might help to have this while iterating on environment PR.
This also includes a build fix from PR #16298.