Roundup Robot added the comment:
New changeset c3e7a670dda2 by Victor Stinner in branch '3.4':
Issue #22351: Fix test_nntplib if the ssl module is missing
https://hg.python.org/cpython/rev/c3e7a670dda2
--
___
Python tracker rep...@bugs.python.org
Serhiy Storchaka added the comment:
Good catch. But may be better to report MockSslTests as skipped instead of
silently omit them.
--
Added file: http://bugs.python.org/file38814/issue22351_skip_MockSslTests.patch
___
Python tracker
Serhiy Storchaka added the comment:
See the patch.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22351
___
___
Python-bugs-list mailing list
STINNER Victor added the comment:
Serhiy’s patch looks like it should do the trick. Just get rid of the “self”
parameter to make it clearer,
+class MockSslTests(MockSocketTests):
+def nntp_class(self, *pos, **kw):
Hum, you should use the @staticmethod decorator here.
--
Roundup Robot added the comment:
New changeset 7a91363f31e1 by Serhiy Storchaka in branch '3.4':
Issue #22351. MockSslTests tests in test_nntplib now are reported if skipped.
https://hg.python.org/cpython/rev/7a91363f31e1
New changeset c935c1e1d001 by Serhiy Storchaka in branch 'default':
Issue
STINNER Victor added the comment:
Except of the self parameter/@staticmethod and the unrelated SAX changes,
issue22351_skip_MockSslTests.patch looks good to me.
--
resolution: fixed -
status: closed - open
___
Python tracker rep...@bugs.python.org
Martin Panter added the comment:
Serhiy’s patch looks like it should do the trick. Just get rid of the “self”
parameter to make it clearer, and watch out for the stray XML changes!
--
___
Python tracker rep...@bugs.python.org
Serhiy Storchaka added the comment:
Agree, factory method can be staticmethod.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22351
___
___
Serhiy Storchaka added the comment:
Now you have used time machine in reverse mode.
--
resolution: - fixed
status: open - closed
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22351
___
Martin Panter added the comment:
Sorry, that self parameter is okay. I was just confused because it now means
something different :S
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22351
STINNER Victor added the comment:
Good catch. But may be better to report MockSslTests as skipped instead of
silently omit them.
I don't know how to do that.
--
nosy: +haypo
___
Python tracker rep...@bugs.python.org
Changes by Serhiy Storchaka storch...@gmail.com:
--
resolution: - fixed
stage: commit review - resolved
status: open - closed
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22351
___
Serhiy Storchaka added the comment:
Thanks Martin. The patch LGTM.
--
stage: test needed - commit review
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22351
___
Roundup Robot added the comment:
New changeset e8878579eb68 by Serhiy Storchaka in branch '3.4':
Issue #22351: The nntplib.NNTP constructor no longer leaves the connection
https://hg.python.org/cpython/rev/e8878579eb68
New changeset 6622f68b064b by Serhiy Storchaka in branch 'default':
Issue
Martin Panter added the comment:
Thanks for reviewing, Serhiy. I am posting a new patch addressing the comments:
* Removed underscore from method name
* Made separate MockSslTests subclass, using a hacked ssl_context parameter to
bypass the SSL module
* Separated asserts for closed socket and
Martin Panter added the comment:
Oops, meant to link to Issue 14534 about the test case mixing hack.
BTW this new patch also tests NNTP_SSL, by bypassing the encryption step and
repeating the non-SSL tests.
--
___
Python tracker
Martin Panter added the comment:
Posting issue22351_nntp_fail_4.patch; changes:
* Merge mixin and main TestCase class; mixins like this are sometimes useful to
work around Issue 22351, but not necessary in this case, because there is only
one TestCase class involved.
* Turn the local server
Changes by Serhiy Storchaka storch...@gmail.com:
--
stage: patch review - test needed
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22351
___
___
Serhiy Storchaka added the comment:
Is it possible to make tests based on mocked server instead of real local
server?
NNTP_SSL still is not tested.
--
versions: +Python 3.5
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22351
Changes by Serhiy Storchaka storch...@gmail.com:
--
assignee: - serhiy.storchaka
nosy: +serhiy.storchaka
stage: - patch review
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22351
___
Martin Panter added the comment:
Posting a new patch which closes the file object, and also does the same for
the SSL class.
--
Added file: http://bugs.python.org/file38137/issue22351_nntp_fail_3.patch
___
Python tracker rep...@bugs.python.org
Martin Panter added the comment:
I believe this still leaves the socket open via the “file” object. Just that
there is no ResourceWarning generated due to the way IOBase.__del__() works.
--
___
Python tracker rep...@bugs.python.org
Rishi added the comment:
patch updated based on comments.
--
Added file: http://bugs.python.org/file36873/issue22351_2.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22351
___
Rishi added the comment:
Here is my attempt to fix this issue. This is my first patch ever :).
IMO checking socket leaks in the constructor requires an actual server, so I
create an actual localhost dummy server and test some error conditions that are
encountered by the constructor.
Rishi added the comment:
patch updated to use just plain exception
--
Added file: http://bugs.python.org/file36819/issue22351_1.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22351
___
Changes by Terry J. Reedy tjre...@udel.edu:
--
nosy: +pitrou
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22351
___
___
Python-bugs-list mailing
Changes by Benjamin Peterson benja...@python.org:
--
keywords: +easy
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22351
___
___
Python-bugs-list
New submission from Martin Panter:
If the nntplib.NNTP constructor fails, it often leaves the connection and
socket open until the garbage collector cleans them up and emits a
ResourceWarning:
try:
... NNTP(localhost)
... except Exception as err:
... print(repr(err))
...
28 matches
Mail list logo
