-
Notifications
You must be signed in to change notification settings - Fork 36
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: always kill warnet server #141
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
07cd277
to
d81d56b
Compare
d81d56b
to
b77148d
Compare
@pinheadmz FYI all tests passing now (just restarted to pass). I also covered both startup CLI/RPC commands, i.e. |
Currently, if a test fails warnet server might not be properly killed and so remain running. This is annoying for iteration; if the test fails we want everything gone ready to run again. Also only try to bring the network down if we first brought it up to avoid spurious error messages in the logs.
b77148d
to
5c19770
Compare
# kill the server process by its PID if it still survives | ||
if self.server: | ||
print(f"killing server with pid {self.server.pid}") | ||
os.killpg(os.getpgid(self.server.pid), signal.SIGTERM) |
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 looks to me like the most useful part of this PR, I dont really understand the need for the other changes. Why couldn't this emergency termination just be part of the original cleanup()
in the exception handler (i.e. if warcli stop
fails?)
Currently, if a test fails warnet server might not be properly killed and so remain running.
This is annoying for iteration; if the test fails we want everything gone ready to run again.
Also only try to bring the network down if we first brought it up.