fix: do not end server process in CI #3659
Merged
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.
Description
This fixes a problem running the vite server in docker and github actions (ci) as documented in material-svelte/vite-web-test-runner-plugin#6. It took me a long time to figure out why my tests were completing just fine locally but exiting early with a
0
exit code in CI without running any tests. I eventually traced it down to theprocess.exit(0)
added in #1857.With the changes here, I am able to run tests successfully both locally, as well as in CI.
I would love if @nullpilot, @jonathanstiansen, and @1player could take a look and try this out to see if it meets their needs as well.
Additional context
Docker (and by extension github ci) do not always run as an interactive tty. Create-react-app had an issue when they tried to add a shutdown based on detecting an interactive terminal, and addressed it by only checking for a CI environment variable instead (see facebook/create-react-app#8845 and the referenced issue, facebook/create-react-app#8688).
As an aside, note that webpack-dev-server listens to both
SIGINT
andSIGTERM
for docker support (webpack/webpack-dev-server#787), butSIGINT
was factored out of vite and I'm not quite sure why based on the commit.I'm also not sure how to write a test for this particular feature, but I did check that the server is still torn down when I run
vite
locally and then kill it withctrl+c
. I think it will also be interesting to see if CI completes successfully, since that was a problem the last time thatprocess.stdin.resume()
was added, which I've added here to match CRA and to allow the stream to drain correctly.What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).