classification
Title: Make full use of test discovery in test subpackages
Type: enhancement Stage: resolved
Components: Tests Versions: Python 3.5, Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: zach.ware Nosy List: BreamoreBoy, berker.peksag, brett.cannon, ezio.melotti, haypo, python-dev, r.david.murray, serhiy.storchaka, zach.ware
Priority: normal Keywords: patch

Created on 2014-07-17 20:09 by zach.ware, last changed 2014-07-23 17:11 by zach.ware. This issue is now closed.

Files
File name Uploaded Description Edit
package_discovery.diff zach.ware, 2014-07-17 20:09 Patch against 3.4 review
Messages (9)
msg223366 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-07-17 20:09
Here's a patch against 3.4 implementing Serhiy's suggestion in msg223277 and taking it a step further, actually using test discovery in all of the test.test_* subpackages.

To reduce duplication, the patch adds a 'load_package_tests' function to test.support, which is then used in each of the subpackages' load_tests function.

test_json and test_tools should have no visible changes from this patch.  test_asyncio and test_email do have slight differences, but only in verbosity level: pre-patch, `python -m test.test_(asyncio|email)` runs at verbosity=2 (support.run_unittest default); with patch, they run at verbosity=1 (unittest default).  test_asyncio also reports one more skipped test on Windows, due to a module that raises SkipTest on import.

@Brett: test_importlib sees the most changes, and I'd like to be sure that things are as you expect them to be.  It looks like all of the test_suite() stuff is unused leftovers from when "test_importlib" was "importlib.test" and that test_importlib has actually been relying on unittest discovery (but bypassing load_tests and thereby not working with `python -m unittest test.test_importlib`), but I'd like confirmation on that.

A nice bonus with this patch is that (for example) `python -m test.test_importlib.source` works, testing just the named test_importlib subpackage.
msg223407 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2014-07-18 12:45
I can confirm everything you said is accurate, Zachary. And I very much look forward to the results of the patch. =)
msg223517 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-07-20 15:46
I have no problem with the change to test_email.  (I could have sworn I tried unittest.main() in __main__ when I set it up, but I must have screwed something up).

I didn't go over the patch line by line, but it looks good to me.
msg223536 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-07-20 20:41
I'm getting lost in all the enhancement requests for tests.  How does this relate to e.g. 16748 or 10572?
msg223576 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-07-21 14:27
Thank you, Brett and David.

@Mark: This issue is a continuation of the #16748 effort (#16748 is a meta-issue for this kind of thing) and unrelated to #10572 (though this will add a feature to test.support that can be useful for the scattered test packages).
msg223577 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-07-21 14:27
Victor, could you give me a yay or nay on the test_asyncio change?
msg223690 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-07-22 20:23
Nice. Could you please comment multiple dirname()-s in load_package_tests() as it done for basename in Lib/test/test_tools/__init__.py?
msg223745 - (view) Author: Roundup Robot (python-dev) Date: 2014-07-23 17:07
New changeset 4f9f7e0fe1fd by Zachary Ware in branch '3.4':
Issue #22002: Make full use of test discovery in test sub-packages.
http://hg.python.org/cpython/rev/4f9f7e0fe1fd

New changeset 6298895a52de by Zachary Ware in branch 'default':
Closes #22002: Merge with 3.4
http://hg.python.org/cpython/rev/6298895a52de
msg223747 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-07-23 17:11
Committed with a couple of changes from the posted patch: commented on the multiple dirname() calls as Serhiy suggested, removed the 'with DirsOnSysPath():' line since it turns out to be unnecessary when supplying top_level_dir to loader.discover(), and added some documentation for the new function to Doc/library/test.rst.

Victor, I went ahead and did the change to test_asyncio since it looks like the tulip project test package doesn't actually have __init__.py or __main__.py, and thus should be no conflict.  Please correct me if I'm wrong on that.

Thanks for the reviews!
History
Date User Action Args
2014-07-23 17:26:18zach.warelinkissue21500 superseder
2014-07-23 17:11:04zach.waresetassignee: zach.ware
messages: + msg223747
2014-07-23 17:07:11python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg223745

resolution: fixed
stage: patch review -> resolved
2014-07-22 20:23:16serhiy.storchakasetmessages: + msg223690
2014-07-21 14:27:59zach.waresetnosy: + haypo
messages: + msg223577
2014-07-21 14:27:16zach.waresetmessages: + msg223576
2014-07-21 14:26:35zach.warelinkissue16748 dependencies
2014-07-20 20:41:18BreamoreBoysetnosy: + BreamoreBoy
messages: + msg223536
2014-07-20 15:46:18r.david.murraysetnosy: + r.david.murray
messages: + msg223517
2014-07-18 12:45:16brett.cannonsetmessages: + msg223407
2014-07-17 20:09:20zach.warecreate