Avoid test_main() in test_httplib; gc.collect() dangling threads

[vadmium]

BPO	27787
Nosy	@vstinner, @vadmium
Files	httplib-tests.patch httplib-tests.v2.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2016-08-23.13:10:17.082>
created_at = <Date 2016-08-17.14:33:36.533>
labels = ['type-feature', 'tests']
title = 'Avoid test_main() in test_httplib; gc.collect() dangling threads'
updated_at = <Date 2016-08-23.18:20:55.818>
user = 'https://github.com/vadmium'

bugs.python.org fields:

activity = <Date 2016-08-23.18:20:55.818>
actor = 'python-dev'
assignee = 'none'
closed = True
closed_date = <Date 2016-08-23.13:10:17.082>
closer = 'martin.panter'
components = ['Tests']
creation = <Date 2016-08-17.14:33:36.533>
creator = 'martin.panter'
dependencies = []
files = ['44134', '44163']
hgrepos = []
issue_num = 27787
keywords = ['patch']
message_count = 7.0
messages = ['272954', '272955', '273004', '273193', '273197', '273433', '273502']
nosy_count = 3.0
nosy_names = ['vstinner', 'python-dev', 'martin.panter']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue27787'
versions = ['Python 3.5', 'Python 3.6']

[vadmium]

In bpo-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.

[vstinner]

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).

[vadmium]

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.

[python-dev]

New changeset 7ddbc2263943 by Martin Panter in branch '3.5':
Issue bpo-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 bpo-27787: Merge regrtest fixup from 3.5
https://hg.python.org/cpython/rev/e5777c5d108c

[vadmium]

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.

[python-dev]

New changeset 1f73355afebb by Martin Panter in branch 'default':
Issue bpo-27787: Remove test_main() and hard-coded list of test classes
https://hg.python.org/cpython/rev/1f73355afebb

[python-dev]

New changeset 31e4495a34ce by Terry Jan Reedy in branch 'default':
Issue bpo-27787: No longer call deleted test_main().
https://hg.python.org/cpython/rev/31e4495a34ce