test.regrtest considers an ImportError to be a test to skip. It then uses this info to decide what skipped tests were expected (or not) based on a list kepted in regrtest.py.

For detecting compiler failures, an ImportError should be a test error or failure. Tests for optional modules should instead raise TestSkipped directly if an import fails. Something like test.support.optional_import() should be created which raises TestSkipped if the requested module could not be imported. It could also be made optional based on the OS (not sure if it should be inclusive, exclusive, or either). That way the list of expected skips in regrtest.py can be moved into the individual test modules where it belongs.

Once the proper function in test.support comes about then a dev task to help move everything over can be created. And then once all needed test modules have been switched over the ImportError try/except statement in regrtest can be removed.

How is this different from issue 2409?

On second reading I see one way it is different: you suggest to move the list of expected skips out of regrtest.  So, are you suggesting, essentially, that support.import_module be replaced by an optional_import that takes arguments to indicate on which platforms the skip is expected?

Yes. So for _winreg (if we even have tests) it would be skipped on all OSs other than Windows, on on Windows it would be a test failure if it didn't work as it is expected to exist.

Ah, I see what you are getting at now.  I was confused by the "raise SkipTest directly" part, since the test suite currently does raise SkipTest instead of ImportError.  So the key change here is to make the test show as a *failure* on those platforms where it is currently reported as an unexpected skip.  We can then eliminate the whole "these skips are expected" bit from regrtest.  That would be a very nice cleanup.

I'm going to mark this as easy, since it is basically a mechanical transformation from the expected skip list in regrtest to calls to a fairly simple new test.support function, and then ripping the obsolete code out of regrtest.  Tedious, but doable in a day.

Here is a patch which has added the necessary changes to test.support.import_module() and placed the necessary changes in the test suites themselves. I still need to rip out the (un)expected test stuff from regrtest before I consider the patch complete.

Hmm. I'm not sure passing a list of platforms is a good idea. People may want to write e.g. "'bsd' in platform". Also, there are a lot of platforms we don't have access to, so we can't actually maintain a list of platforms.

Also, I really don't like the idea that a missing extension module is a test failure. It can be quite annoying to install all build dependencies (why should people install tcl/tk if they want to hack on e.g. the logging module?), let's not scare people who haven't done so by printing out test failures.

So os.name is also supported. But the point is that if a platform wants to be considered supported then they need to give us a patch to update the tests to make them acceptable to skip.

As for test_ttk and such, those that have a third-party dependency are still optional no matter what. This change is **only** for modules we expect to always build on certain platfoms (e.g., winreg under Windows or crypt on UNIX systems). I mean do we really think ctypes is optional at this point? In general no, only on select platforms where libffi support is lacking. So having the test skip because of a build failure under OS X (like I had for LLVM 2.8 for a while) should not be left behind but instead be considered a failure as ctypes is **supposed** to be supported under darwin. Extension build failure should not always be considered a non-failure.

I should also mention that if the idea of whitelisting failures
doesn't fly, it can always be changed to be a blacklist of failures
(i.e., ditch 'optional' and only use 'required_on'). But I did it this
way to force people to clearly state on what platforms a test was
expected to be skipped on reactively instead of letting the list
slowly degrade as it already has (bunch of stuff inconsistently listed
in regrtest as it is).

On Thu, Feb 10, 2011 at 10:20, Brett Cannon <report@bugs.python.org> wrote:
>
> Brett Cannon <brett@python.org> added the comment:
>
> So os.name is also supported. But the point is that if a platform wants to be considered supported then they need to give us a patch to update the tests to make them acceptable to skip.
>
> As for test_ttk and such, those that have a third-party dependency are still optional no matter what. This change is **only** for modules we expect to always build on certain platfoms (e.g., winreg under Windows or crypt on UNIX systems). I mean do we really think ctypes is optional at this point? In general no, only on select platforms where libffi support is lacking. So having the test skip because of a build failure under OS X (like I had for LLVM 2.8 for a while) should not be left behind but instead be considered a failure as ctypes is **supposed** to be supported under darwin. Extension build failure should not always be considered a non-failure.
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue10966>
> _______________________________________
>

> As for test_ttk and such, those that have a third-party dependency are
> still optional no matter what. This change is **only** for modules we
> expect to always build on certain platfoms (e.g., winreg under Windows
> or crypt on UNIX systems).

Ah, thanks for pointing that out to me. That's certainly better than
what I thought :)

> I mean do we really think ctypes is optional at this point?

Why wouldn't it be? It doesn't offer any essential functionality, and
most applications and libraries certainly don't depend on it.
It's far more optional than zlib (which a bunch of stdlib functionality
relies on) IMO.

What's more, standard C isn't enough to compile ctypes (as you witnessed
with LLVM). So, really, making absence of ctypes a failure sounds too
severe to me.

Well, ctypes failing because it cannot compile is only when a platform that is not listed as optional cannot import it. So if some platform does not support ctypes then it gets added to the list, end of story.

We only support so many platforms as it is. And with the hg transition, if someone maintains a closed-source fork they can easily patch their own version of Python so that test_ctypes is skipped without issue w/o causing us to not notice when ctypes stops compiling on a platform we do support **and** expect it to be available on.

Anyway, I am going to continue to rip out the (un)expected test skip code regardless of how people come down on whether this is too strict or not on a per-test basis as I still think the expected skip list is worthless as-is. We can then discuss this on python-committers (since we have to maintain the test compatibility).

> Well, ctypes failing because it cannot compile is only when a platform
> that is not listed as optional cannot import it. So if some platform
> does not support ctypes then it gets added to the list, end of story.

How do you add "llvm under darwin" (or any similar case, e.g. "Sun CC
under Solaris") to that list of platforms?

You don't. ctypes failing under LLVM 2.8 should not be a special case of skipping; ctypes not building on darwin regardless of whether it is gcc or clang is a failure. If someone uses a compiler we don't support, that's their decision.

One other option is to simply have a whitelist of platforms that test.support knows of so it only considers it a failure when the platform being run on is known (regrtest does this already).

I should mention this would act as a nice testing doc for exactly which platforms CPython considers supported.

Here is the completed patch. It adds required_on and 'optional' args to test.support.import_module() to help delineate if a test should **not** be skipped simply because a module could not be imported. It also changes various tests to use the new args. Finally, the entire concept of (un)expected skips is removed from regrtest.

The patch works fine on Linux but breaks test_pipes and test_sqlite on
Windows:

[1/1] test_pipes
testBadAppendOptions (test.test_pipes.SimplePipeTests) ... ok
testBadOpenMode (test.test_pipes.SimplePipeTests) ... ok
testBadPrependOptions (test.test_pipes.SimplePipeTests) ... ok
testClone (test.test_pipes.SimplePipeTests) ... ok
testEmptyPipeline1 (test.test_pipes.SimplePipeTests) ... 'cat' is not
recognized
 as an internal or external command,
operable program or batch file.
FAIL
testEmptyPipeline2 (test.test_pipes.SimplePipeTests) ... ok
testEmptyPipeline3 (test.test_pipes.SimplePipeTests) ... ok
testQuoting (test.test_pipes.SimplePipeTests) ... ok
testReadOpenSink (test.test_pipes.SimplePipeTests) ... ok
testRepr (test.test_pipes.SimplePipeTests) ... ok
testSetDebug (test.test_pipes.SimplePipeTests) ... ok
testSimplePipe1 (test.test_pipes.SimplePipeTests) ... 'tr' is not
recognized as
an internal or external command,
operable program or batch file.
FAIL
testSimplePipe2 (test.test_pipes.SimplePipeTests) ... The system cannot
find the
 file specified.
ERROR
testSimplePipe3 (test.test_pipes.SimplePipeTests) ... The system cannot
find the
 file specified.
FAIL
testWriteOpenSource (test.test_pipes.SimplePipeTests) ... ok

======================================================================
ERROR: testSimplePipe2 (test.test_pipes.SimplePipeTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "Z:\__svn__\lib\test\test_pipes.py", line 34, in testSimplePipe2
    with open(TESTFN2) as f:
IOError: [Errno 2] No such file or directory: '@test_560_tmp2'

======================================================================
FAIL: testEmptyPipeline1 (test.test_pipes.SimplePipeTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "Z:\__svn__\lib\test\test_pipes.py", line 58, in
testEmptyPipeline1
    self.assertEqual(f.read(), d)
AssertionError: '' != 'empty pipeline test COPY'
+ empty pipeline test COPY

======================================================================
FAIL: testSimplePipe1 (test.test_pipes.SimplePipeTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "Z:\__svn__\lib\test\test_pipes.py", line 26, in testSimplePipe1
    self.assertEqual(f.read(), 'HELLO WORLD #1')
AssertionError: '' != 'HELLO WORLD #1'
+ HELLO WORLD #1

======================================================================
FAIL: testSimplePipe3 (test.test_pipes.SimplePipeTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "Z:\__svn__\lib\test\test_pipes.py", line 44, in testSimplePipe3
    self.assertEqual(f.read(), 'HELLO WORLD #2')
AssertionError: '' != 'HELLO WORLD #2'
+ HELLO WORLD #2

----------------------------------------------------------------------
Ran 15 tests in 0.250s

FAILED (failures=3, errors=1)
test test_pipes failed -- multiple errors occurred

test test_sqlite crashed -- <class 'ImportError'>: No module named _sqlite3
Traceback (most recent call last):
  File "Z:\__svn__\lib\test\regrtest.py", line 948, in runtest_inner
    the_package = __import__(abstest, globals(), locals(), [])
  File "Z:\__svn__\lib\test\test_sqlite.py", line 4, in <module>
    test.support.import_module('_sqlite3', required_on=['win32'])
  File "Z:\__svn__\lib\test\support.py", line 115, in import_module
    return importlib.import_module(name)
  File "Z:\__svn__\lib\importlib\__init__.py", line 124, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "Z:\__svn__\lib\importlib\_bootstrap.py", line 822, in _gcd_import
    raise ImportError(_ERR_MSG.format(name))
ImportError: No module named _sqlite3

New patch which makes test_pipes back into an explicit skip if os.name !+ posix and makes test_sqlite optional on all platforms.

The attached patch has both the code to make test skipping more obvious as well as eliminating the concept of expected skips.

If someone can double-check that what I am doing here is sane and desirable I would appreciate it.

I’m probably the one with the least regrtest knowledge among us, but I like the general idea of moving the compat info from one huge dict into the tests themselves.  It looks more readable and maintainable.

The new tests for test.support looks good.

I have a reputation to maintain, so here are some nits:
- “FS encoding” would be clearer as “filesystem encoding”.
- The argument names “required_on” and “optional” are not symmetrical.
- The indentation is hard to read here:

+pty = import_module('pty',
+            optional=['win32', 'os2emx', 'freebsd4', 'freebsd5', 'freebsd6',
+                        'freebsd7', 'freebsd8'])

In terms of the symmetrical comment, what exactly do you mean? The semantics are opposites of each other. Do you not like the name? Or did I screw up and they truly aren't opposites?

It’s only the names that are not symmetrical (“_on” or not).

> The attached patch has both the code to make test skipping more
> obvious as well as eliminating the concept of expected skips.

I still don't like the idea that we have to hand-maintain lists of
"optional" or "required" platforms. It is not more manageable than the
list of "expected" tests.

For example:

-import_module('_ctypes')
+import_module('_ctypes', optional=['openbsd3', 'netbsd3'])

there are other platforms where _ctypes is not supported. We don't want
to maintain a list of them.

Same for:

+pty = import_module('pty',
+            optional=['win32', 'os2emx', 'freebsd4', 'freebsd5', 'freebsd6',
+                        'freebsd7', 'freebsd8'])

Do you want to keep track of the specificities of each version of the *BSDs?

I'm not sure why you have "optional" and "required_on". Either
"optional" and "required", or "optional_on" and "required_on", would be
more logical (the former looks less verbose to me).

I can change it to 'required' and 'optional'.

As for Antoine's comment, do you have another suggestion? I realize it isn't necessarily easier per se to manage these lists than the 'expected' list, but what would you rather have happen? Simply say tests are skipped because they were not imported and assume people pay enough attention to realize that some compilation error occurred for the modules? That doesn't seem like a much better solution; Nick once didn't realize that there was a compilation issue for an extension as the test was simply skipped to begin with. And when ctypes wouldn't compile under LLVM that was not exactly a test being skipped but a failure (albeit on LLVM's side in that case).

> I can change it to 'required' and 'optional'.
> 
> As for Antoine's comment, do you have another suggestion? I realize it
> isn't necessarily easier per se to manage these lists than the
> 'expected' list, but what would you rather have happen?
> Simply say tests are skipped because they were not imported and assume
> people pay enough attention to realize that some compilation error
> occurred for the modules?

Well, "make" clearly tells you which modules couldn't be built at the
end. Many people are already concerned enough by that message :)

Perhaps we can keep required/optional for core things like "posix" or
"winreg". But failing the test suite because ctypes doesn't compile on a
platform is IMO overblown. Especially given that we also go out of our
way to make the test suite pass successfully when e.g. zlib or threads
are unavailable (or at least Ezio did at one point).

On Thu, Mar 24, 2011 at 15:22, Antoine Pitrou <report@bugs.python.org>wrote:

Sure, but do the buildbots pick up on this fact in some visible way?

> Perhaps we can keep required/optional for core things like "posix" or
> "winreg". But failing the test suite because ctypes doesn't compile on a
> platform is IMO overblown. Especially given that we also go out of our
> way to make the test suite pass successfully when e.g. zlib or threads
> are unavailable (or at least Ezio did at one point).
>

Basically anything that isn't built by setup.py? That's the bare minimum I
would want to go with.

Anyone else have an opinion?

Antoine wrote:
> Do you want to keep track of the specificities of each version of the *BSDs?

Currently regrtest does, but they are currently all set to the same list of tests.  Perhaps a FreeBSD generic that implies all versions, and then if we ever have a test that varies per version we can add support for that :)

Brett, I think I misunderstood what you were proposing.  I agree with Antoine that it doesn't seem sensible to make the test suite *fail* for those cases where regrtest currently just says "unexpected skip".  I am, however, in favor of moving the infrastructure for marking tests as expected/unexpected skips out of regrtest and into the tests themselves.  And the expected skip report could be dropped, given that skip messages are printed during the test run for things other than whole modules.

I rely on that list of unexpected skips at the end of regrtest as part of my "things look like I expect" check.  If I don't have given optional-but-normal modules installed I know which ones they are.

IMHO the current "unexpected" list is fairly pointless.  I just ran the test suite on 3.3 and I got the usual expected list of unexpected skips:
7 skips unexpected on linux2:
    test_bz2 test_dbm_gnu test_dbm_ndbm test_tcl test_tk
    test_ttk_guionly test_ttk_textonly
If these tests were really *expected to run* on my machine, having them skipped would be a failure.  They are instead just a set of tests that I *might be able to run* once I install all the right dependencies.

I think the tests can be classified in 3 categories:
 1) tests that are known to always run everywhere (e.g. test_unittest);
 2) tests that are known to always run on specific platforms and never on others (e.g. test_winreg);
 3) tests that might run on some platforms depending on some specific condition (e.g. test_zlib);

For each of these cases, regrtest should:
 1) fail if the test is not run;
 2) fail if the test is not run on the platform(s) where it's always available, always skip it on the others;
 3) skip the test conditionally without failing (e.g. checking if zlib can be imported or not);

A special case of 3) is when the skip depends on the platform name rather than the availability of some specific module/function (e.g. if  some platforms shipped with a buggy version of a library and there's no way to check if the library is buggy other than keeping a list of platforms that are known to use that version).  In this case the condition should check only for the platform where it's known to fail, and more platforms can be added if/when people report them.

I agree that the logic for this should be moved out of regrtest and added to the specific tests.

It's also possible to provide better skip messages, e.g.:
-test_winreg skipped -- No module named 'winreg'
-test_tk skipped -- No module named '_tkinter'
-test_urllibnet skipped -- Use of the `network' resource not enabled
+test_winreg skipped -- No module named 'winreg' (winreg is available on windows only)
+test_tk skipped -- No module named '_tkinter' (tcl/tk not installed)
+test_urllibnet skipped -- Use of the `network' resource not enabled (run the tests with -unetwork or -uall to enable it)

Well, I'm not so attached to the unexpected skip list that I want to block this from getting implemented.  So I guess the bottom line is that things that are unexpected skips now should not be failures.

+1 to the counter argument that "Unexpected Skips should not be marked as failures".

Also, I like the following proposal of giving better messages on skips
useful.

On Sat, Mar 26, 2011 at 07:53:34AM +0000, Ezio Melotti wrote:
> It's also possible to provide better skip messages, e.g.:
> -test_winreg skipped -- No module named 'winreg'
> -test_tk skipped -- No module named '_tkinter'
> -test_urllibnet skipped -- Use of the `network' resource not enabled
> +test_winreg skipped -- No module named 'winreg' (winreg is available on windows only)
> +test_tk skipped -- No module named '_tkinter' (tcl/tk not installed)
> +test_urllibnet skipped -- Use of the `network' resource not enabled (run the tests with -unetwork or -uall to enable it)

+1 to Ezio’s last message.

Here is a new patch which removes the expected skips stuff and adds a required_on argument to test.support.import_module() for those cases where missing a module is an error (in the patch it's _winreg on Windows).

If people are fine with this cleanup then the question is whether there are other tests that should be required to exist on certain platforms. E.g. should test_epoll and test_kqueue be required on Linux and BSD, respectively? Any other modules people can think of that should be required on a specific platform? If not then perhaps required_on doesn't need to be an iterable or the whole argument is pointless because there are so few use-cases there is no point in adding the support.

I will improve the message as to why a test was failed in a separate patch since regrtest is messy and it's going to take some work.

I'm at PyCon Argentina Friday and Saturday, so I have a rare opportunity to get stuff done. If anyone has issues or comments, please get them in over the next day or so, else I'm going to check this in as-is.

New changeset eb3714863872 by Brett Cannon in branch 'default':
Issue #10966: Remove the concept of unexpected skipped tests.
http://hg.python.org/cpython/rev/eb3714863872