Title: Make full use of test discovery in test subpackages
Type: enhancement Stage: resolved
Components: Tests Versions: Python 3.5, Python 3.4
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: zach.ware Nosy List: BreamoreBoy, berker.peksag, brett.cannon, ezio.melotti, python-dev, r.david.murray, serhiy.storchaka, vstinner, 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.

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, 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 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/
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.

New changeset 6298895a52de by Zachary Ware in branch 'default':
Closes #22002: Merge with 3.4
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, 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 or, and thus should be no conflict.  Please correct me if I'm wrong on that.

Thanks for the reviews!
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: + vstinner
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