| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
… building Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
|
LGTM! |
Sorry, something went wrong.
There was a problem hiding this comment.
I think you diagnosed the issue correctly, but this changes the test so that it no longer actually tests the intended behavior of close().
Here is the (migrated) bug report that led to this test:
The point is to test that conn.close() actually closes the underlying socket, so that a recv/select on the other end (self.cli) with a short timeout returns immediately with an empty buffer.
We might need a few more tweaks after that.
Sorry, something went wrong.
|
Something like: def testClose(self):
conn, addr = self.serv.accept()
conn.close()
# Calling close() many times should be safe.
conn.close()
conn.close()
def _testClose(self):
self.cli.connect((HOST, self.port))
read, write, err = select.select([self.cli], [], [], support.SHORT_TIMEOUT)
self.assertEqual(read, [self.cli])
self.assertEqual(self.cli.recv(1), b'')
# The other end should be closed now, so select should immediately
# return with the socket ready for reading.
read, write, err = select.select([self.cli], [], [], 0)
self.assertEqual(read, [self.cli])
self.assertEqual(self.cli.recv(1), b'') |
Sorry, something went wrong.
Thanks for the tips, I'll update the patch ASAP |
Sorry, something went wrong.
Signed-off-by: Manjusaka <me@manjusaka.me>
There was a problem hiding this comment.
This version of the PR has a few issues. The time.sleep(1.0) adds an unnecessary one second delay. The 0.1 timeout will make the test too sensitive to timing variations and lead to spurious failures on heavily loaded machines.
Please see the suggested code in my comment at: #123697 (comment)
Sorry, something went wrong.
Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
Thanks for the review. I have updated this PR. I think this version of this PR is similar to your suggested code. I add an extra step to check the socket which is generated bt accept call works fine before the close action. I'm not sure this is necessary but I think this should be better. |
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Fix #117657