-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
fix: server.address before listen, chdir in test, basic cli test #5059
Conversation
not cool windows, not cool... looks like If anyone has ideas how to actually kill a process on windows, please chime in. |
this is going to have to wait until #5060 has been merged as it's likely this needs to be adapted to be usable with pnpm. |
force-pushed to update to pnpm |
I tried and failed to kill the process in windows. Maybe we should avoid this test in windows until we figure this out? At least we can check that the cli is working correctly in other environments (so missing imports at least will not pass). |
on windows it only works with taskkill see https://github.com/pkrumins/node-tree-kill/blob/cb478381547107f5c53362668533f634beff7e6e/index.js#L29 i'll add that later tonight. might also look into group killing for unix to be extra safe. |
i'm at a loss here, the tests run fine locally, but fail on github ci with a strange error that the server isn't starting stdout:
stderr:
|
…ands; add promise race helper
…skkill for windows only
…g to log network details
…e actual address; restore original for startup duration
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.
🚀🥳
Thanks for not giving up 👏
this is now ready for review. originally i only wanted to add the testsuite itself, but it uncovered 2 problems along the way, both of which are fixed here too.
|
Merging this one to be able to do a beta.3 release |
Description
today vite-2.6.0-beta.1 release broke cli on node < 16 and it went unnoticed because the current tests never use the cli
This PR adds a new cli playground that uses a custom
serve.js
implementation that runsvite
,vite build
andvite preview
in a subprocess.The tests themsselves are very basic, just checking that the page comes up with expected content. No tests for arguments or anything.
Additional context
killing of the subprocess is a pain, in vite-plugin-svelte i used tree-kill for that because there's another indirection. Here it relies on execa's kill.
There are relatively short timeouts on process handling to avoid running out of the 10s jest limit, hopefully this works on CI too, otherwise it might need a larger timeout.
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).