Make CPython test package discoverable

[zware]

BPO	16748
Nosy	@terryjreedy, @ezio-melotti, @merwok, @bitdancer, @cjerdonek, @ericsnowcurrently, @zware, @serhiy-storchaka
Dependencies	bpo-14408: Support ./python -m unittest in test_socket bpo-16852: Fix test discovery for test_genericpath.py bpo-16888: Fix test discovery for test_array.py bpo-16968: Fix test discovery for test_concurrent_futures.py bpo-18258: Fix test discovery for test_codecmaps*.py bpo-20008: Clean up/refactor/make discoverable test_decimal bpo-22002: Make full use of test discovery in test subpackages
Files	issue16748.diff issue16748_genericpath.diff: For test_*path.py issue16748_genericpath.v2.diff: test_*path.py version 2 checkTestOverriding.diff: Hacking patch which prints test methods overriding test_overriding.txt: List of overridden test methods

^{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 = 'https://github.com/ezio-melotti'
closed_at = None
created_at = <Date 2012-12-22.01:02:27.055>
labels = ['type-bug', 'tests']
title = 'Make CPython test package discoverable'
updated_at = <Date 2020-01-29.00:34:50.166>
user = 'https://github.com/zware'

bugs.python.org fields:

activity = <Date 2020-01-29.00:34:50.166>
actor = 'brett.cannon'
assignee = 'ezio.melotti'
closed = False
closed_date = None
closer = None
components = ['Tests']
creation = <Date 2012-12-22.01:02:27.055>
creator = 'zach.ware'
dependencies = ['14408', '16852', '16888', '16968', '18258', '20008', '22002']
files = ['28523', '28535', '28542', '28689', '28690']
hgrepos = []
issue_num = 16748
keywords = ['patch']
message_count = 49.0
messages = ['177913', '177925', '177927', '178091', '178140', '178156', '178420', '178425', '178428', '178737', '178749', '178795', '178812', '178831', '178835', '178839', '178840', '178841', '178861', '178932', '178947', '178956', '178958', '178965', '179055', '179286', '179288', '179292', '179296', '179297', '179298', '179305', '179308', '179316', '179319', '179347', '179349', '179361', '179370', '179634', '179635', '179663', '179665', '179956', '179971', '179980', '179996', '189292', '191487']
nosy_count = 9.0
nosy_names = ['terry.reedy', 'ezio.melotti', 'eric.araujo', 'r.david.murray', 'chris.jerdonek', 'python-dev', 'eric.snow', 'zach.ware', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue16748'
versions = ['Python 3.4', 'Python 3.5']

[zware]

As pointed out by Éric Araujo in msg177908 of bpo-16694, tests using the idiom presented in PEP-399 are subject to breakage of test discovery. This issue's goal is to root out and fix all usages of this idiom.

So far, only test_heapq is known to be affected.

As for fixing the issue, each module get its own mixin class, or should a simple class such as:

class TestPyAndCMixin:
    module = None

be added to test.support?

[serhiy-storchaka]

No need to add this trivial class to test.support. Mixin can have different parameters in additional to 'module' (see 16659).

I found a lot of usages of this idiom in tests and some of them can't be changed so simple (for example test_genericpath and test_functools).

[serhiy-storchaka]

Éric, can you submit a test which exposes the problem?

[merwok]

I won’t be online for the next two weeks.

I think there is a bug with Michael Foord, RDM and I where we talk about this very issue.

[skrah]

Perhaps I misunderstood something, but test_decimal.py *is* using the
exact idiom from PEP-399 and it works. Why do you want to "fix" the
usage of this idiom?

[skrah]

I finally understood the issue. So this does not work:

./python -m unittest discover [Lib/test/](https://github.com/python/cpython/blob/main/Lib/test) 'test_dec*.py'

Neither does this:

./python -m unittest discover [Lib/test/](https://github.com/python/cpython/blob/main/Lib/test) 'test_multipro*.py'

And this fails, too (still hanging):

./python -m unittest discover [Lib/test/](https://github.com/python/cpython/blob/main/Lib/test) 'test_thread*.py'

I'm not sure why tests in the Python test suite should be discoverable.
If I read this ...

http://www.voidspace.org.uk/python/weblog/arch_d7_2009_05_30.shtml

..., the feature is for projects that don't have a test collection machinery.

[brettcannon]

Making tests discoverable allows for the future possibility of dropping our custom test runner code and using more of unittest code by having regrtest use unittest's discovery code to find all tests.

[terryjreedy]

I should think that the first fix should be to the PEP. If I understand msg177908, that would mean removing unittest.TestCase as a base for ExampleTest and adding it as bases for AcceleratedExampleTest and PyExampleTest. Or have I missed something?

[bitdancer]

That sounds right to me.

Note that PEP-399 is following older conventions that were laid down in a time when unittest did not *have* test discovery, so this is a new paradigm we'd like to move to (for the reasons Brett mentioned), and it may take a while to get there. It applies to more than just the python/C accelerator distinction; it applies any time a base class plus specialized classes are used to construct test cases. (I do this a bunch in the email tests, for example, and that has no accelerator.)

[ezio-melotti]

I just came across the problem described here while reviewing bpo-16694.

The idiom I used for the JSON tests[0] (and possibly a couple of other tests) when I rewrote them was to have something like:

class FooTest:
    # all the test methods here

class CFooTest(FooTest, unittest.TestCase):
    module = c_foo

class PyFooTest(FooTest, unittest.TestCase):
    module = py_foo

The reason to only have the subclasses inheriting from unittest.TestCase was exactly to avoid having FooTest as a discoverable TestCase.

I think PEP-399 should be updated and the tests should be fixed.
I don't think it's necessary to provide a base class as suggested in the first message (it's actually not even necessary to set the module to None, because eventually it will be set to either some cmodule or pymodule. In case something goes wrong it's even better if the AttributeError says "self has no attribute 'module'" rather than "NoneType has no attribute 'somemeth'".)

@brett, should I open a separate issue for the PEP-399 changes?

[0] see e.g.: Lib/test/json_tests/test_float.py and Lib/test/json_tests/init.py (note that here the idiom is a bit more complicated, because in addition to set the self.module, I also had to set additional attributes and work with different test files in the same package. I also added additional tests in __init__ to make sure that import_fresh_module worked after adapting it to work with packages.)

[brettcannon]

I created http://bugs.python.org/issue16835 to remind me to update PEP-399.

[ezio-melotti]

Attached patch fixes Lib/test/test_heapq.py.
I also replaced the test_main() with unittest.main() and got rid of the code previously used to "test" reference leaks. As it is the patch can be applied on 3.3/default. If you think it should be backported to 2.7/3.2 too, the changes on test_main() should be reverted.

[brettcannon]

Why remove the refleak tests? I'm sure Raymond put those in for a reason as I know he tries to reuse various objects a lot and this helped make sure he didn't mess anything up.

I don't think the tests need to be backported.

[ezio-melotti]

Why remove the refleak tests?

Because regrtest already provides the feature, so I don't see a reason to duplicate it here. FWIW the same code is copy/pasted elsewhere too, e.g. in Lib/test/test_operator.py:438.

[brettcannon]

The patch LGTM

[python-dev]

New changeset 008bac4e181c by Ezio Melotti in branch '3.3':
bpo-16748: test_heapq now works with unittest test discovery.
http://hg.python.org/cpython/rev/008bac4e181c

New changeset de6bac0a40cc by Ezio Melotti in branch 'default':
bpo-16748: merge with 3.3.
http://hg.python.org/cpython/rev/de6bac0a40cc

[ezio-melotti]

Fixed, thanks for the review!

[serhiy-storchaka]

As I mentioned above, not only test_heapq uses this idiom, but a lot of other tests. Try to fix hard cases first: test_genericpath and test_functools.

[zware]

Here's a patch that should clear up test_genericpath.py (and other path tests).

[serhiy-storchaka]

See also test_functools, test_xml_etree, test_bisect, test_bz2, test_warnings, test_decimal, test_datetime, json_tests, test_io, test_concurrent_futures, and many, many other undiscoverable tests.

[zware]

Here's version 2 of the genericpath patch.

Should I try to fix everything in one patch, or one patch per test module (or group of test modules like test_(generic|mac|nt|posix)path.py)? And if separate, should each one get its own issue, or just keep them all here?

[bitdancer]

I would suggest one patch and issue per test module. If multiple test modules are related enough, they could go in one patch/issue; that's a judgement call. We can make this issue dependent on those individual issues.

[zware]

Sounds good to me. Shall I move the genericpath fix to a new issue, or leave that one here and begin starting new issues with the next one tackled?

Any volunteers for being nosied on new issues to make the dependency link back to this one for me? :)

[brettcannon]

You can go ahead and start a new issue so it isn't forgotten about as this becomes a meta issue.

And you can always add me to the nosy, just can't guarantee how fast I will do the reviews. =)

[ezio-melotti]

Feel free to add me to the nosy.

[zware]

I've come up with a semi-comprehensive list of the tests that cause ugly failures with test discovery. Tests were run on 3.4 debug, on Windows 7 32bit without several of the 'external' projects built, using the command PCbuild\python_d.exe -m unittest discover [Lib/test/](https://github.com/python/cpython/blob/main/Lib/test) "test_x*.py" where 'x' was whatever it took to get a specific test or group of tests to run.

There are two basic causes of failure, and a couple of oddballs.

First up, tests that fail with discovery, but run fine with PCbuild\python_d.exe -m test test_name:

test_array test_asyncore test_bisect test_bufio test_bytes test_codecs test_concurrent_futures test_configparser test_ctypes test_dbm test_decimal test_file test_format test_ftplib test_future3 test_hash test_imaplib test_import test_index test_io test_iterlen test_locale test_multiprocessing test_module test_osx_env test_pickle test_random test_robotparser test_runpy test_set test_shelve test_socket test_sqlite test_sys test_tarfile test_time test_univnewlines test_warnings test_xml_etree

These 39 should be relatively straightforward fixes like heapq and genericpath have been.

The next group are tests that are properly skipped by the test package, but fail noisily with unittest discovery:

test_codecmaps_cn test_codecmaps_hk test_codecmaps_jp test_codecmaps_kr test_codecmaps_tw test_crypt test_curses test_epoll test_fcntl test_gdb test_grp test_hashlib test_ioctl test_kqueue test_largefile test_nis test_openpty test_ossaudiodev test_pipes test_poll test_posix test_pty test_pwd test_readline test_resource test_smtpnet test_socketserver test_ssl test_syslog test_tcl test_threadsignals test_tk test_ttk_guionly test_ttk_textonly test_urllib2net test_wait3 test_wait4 test_winsound test_zipfile64

Some of these are skipped due to certain resources not being allowed, some don't have the proper module available. In any case, discovery does not respond well to the skip methods used for each of them.

The 'oddballs' I mentioned before are as follows:

json_tests.test_fail leakers.test_gestalt test_glob test_json test_shutil test_urllib2_localnet

Each of these fail on my machine whichever way I run them. I'm not sure if any of them actually have any issues with discovery, I'm merely listing them here for completeness' sake. I will try to test them again myself on Linux later, and report back what I find.

If anyone else feels led to tackle any of these, please add me to the nosy list of any new issues filed. I plan to eventually work through all of these myself, but don't want to duplicate effort :)

Thanks,

Zach Ware

[bitdancer]

Great list, thanks.

The ones that fail to be run/skipped when run under discovery can probably be fixed by moving them to the more modern unittest 'skip' functions instead of depending on being run under regrtest and using the test.support resource functions. When run directly they should not skip due to a resource not being set, but when run under regrtest (I'm not sure how you detect that but if there isn't currently a way we should make one via test.support) they should respect the resources.

Unittest doesn't yet have a concept of resources, but I believe there is an open issue for it, so at some point we will hopefully be able to move all resource management to unittest and out of regrtest. But not yet.

[ezio-melotti]

While we are at it, should we also move these tests to use unittest.main() instead of test_main() and similar?

[terryjreedy]

test_main makes it trivial to import a test file and run the test.
>>> import test/test_xxx as t; t.test_main()
I do not know the implications of unittest.main(), but I would dislike losing the above ability.

[bitdancer]

As we discussed in another issue, you just need to change your pattern to:

>> import unittest.main as runtest
>> import test.test_xxx as t
>> runtest(t)

Which granted is more typing if you are running just one test, but not much more if you are running more than one. You could also put the import for unittest into your rc file.

[ezio-melotti]

Without test_main you would have to do unittest.main(t, exit=False). I'm not sure there's an easier way.

[cjerdonek]

>>> import test.test_xxx as t; t.test_main()

As long as the module has "import unittest", you could also do the following, which has just 5 more characters :)

>> import test.test_xxx as t; t.unittest.main(t)

[cjerdonek]

One more. Since unittest imports strings, you can also do:

>> import unittest as u; u.main("test.test_xxx")

[ezio-melotti]

I think that's the last nail in the coffin of test_main.
Terry, do you agree?

[serhiy-storchaka]

There are tests outside of Lib/test/ hierarchy.

[bitdancer]

Yes, but not many, and not as many as there used to be. I'd like to see them all moved, but met resistance on that front. It may be that those just can't be run using unittest discovery, and perhaps that will eventually convince the maintainers to move them. But probably not until a number of years from now :)

[bitdancer]

Also, it may be possible to add unittest discovery hooks to the stubs that *are* in Lib/test.

[brettcannon]

It's totally doable to set up stubs in Lib/test to use test discovery on those tests which exist outside of that directory. test_importlib actually has some code for that left over from when it lived in Lib/importlib/test: http://hg.python.org/cpython/file/4617b7ac302a/Lib/test/test_importlib/__init__.py

[cjerdonek]

Also, it may be possible to add unittest discovery hooks to the stubs that *are* in Lib/test.

The load_tests protocol (2.7, 3.2+) seems like the right approach for this:

http://docs.python.org/dev/library/unittest.html#load-tests-protocol

[ezio-melotti]

There are tests outside of Lib/test/ hierarchy.

See bpo-10572.

[cjerdonek]

There are lots of modules to change here. I wonder if some or most of this couldn't be automated.

For example, is there any reason we couldn't write a script to check for the type of test duplication fixed documentation-wise in bpo-16835? We could use such a script to find the classes that need to be updated when removing test_main (or to double-check existing test modules).

I don't know if there are ever times we want such duplication.

[cjerdonek]

As suggested in the previous comment, here is simple code to find candidates for test duplication (TestCase subclasses subclassing other TestCase classes):

def find_dupes(mod):
    objects = [getattr(mod, name) for name in sorted(dir(mod))]
    classes = [obj for obj in objects if isinstance(obj, type) and
               issubclass(obj, unittest.TestCase)]
    for c in classes:
        for d in classes:
            if c is not d and issubclass(d, c):
                print("%s: %s < %s" % (mod.__name__, c.__name__, d.__name__))

Out of curiosity, I ran a modified form of this against all test modules to find which ones fit this pattern and *already* rely on unittest discovery (i.e. don't have test_main()). We might want to adjust these as well. There were four: test_asyncore, test_configparser, test_heapq, test_ipaddress

[serhiy-storchaka]

Here is a dirty patch which hacks unittest to search possible test overriding. Just apply the patch and run regression tests.

[zware]

Chris:

There are lots of modules to change here. I wonder if some or most of
this couldn't be automated.

Possibly, but I don't mind going through individually if Ezio (or others) don't mind committing individually. From what I've seen, the test suite is varied enough within itself that it would be pretty hard to properly automate the changes I've been making. Also, I'm of the opinion that we'll end up with a higher quality result doing things by hand anyway.

[cjerdonek]

Also, I'm of the opinion that we'll end up with a higher quality result doing things by hand anyway.

Automation doesn't (and shouldn't) preclude careful review and modifications by hand. My point was that it seems like it might be able to get you, say, 80% of the way there (e.g. by deleting test_main(), determining which classes are called by test_main(), adding ", unittest.TestCase", creating stub classes that could be retained or deleted). Or at least provide some additional checks or info. There are nearly 400 test files using test_main(), no?

[zware]

For the conversion from test_main() to unittest.main(), I could certainly see automation helping a lot; most cases are very simple. But really, that issue is somewhat tangential to this one and in my last message I was thinking from the perspective of just the 80 or so modules that completely fail test discovery otherwise. I believe that trying to automate those would be more trouble than it's worth. Especially if bpo-16935 is fixed, as that will knock out most of the tests I listed above that failed to skip properly.

[cjerdonek]

Okay, well the title of this issue is "Make CPython test package discoverable," so as part of that we should be confirming that test discovery finds the same tests that test_main() runs (and if not, to understand why and/or make them the same).

[terryjreedy]

Chris: >>> import unittest as u; u.main("test.test_xxx")
Ezio: I think that's the last nail in the coffin of test_main.
Terry, do you agree?

Belated answer: now that I use and understand the idiom, yes.

Chris: The load_tests protocol (2.7, 3.2+) seems like the right approach for [tests in directories outside of /test].

I am using that in the new idlelib/idle_test/init.py. But perhaps the load_tests() example in the doc, which I copied, should be changed from 'do nothing' to 'discover all tests in the directory' as a more useful default.

[zware]

So many things have been fixed since I last made a list of failing modules that it seemed like time to make a new one. First, failing modules that already have open issues with patches:

• test_codecmaps* (bpo-18258)
• test_concurrent_futures (bpo-16968)
• test_locale (bpo-17767)
• test_multiprocessing (bpo-17778)
• test_socket (bpo-14408 -- incomplete patch, waiting on resolution of bpo-16968 for how to handle thread/process reaping)

Other tests that fail, that may be the fault of my test machine rather than the tests themselves (or, ones that I'd like others to try before I call them broken):

• test_urllib2net (fails on my machine on normal runs due to network setup)
• test_shutil (fails on my machine on normal runs for unknown reasons relating to symlinks)

The below tests fail for reasons that I expect to probably be related to test run order, as all of them pass when narrowing the pattern to match only them (e.g., "test_dbm*.py" rather than "test_*.py"):

• test_dbm (had a permission error on a test file)
• test_venv (tried to delete a temp dir that was unexpectedly non-empty)
• test_wsgiref (failure in testEnviron)
• test_logging (failure in test_warnings)
• test_inspect (had some issue with locating its test files)

This leaves only 6 other tests that fail due to improper inheritance, setup done in test_main rather than setUp/setUpClass/setUpModule, or other reasons directly related to discovery:

• test_decimal
• test_largefile
• test_pickle (pickletester)
• test_runpy
• test_shelve
• test_xml_etree

Other tests that need some manner of attention related to discovery:

• test_json: it half-uses discovery, it could use it completely
• test_importlib: about the same as test_json
• leakers package: either rename to non-discoverable names or otherwise make leakers.test_gestalt (in particular) not report an error when discovered.

This may not be a comprehensive list, but it does cover all of the most obvious failures that remain.

[serhiy-storchaka]

Can this issue be closed? I finished conversion from test_main() to unittest.main() in #89392, but I did test all tests with unittest discover.

[zware]

Sounds good to me. Thanks for finishing it off, Serhiy!