classification
Title: Avoid test_main() in test_httplib; gc.collect() dangling threads
Type: enhancement Stage: resolved
Components: Tests Versions: Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: martin.panter, python-dev, vstinner
Priority: normal Keywords: patch

Created on 2016-08-17 14:33 by martin.panter, last changed 2016-08-23 18:20 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
httplib-tests.patch martin.panter, 2016-08-17 14:33 review
httplib-tests.v2.patch martin.panter, 2016-08-20 08:43 review
Messages (7)
msg272954 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-08-17 14:33
In Issue 12319, there are many iterations of a patch that adds a new TestCase subclass to Lib/test/test_httplib.py. However it never got run by the main regrtest infrastructure, because nobody remembered to add the class to the list of test classes. So I want to remove test_main(). It seems this would let the classes be automatically discovered (like normal unittest usage).

I understand @reap_threads is to avoid background threads continuing between tests (especially when a test fails). I improved the cleanup of the background thread in one test. There are three other test methods using test.ssl_servers.make_https_server(), which also runs a background thread, but that already seems to clean itself up properly, via case.addCleanup(server.join).

I found that the test infrastructure randomly complained about dangling threads without @reap_threads. It uses a set of weak references to thread objects. The solution seems to be to call gc.collect() before checking. This is what @reap_threads does, so maybe my patch would eliminate the need for @reap_threads in other test files as well. In fact, if everybody called join() on their threads, we may be able to eliminate @reap_threads altogether.
msg272955 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-08-17 14:38
Your patch changes 3 different things. After the code review, I suggest to split it in two changes (fix test_httplib threading ripper, fix save_env, fix test_httplib main).
msg273004 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-08-18 00:07
Yes I agree it would make sense to separate the test_httplib changes from the general change. I thought this task would be a very easy change, and noticed it wasn’t that simple the last minute.

I would like to adjust the cleanup call to

self.addCleanup(thread.join, float(1))

That should be a better equivalent to what @reap_threads does. Or I can just use @reap_threads directly on test_response_fileno(), if people prefer.
msg273193 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-08-20 07:45
New changeset 7ddbc2263943 by Martin Panter in branch '3.5':
Issue #27787: Clean up weak references before checking for dangling threads
https://hg.python.org/cpython/rev/7ddbc2263943

New changeset e5777c5d108c by Martin Panter in branch 'default':
Issue #27787: Merge regrtest fixup from 3.5
https://hg.python.org/cpython/rev/e5777c5d108c
msg273197 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-08-20 08:43
I committed the gc_collect() change to 3.5 as well, because it helped me with a separate test case.

Here is the remaining change, for 3.6 only. I added a timeout to the join() call. This matches the @reap_threads decorator, which times out after 1 s.
msg273433 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-08-23 10:49
New changeset 1f73355afebb by Martin Panter in branch 'default':
Issue #27787: Remove test_main() and hard-coded list of test classes
https://hg.python.org/cpython/rev/1f73355afebb
msg273502 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-08-23 18:20
New changeset 31e4495a34ce by Terry Jan Reedy in branch 'default':
Issue #27787: No longer call deleted test_main().
https://hg.python.org/cpython/rev/31e4495a34ce
History
Date User Action Args
2016-08-23 18:20:55python-devsetmessages: + msg273502
2016-08-23 13:10:17martin.pantersetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2016-08-23 10:49:10python-devsetmessages: + msg273433
2016-08-20 08:43:19martin.pantersetfiles: + httplib-tests.v2.patch

messages: + msg273197
versions: + Python 3.5
2016-08-20 07:45:17python-devsetnosy: + python-dev
messages: + msg273193
2016-08-18 00:07:11martin.pantersetmessages: + msg273004
2016-08-17 14:38:23vstinnersetnosy: + vstinner
messages: + msg272955
2016-08-17 14:33:36martin.pantercreate