-
Notifications
You must be signed in to change notification settings - Fork 619
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
pytest: kill node process through Popen class and then wait for it #5578
Conversation
This fixes the following warning when trying to run tests with unittest fixture: sys:1: ResourceWarning: unclosed file <_io.TextIOWrapper name='/home/mpn/.near/test0/stdout' mode='a' encoding='UTF-8'> sys:1: ResourceWarning: unclosed file <_io.TextIOWrapper name='/home/mpn/.near/test0/stderr' mode='a' encoding='UTF-8'> Issue: near#3186 Issue: near#4618
Note: This is on top of #5576 so look at the second commit only. |
pytest/lib/cluster.py
Outdated
@@ -398,24 +398,25 @@ def start(self, *, boot_node: BootNode = None, skip_starting_proxy=False): | |||
except: | |||
logger.error( | |||
'=== failed to start node, rpc does not ready in 10 seconds') | |||
self.stdout.close() | |||
self.stderr.close() | |||
if os.environ.get('BUILDKITE'): |
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.
please comment what is the special thing that we need to do for 'BUILDKITE'
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.
With the other PR merged this PR no longer touches this piece of code so documenting this here feels out of place for this PR.
@mm-near, PTAL |
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.
LGTM
This fixes the following warning:
which is caused by LocalNode not waiting for the thread to terminate
after it’s killed.
Issue: #3186
Issue: #4618