In the past (<= 2.6) regrtest skipped a test if any import failure
happened, which masked various real test failures. This was fixed, and
tests that should be skipped if certain modules are not available were
changed to use (test_)support.import_module, which causes a skip if that
particular module cannot be imported.

If python is compiled --without-threads, then the following tests
currently 'crash' because they cannot import thread and/or threading:

test_hashlib test_asyncore test_wait3 test_threading test_socket
test_wait4 test_capi test_xmlrpc test_ctypes
test_zipimport_support test_threading_local test_multiprocessing
test_file2k test_smtplib test_threadedtempfile test_threadsignals
test_thread test_queue test_asynchat test_contextlib
test_bz2 test_ftplib test_cmd test_pdb test_io test_doctest
test_sqlite test_logging test_telnetlib test_threaded_import
test_httpservers test_fork1 test_docxmlrpc test_urllib2_localnet
test_poplib test_socketserver

All of these tests should either be changed to use import_module when
importing thread/threading, or changed so that the tests requiring
thread support are skipped if thread support is not available.

Note that test_bsddb3 also fails, but it is not an import error crash.

The patch makes it so that tests that use threading skip rather than
generate errors when python is compiled --without-threads.

I added a skip_if_no('modulename') decorator to test_support.

Let me know if this patch is too big to review and I'll break it up. It
is 800 lines.

One thing to note: when threading is disabled, all sqlite3 tests are
skipped. There are a few test files that do not use threading, but I
couldn't think of an easy way to split them out.

Tested on OS X and Linux.

Comments about nothreads.patch. There are unrelated changes:

Lib/test/test_xmlrpc.py:

•    p = xmlrpclib.ServerProxy(URL, transport=t)
  

+ p = xmlrpclib.ServerProxy('http:', transport=t)

Lib/test/test_macostools.py (working copy)

•    try:
  

•        os.unlink(TESTFN2)
  

•    except:
  

•        pass
  

Or you should explain me why you changed that :-)

Why not using test_support.import_module('threading') in test_xmlrpc.py, test_file2k.py, etc.?

I don't like importing a module in a function: you should use test_support.import_module('threading') in test_io.py, test_logging.py, test_capi.py, etc.

skip_if_no() function name is a little bit too generic. I don't have any suggestion.

In the test_xmlrpc.py case I changed the value from URL to 'http:' because the code that sets URL to a valid url relies on threading. When threading is disabled, URL is set to None and the test will fail. The two ServerProxy test cases that were modified in this way do not actually use the network at all. They instead test that the close() method returns None and that the transport() method returns the transport passed in to the constructor. I figured setting the url to 'http:' instead of an empty string was more readable. The reader would know that the string was supposed to be a url and that it was utterly meaningless in this case.

In the test_macostools.py case, the os.unlink(TESTFN2) call is a copy and paste error from the previous test. This test tried to remove an alias it never created, and it failed to check that the destination directory for the alias actually was a directory (it only checked that the path existed - in my case it was a file, not a directory). I fixed the test to check that sys.prefix is a directory, and then clean up sys.prefix/TESTFN2.

The skip_if_no decorator is not absolutely necessary and could have been skipped. I believe it adds to the readability of the code because with the decorator it becomes obvious that the test should skip in some cases. Perhaps this is what import_module() is for - if so, should I document it? I also believe the decorator helps prevent cases where a resource is allocated (like creating a directory), then the import_module() call fails and a test artifact is left laying around on disk. Having said that, I do not know if this actually happens in any of the tests and so might be a moot point.

In reference to disliking the naming of skip_if_no(), I do not like the naming either. The decorator attempts to import the module, then raises SkipTest if there was an ImportError. I think it is essential to have the words "import" and "skip" in the method name to help indicate what the decorator does. These are names I could live with:

import_or_skip_test('threading')
import_module_or_skip_test('threading')
skip_test_unless_import('threading')

My preference is for the last one. Let me know which one you like best and I'll change the name to that.

I haven't reviewed the whole patch, but I would suggest that instead of making a test_support.skip_if_no (or any other name), you use use @unittest.skipUnless(threading) (note that lack of quotes around threading...you seem to already be getting it defined correctly in all the modules I scaned). That's what other test modules do in similar situations.

In the test_xmlrpc.py case I changed the value from URL to 'http:' because
the code that sets URL to a valid url relies on threading. When threading
is disabled, URL is set to None and the test will fail.

I would prefer something like:

if support threads:
   url = URL
else:
   url = "http:"

To avoid modifying the current tests in the most common case, Python with
threads.

In the test_macostools.py case, the os.unlink(TESTFN2) call is a copy and
paste error from the previous test (...)

Is this fix related to threads? If not, please open a different issue with a
patch just to fix that.

The skip_if_no decorator is not absolutely necessary and could have been
skipped. I believe it adds to the readability of the code because with
the decorator it becomes obvious that the test should skip in some
cases. Perhaps this is what import_module() is for - if so, should I
document it? (...)

I like your decorator, but not it's name :-)

import_module() is not a decorator, but your decorator call it, so it's fine.

These are names I could live with:

import_or_skip_test('threading')
import_module_or_skip_test('threading')
skip_test_unless_import('threading')

The main goal is to skip a test, so a name starting with "skip" is a good
idea. You might drop "test_" because we already have a context (namespace):
the function is defined in the test_support module.

@test_support.skip_unless_import('thread') doesn't look bad :-)

--

Comments about your new patch.

Lib/ctypes/test/test_errno.py is completly skipped whereas only half the tests
depends on threads.

I don't really like the idea of skipping the whole file if threads are
disabled (except for tests only based on tests, like test_threading.py). If
someone adds a new test without reading the imports header, he will not notice
that the test will be skipped if threads are not available, even if his test
doesn't depend on threads. Since Python 2.7 and 3.0+ supports class
decorators, why not using a decorator on classes instead of the blacklisting
the whole file? I don't know if you decorator can be used on a class without
any change. You need maybe to add **kw to the wrapper() function.

Lib/test/test_sqlite.py: only Lib/sqlite3/test/dbapi.py depends on threads,
all other tests should work without thread.

Lib/test/test_urllib2_localnet.py shouldn't inherit from BaseTestCase because
it overrides all methods (setUp and tearDown), but simply from
unittest.TestCase. This is not directly related to your patch.

reap_threads(): you should move the "import thread" to do it only once, and
write a dummy decorator if threads are disabled. Something like:

if have threads:
  def reap_threads(func):
    ...
else:
  def reap_threads(func):
    # do nothing
    return func

And write a doc string to explain that the function does nothing if threads
are disabled.

fork_wait.py, test_bz2.py: the decorator is maybe useless here (import_module
should be enough). Or do you suggest it for the readability?

Lib/test/test_multiprocessing.py: I guess that some tests can be executed
without thread (only testcases_threads require threads?).

Lib/test/test_capi.py: because of import_module('threading'), TestPendingCalls
will be skipped whereas TestPendingCalls.test_pendingcalls_non_threaded()
works without threads.

Lib/test/test_hashlib.py can simply use @reap_threads and import_module() in
test_threaded_hashing() (instead of using a try/except at the beginning of the
file).

...

I'm too tired to re-read the whole patch. I think that you understood my
ideads, and I don't have to detail changes on each file.

Can you write a new version of the patch?

Uploaded a new version of the patch, nothreads_3.patch.

@r.david.murray - Good point about unittest.skipUnless, I didn't know about that function. I removed my decorator and used skipUnless() instead.

@Haypo:
test_xmlrpc.py - Modified the URL changes so that now the tests should run exactly as before in the multithreaded case.
test_macostools.py - The TESTFN changes are unrelated to the work I am doing. I removed them and will file as a separate item in Roundup.

The skip_if_no decorator has been removed and replaced with unittest.skipUnless().

test_errno.py - You mentioned that only half the tests use threads. The file has two test methods, both of which use threads.
test_sqlite.py - Fixed so that only tests that require threading are skipped. Thank you for the reminder about class decorators.
test_urllib2_localnet.py - I will create an issue in the bug tracker to fix the usage of BaseTestCase. From the svn revision lot it appears the functionality in BaseTestCase is there for a reason and I think it should be fixed.
test_support.py:

• The thread module is now imported once at the top of test_support.
• The reap_threads() function decorates with a thread cleanup function if thread is available, or with a no-op function if thread is not available. Added a docstring.
• Removed "import thread" calls in threading_setup() and threading_cleanup(). Replaced with "if thread" statements.
  fork_wait.py - I removed the decorator and use import_module() instead.
  test_bz2.py - Modified the import so all but one of the tests run when threading is disabled.

test_multiprocessing.py - Modified to use the try: import threading method of importing the module. It's kind of a moot point because the multiprocessing module does not exist when python is built without threading.
test_capi.py - test_pendingcalls_non_threaded eventually makes a call to testcapi._pending_threadfunc(). testcapi._pending_threadfunc() does not exist when python is built without threading. What I'm trying to say is that both tests rely on threading. I added the skipUnless decorator to both test methods.
test_hashlib.py - Switched to use skipUnless instead.

I think this issue is going to reoccur every time someone adds a unit test that relies on threading. What do you think about using a buildbot slave to run the tests with a non-multithreaded build of python?

I think it is a good idea, but someone has to set one up.

+1 to setup a buildbot, but only after all bugs are fixed :-)

I'm applying fixes one by one in my local git-svn repository. I will commit them (in one huge patch \o/) when I'm done, or maybe post a new version of the patch (if I'm too scared of the patch).

I think the latest (v3) patch is in pretty good shape. You and David have been through the changes and the result is a much improved set of changes. If you want (and you're okay with it), I can commit the changes from here and take the beating if it makes the buildbots fall over. :)

I'll break up the patch into multiple files as well. It will make it easier to deal with if I cause a regression.

I think the latest (v3) patch is in pretty good shape. (...)
I'll break up the patch into multiple files as well.
It will make it easier to deal with if I cause a regression.

I'm already doing that (apply the patch file by file). The patch v3 is not perfect, some tests are still disabled whereas they can be executed without tests. Give me a few days to work on a new version of the patch.

Ok, here is my patch "version 4", based on jerry's nothreads_3.patch. Changes between version 3 and 4:

• test_support: threading_setup() returns (1,) instead of None; reap_threads()
  simply returns the function instead of a dummy decorator
• regrtest.py: display an error if -j/--multiprocess option is used
• ctypes/test/test_errno.py, test_smtplib, test_socket, test_asynchat: skip
  some tests, not the whole file
• test_logging, test_xmlrpc, test_file2k: move decorator from setUp() method
  to the class
• test_capi: test threading variable value instead of the presence of the
  _test_thread_state function
• test_capi, test_contextlib: move the decorator from methods to the class
• test_multiprocessing: import threading (without try/except) after
  _multiprocessing because _multiprocessing depends on threading and so it
  will fail

All tests pass on both builds: with and without threads.

I'm not sure that my changes moving decorators from the setUp() method or other methods to the class is the right thing to do.

FYI tests skipped because of the missing thread module are listed in "xx skips unexped on linux2: ...". I don't think that it's a problem. We discuss about that on IRC and bitdancer suggested to leave regrtest.py unchanged:

"bitdancer> You should *definitely* not disable the skip message just because the threading module is missing, that would defeat the whole purpose of the message."

Did someone notice that on Windows, subprocess imports threading, and use threads for the communicate() method?

Did someone notice that on Windows, subprocess imports threading,
and use threads for the communicate() method?

No, I didn't ran the tests on Windows. I can expect that someone build Python on Linux without threads, but is Python used in embedded systems with Windows but without threads?

Since the patch is already huge, I would suggest to open a new issue (fix tests for Windows without threads).

I commited the patch into small parts:

1 (r80552): fix test_support.py for Python compiled without thread
2 (r80553): regrtest.py -j option requires thread support
3 (r80554): test_doctest: import trace module in test_coverage()
4 (r80555): skip test_multiprocessing if thread support is disabled
5 (r80556, r80569): split Test.test_open() of ctypes/test/test_errno.py
6 (r80564): fix test_hashlib for missing threading module
7 (r80565): simplify threading detection in test_capi
8 (r80566): don't skip the whole test_asynchat if threading is missing
9 (r80568): fix test_xmlrpclib for missing threading module
10 (r80570): test_cmd imports trace module using test_support.import_module()
11 (r80571): fix many tests if thread support is disabled

I created the patch to Jerry Seutter in the Misc/NEWS file.

I'm waiting for the buildbot tests to port the commits to py3k. First commit of Part 5 (r80556) was wrong: I fixed it with a new commit (r80569).

Ported to py3k (r80600), blocked in 2.6 (r80602) and 3.1 (r80601).

Victor, I think one more skip is required in test_socketserver.

I think one more skip is required in test_socketserver.

Fixed: 2.7 (r81543), 3.2 (r81545), 3.1 (r81546). Blocked in 2.6 (r81544).