classification
Title: Support ./python -m unittest in test_socket
Type: enhancement Stage:
Components: Tests Versions: Python 3.3
process
Status: open Resolution:
Dependencies: 16968 Superseder:
Assigned To: Nosy List: anacrolix, eric.araujo, ezio.melotti, michael.foord, r.david.murray, vstinner, zach.ware
Priority: normal Keywords: patch

Created on 2012-03-26 04:13 by anacrolix, last changed 2020-02-11 00:04 by vstinner.

Files
File name Uploaded Description Edit
stdlib-tests-test_cases-protocol.patch anacrolix, 2012-03-26 04:13 review
test.test_socket-discoverability.patch anacrolix, 2012-04-01 11:05
test_concurrent_futures-unittest-discoverability.patch anacrolix, 2012-04-01 11:29
Messages (30)
msg156797 - (view) Author: Matt Joiner (anacrolix) Date: 2012-03-26 04:13
Python 3.2 added the test_cases protocol. Many of the stdlib tests won't run using the `$ python3 -m unittest test.test_blah` method due to select unit test class names, and some regrtest arcanity. Defining test_cases makes these tests work as expected.

A patch is attached for several of the unit tests I've dealt with more often.
msg156862 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2012-03-26 20:04
Thanks for doing the work Matt. In principle the patch looks good, and being able to run standard library tests direct from unittest is nice. It's also a step towards being able to use test discovery to run standard library tests.

Is the thread setup and cleanup not needed in test_socket when run from unittest?
msg156968 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-03-28 04:34
I’m a bit disappointed to see that the files after the change are as long.  I hoped we could avoid explicitly listing test cases, thanks to auto-discovery of TestCase subclasses combined with skips for specific platforms or optional modules.  Of course this would remove the possibility to run things like “../../python test_shutil.py” (and I do use these kinds of calls to run tests from my editor all the time), forcing us to always call “./python -m unittest test.test_shutil”.  Michael, do you think this should go to python-dev or do you disapprove (or think people will disapprove)?
msg156970 - (view) Author: Matt Joiner (anacrolix) Date: 2012-03-28 04:54
I think if you can correctly construct the same test case list using discovery then that's far superior. But I haven't tried this, and don't know if you can correctly predicate the support classes using only class decorators.
msg156981 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2012-03-28 15:30
Yes, it would be preferable if unittest could load the set of test classes for each of these test modules without *requiring* a load_tests function.

Each test module will need looking at to see if the standard set of test classes exported by the module is the same as the set built by load_tests. Where they are the same load_tests is not needed. 

I *presume* the point of this patch is that these test modules *can't* just be run by the standard unittest invocation, and load_tests is *needed*.

(And I don't know what you mean by "predicate the support classes" Matt.)
msg156983 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-03-28 15:44
Your presumption is probably correct, however if that is the premise of the patch it is incorrect in detail, since we've already fixed test_queue specifically to be runnable with -m unittest without adding a load_tests.  I haven't looked at the other modules Matt is looking at, but it may well be that they can also be fixed without a load_tests.

The test_queue fix is issue 14333, also opened by Matt, but fixed by Michele.

I think we should make the tests runnable by unittest, but that we should use load_tests only if making discovery work for a particular test is too invasive.
msg156988 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-03-28 16:43
> Of course this would remove the possibility to run things like “../../python test_shutil.py”

I was wrong: we would just have the usual two-liner invoking unittest.main.  What would be broken is running via regrtest if we remove test_main functions (IIRC); the functions could stay until regrtest learns to use unittest discovery.
msg156989 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-03-28 17:14
The test_main functions can be converted to use unittest discovery, though.  That's what I did for test_email.
msg156990 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2012-03-28 17:18
Test discovery is only needed for finding tests in directories of tests - for a single test module the standard test loader should work fine.
msg156992 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-03-28 17:24
Right.  What I meant to say was "test_main can be converted to use normal unittest test loading".  test_email is not an example of that, since it does use test discovery, but is a good example of a test collection that works with both regrtest and unittest without excess boilerplate such as listing individual test suites.
msg157010 - (view) Author: Matt Joiner (anacrolix) Date: 2012-03-29 05:05
Michael:

The thread setup and cleanup is not required, AFAICT. You are also correct in that these particular test modules do not run correctly without modification (although test_queue does now that the bug I reported there was fixed).

Sorry by predicated, I mean that we could mask out the base classes that many of the tests are enumerated on top of. Some kind of decorator for the base could prevent the base class being discovered unless it's inherited from. This is the source of the errors one currently gets running the tests via -m unittest.

Eric:

I'm not sure what you mean by the files being long. I've taken the existing test case list and exposed this directly to both load_tests and unittest.main. The load_tests boilerplate is disappointing, but explicit.
msg157015 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-03-29 09:52
It sounds like we just need to fix the TestCase inheritance, like we did in test_queue.

We should also look more carefully at the threading setup/cleanup.  At some point I think we changed the best-practice idiom to be independent of regrtest, but we probably didn't change all the tests.  I'm not sure, though.
msg157018 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2012-03-29 10:01
One way to exclude base classes from being loaded as tests is to have the base class *not* inherit from TestCase (just from object) - and use it as a mixin class for the actual TestCases. 

This isn't particularly elegant (but works fine). A better way of supporting this in unittest would be a reasonable request (a "not_a_test" class decorator or similar perhaps).
msg157022 - (view) Author: Matt Joiner (anacrolix) Date: 2012-03-29 10:22
I'm working on a patch using TestCase a la test_queue. Perhaps we should create an issue for a base class test case decorator or something to that effect?
msg157031 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2012-03-29 11:14
Yes, feel free to create an issue for that. If you provide a patch for it (with tests) I'll review it. 

The decorator itself can be applied to both TestCase and FunctionTestCase in unittest as well. One implementation would be to apply a private attribute to classes and the loader can be taught to ignore any classes that have that attribute in their __dict__ - i.e. not inherited.
msg157037 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-03-29 11:54
Not particularly elegant?  Why not?  I find marking tests that should be executed by having them (and only them) inherit from TestCase to fit my sense of what is Pythonic, while having a hidden please-ignore-me attribute doesn't.
msg157038 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2012-03-29 12:20
Because you then have classes that inherit from object calling methods that clearly don't exist (until you subclass them *and* TestCase). It looks weird and also means the classes can't be tested in isolation.

With a class decorator the code would *look* straightforward, and the hidden attribute is just an implementation detail.
msg157039 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-03-29 13:14
Hmm.  OK, I guess we can just disagree on what looks straightforward, and since you are the maintainer of unittest you win :)  But unless somebody pronounces, I'll probably keep using the mixin pattern for my own modules.
msg157040 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-03-29 13:21
I guess I'm not really done talking about this, though my bow to you as maintainer still stands.

The mixin tests *can't* be run in isolation, that's the whole point.  Otherwise you could just let unittest run them, and wouldn't need to mark them as not-really-a-TestCase.  The typical mixin test class uses attributes that don't exist except on the concrete TestCases, so I'm not sure why calling methods that don't exist on the mixin is any worse :)
msg157042 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2012-03-29 13:26
It still looks weird to see code calling methods that obviously don't exist, and with no indication *at the call site* where they come from. Making it clearer with naming would help: "TestThingMixin" or similar.

There are classes like this in the unittest test suite, and I was very confused by them initially until I found where and how they were used. It is obviously *not* a pattern that is widely known for test base classes, as we have this problem of it not being done even in the standard library tests.

In contrast I think code similar to the following would be clear and readable without knowing about multiple inheritance and the mixin trick:

    @test_base_class
    class SomeTestBase(TestCase):
        ...
msg157044 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2012-03-29 13:30
Besides which, the mixin pattern won't *stop* working if we provide this extra functionality - it would just be an alternative for those (like myself) who think it impedes code readability. :-)

At this point we're off topic for the *specific issue*, and I'm fine with our own standard library tests moving to use mixins to support standard unittest invocation. I would suggest the base test cases include Mixin in their name to make it clear how they should be used.
msg157045 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-03-29 13:41
The convention in the stdlib is to name the mixin classes TestXXXBase.  Granted, a lot of those inherit from TestCase.  I have no objection to calling them Mixin instead, I'm just pointing out that there is an existing convention.

(As an aside, when I first ran into the Base pattern it was in a file where the Base did subclass TestCase, and it took me forever to figure out that the Base test wasn't actually getting run.  So a decorator is definitely superior to listing the test cases that actually run elsewhere in the file!)
msg157053 - (view) Author: Matt Joiner (anacrolix) Date: 2012-03-29 14:55
It could in fact be necessary, if the inheritance cannot be juggled to  give the right MRO. Fortunately this is not the case, I should have a patch using TestCase inheritance for discovery tomorrow.
msg157272 - (view) Author: Matt Joiner (anacrolix) Date: 2012-04-01 11:05
The patch attached, rejigs the TestCase inheritance in test.test_socket so that tests run correctly using unittest discovery. Recent changes have made test_queue, and test_threading run without similar fixes, so I don't think fixes for those are necessary anymore.
msg157276 - (view) Author: Matt Joiner (anacrolix) Date: 2012-04-01 11:29
Attached is a patch for test_concurrent_futures, similar to the patch for test_socket.
msg187114 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2013-04-16 19:02
I just happened across this issue, which I think has been superseded by issues #16748 and #16968.
msg187120 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-04-16 20:20
Zach, what about the test_socket patch?
msg187122 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2013-04-16 20:39
The test_socket patch is incomplete with the way I've been converting test modules, but it is a good starting point.  Should this issue be renamed to be specifically for test_socket?

The necessity of reaping threads at the end of the test poses a bit of an issue for test discovery, which has been discussed pretty thoroughly in #16968.  If the fix in that issue is approved and committed, it would be pretty easy to extend Matt's test_socket patch to complete the conversion.
msg187123 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-04-16 20:52
Sure, let's rename it since there's a useful patch here.
msg361765 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-02-11 00:04
I marked bpo-38063 as a duplicate of this issue.
History
Date User Action Args
2020-02-11 00:04:15vstinnersetnosy: + vstinner
messages: + msg361765
2020-02-11 00:04:06vstinnerlinkissue38063 superseder
2013-12-17 21:26:38zach.warelinkissue16748 dependencies
2013-04-16 20:52:36r.david.murraysetdependencies: + Fix test discovery for test_concurrent_futures.py
2013-04-16 20:52:04r.david.murraysetmessages: + msg187123
title: Support ./python -m unittest in the stdlib tests -> Support ./python -m unittest in test_socket
2013-04-16 20:39:01zach.waresetmessages: + msg187122
2013-04-16 20:20:23r.david.murraysetmessages: + msg187120
2013-04-16 19:02:45zach.waresetnosy: + zach.ware
messages: + msg187114
2012-04-14 04:26:43ezio.melottisetnosy: + ezio.melotti
2012-04-01 11:29:54anacrolixsetfiles: + test_concurrent_futures-unittest-discoverability.patch

messages: + msg157276
2012-04-01 11:05:58anacrolixsetfiles: + test.test_socket-discoverability.patch

messages: + msg157272
2012-03-29 14:55:44anacrolixsetmessages: + msg157053
2012-03-29 13:41:03r.david.murraysetmessages: + msg157045
2012-03-29 13:30:53michael.foordsetmessages: + msg157044
2012-03-29 13:26:55michael.foordsetmessages: + msg157042
2012-03-29 13:21:41r.david.murraysetmessages: + msg157040
2012-03-29 13:14:01r.david.murraysetmessages: + msg157039
2012-03-29 12:20:28michael.foordsetmessages: + msg157038
2012-03-29 11:54:22r.david.murraysetmessages: + msg157037
2012-03-29 11:14:46michael.foordsetmessages: + msg157031
2012-03-29 10:22:18anacrolixsetmessages: + msg157022
2012-03-29 10:01:22michael.foordsetmessages: + msg157018
2012-03-29 09:52:44r.david.murraysetmessages: + msg157015
2012-03-29 05:05:40anacrolixsetmessages: + msg157010
2012-03-28 17:24:13r.david.murraysetmessages: + msg156992
2012-03-28 17:18:16michael.foordsetmessages: + msg156990
2012-03-28 17:14:10r.david.murraysetmessages: + msg156989
2012-03-28 16:44:09eric.araujosettitle: Support the load_tests protocol in the stdlib tests -> Support ./python -m unittest in the stdlib tests
2012-03-28 16:43:39eric.araujosetmessages: + msg156988
2012-03-28 15:44:49r.david.murraysetnosy: + r.david.murray
messages: + msg156983
2012-03-28 15:30:27michael.foordsetmessages: + msg156981
2012-03-28 04:54:56anacrolixsetmessages: + msg156970
2012-03-28 04:34:12eric.araujosetnosy: + eric.araujo

messages: + msg156968
versions: - Python 3.2
2012-03-26 20:16:51michael.foordsettitle: Support the test_cases protocol in the stdlib tests -> Support the load_tests protocol in the stdlib tests
2012-03-26 20:04:24michael.foordsetmessages: + msg156862
2012-03-26 10:09:38pitrousetnosy: + michael.foord
2012-03-26 04:13:57anacrolixcreate