-
Notifications
You must be signed in to change notification settings - Fork 368
[CLI] Fix run-cli leak which was revealed by repeated runCLI() calls during test #2888
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
Conversation
|
It looks like there are a couple of test timeouts and then a WP URL assertion failure due to treating a WP RC as the "latest" version. Fix here: |
Actually, there is another real failure due to a broken check for the |
|
There's a timeout failure so I'll err on the side of cautious and won't merge yet. |
Good instinct, @adamziel. :) There is indeed a problem with cleanup with the |
|
Also, the test-built-npm-packages test appears to be running indefinitely. Maybe it is related. Will see if there is an obvious fix there too. |
eb3da08 to
8221c53
Compare
|
The test failures were due to trying to double-dispose of the initial worker and Playground. That is fixed now, and the tests are passing now. Let's merge. |
Motivation for the change, related issues
Prior to this PR, the initial Playground CLI worker was not being cleaned up when Playground CLI was disposed.
Implementation details
This PR:
servercommand.Testing Instructions (or ideally a Blueprint)