Skip to content
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

Merged
merged 7 commits into from
Dec 7, 2021

Conversation

mina86
Copy link
Contributor

@mina86 mina86 commented Dec 2, 2021

This fixes the following warning:

../usr/lib/python3.9/subprocess.py:1052: ResourceWarning: subprocess 91224 is still running
  _warn("subprocess %s is still running" % self.pid,

which is caused by LocalNode not waiting for the thread to terminate
after it’s killed.

Issue: #3186
Issue: #4618

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
This fixes the following warning:

    ../usr/lib/python3.9/subprocess.py:1052: ResourceWarning: subprocess 91224 is still running
      _warn("subprocess %s is still running" % self.pid,

which is caused by LocalNode not waiting for the thread to terminate
after it’s killed.

Issue: near#3186
Issue: near#4618
@mina86 mina86 added A-testing Area: Unit testing / integration testing A-CI Area: Continuous Integration T-node Team: issues relevant to the node experience team labels Dec 2, 2021
@mina86 mina86 requested a review from mm-near December 2, 2021 01:33
@mina86
Copy link
Contributor Author

mina86 commented Dec 2, 2021

Note: This is on top of #5576 so look at the second commit only.

pytest/lib/cluster.py Outdated Show resolved Hide resolved
@@ -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'):
Copy link
Contributor

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'

Copy link
Contributor Author

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.

pytest/lib/cluster.py Show resolved Hide resolved
@pmnoxx pmnoxx assigned pmnoxx and unassigned pmnoxx Dec 3, 2021
@mina86
Copy link
Contributor Author

mina86 commented Dec 6, 2021

@mm-near, PTAL

Copy link
Contributor

@pmnoxx pmnoxx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@near-bulldozer near-bulldozer bot merged commit 7bf068a into near:master Dec 7, 2021
@mina86 mina86 deleted the pyt-2 branch December 7, 2021 19:32
@gmilescu gmilescu added the Node Node team label Oct 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CI Area: Continuous Integration A-testing Area: Unit testing / integration testing Node Node team T-node Team: issues relevant to the node experience team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants