bpo-33099: Fix test_poplib hangs after error#6428
Conversation
This fixes resource leaks in the test and reveals real errors.
tiran
left a comment
tiran
left a comment
There was a problem hiding this comment.
Please convert try / bare except to try/finally and remove the print_exc line. The same fix should be applied to other tests that use asyncore (ftplib, smtplib, smtpd?)
| self.client = poplib.POP3(self.server.host, self.server.port, timeout=3) | ||
| try: | ||
| self.client = poplib.POP3(self.server.host, self.server.port, timeout=3) | ||
| except: |
There was a problem hiding this comment.
When setUp() succeeded, tearDown() do it.
So this should be except:, not finally:.
| self.client = poplib.POP3_SSL(self.server.host, self.server.port) | ||
| try: | ||
| self.client = poplib.POP3_SSL(self.server.host, self.server.port) | ||
| except: |
| try: | ||
| self.client = poplib.POP3(self.server.host, self.server.port, timeout=3) | ||
| self.client.stls() | ||
| except: |
| try: | ||
| self.client = poplib.POP3(self.server.host, self.server.port, timeout=3) | ||
| except: | ||
| traceback.print_exc() |
|
When you're done making the requested changes, leave the comment: |
| try: | ||
| self.client = poplib.POP3(self.server.host, self.server.port, timeout=3) | ||
| except: | ||
| self.server.stop() |
There was a problem hiding this comment.
Can self.server.stop() fail? Is it worth to rewrite the code as:
try:
self.server.stop()
finally:
self.server = None
raiseor
server = self.server
self.server = None
server.stop()
raise?
There was a problem hiding this comment.
I don't think we shouldn't complicate test code for non-real errors.
It's not late to fix it after the possibility become real.
|
"I have made the requested changes; please review again." @tiran I added same cleanup to test_os and test_ftplib. |
|
Thanks for making the requested changes! @tiran: please review the changes made to this pull request. |
|
Same fix is commited via #6865. |
5 participants
This fixes resource leaks in the test and reveals real errors.
https://bugs.python.org/issue33099