| msg75373 - (view) |
Author: Armin Rigo (arigo) *  |
Date: 2008-10-30 17:02 |
| This patch contains a first step towards classifying CPython tests into language tests and implementation-details tests. The patch is against the 2.7 trunk's test_descr.py. It is derived from a similar patch that we wrote for the 2.5's test_descr.py in order to make it pass on PyPy. The main new feature introduced here is a couple of helpers in test_support - see comments and docstrings in the patch. The main ones are "check_impl_detail", which is a flag which is False on non-CPython hosts; and "impl_detail", a decorator to skip whole functions based on the "check_impl_detail" flag. If this patch is accepted, then we plan to port many more of PyPy's patches for core tests, as a step towards helping non-CPython implementations to obtain a good test suite. |
|
|
| msg75377 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2008-10-30 18:42 |
| Based on your experience, Armin, is it worth having a class decorator as well/instead? The other comment I have is whether impl_detail is really the best name. Would something like cpython work out better to be more obvious that the test is specific for CPython? Otherwise would a naive PyPy use not understand that the impl_detail tests are not meant for PyPy? |
|
|
| msg75379 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2008-10-30 20:42 |
| As I mentioned on Python-dev, I have implemented something similar to this in my testing branch. [1] http://code.python.org/users/benjamin.peterson/new_testing/main |
|
|
| msg75390 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2008-10-30 22:32 |
| I personally wonder if we should be moving towards a more systematic means of identifying the underlying Python VM than the current fairly ad hoc use of sys.platform. By that I mean adding another sys attribute (e.g. sys.vm) which could be checked explicitly for the Python VM identity, rather than the underlying platform. E.g. CPython: sys.vm == "cpython" Jython: sys.vm == "jython" PyPy: sys.vm == "pypy" IronPython: sys.vm == "ironpython" Then instead of relying on a separate flag in test_support the impl_detail decorator could be written based on the VM names: def impl_detail(*vm_names): if not vm_names: vm_names = "cpython", if sys.vm in vm_names: # Test the implementation detail else: # Skip this test Depending on how ambitious an implementer of an alternative VM wants to be, they could either set sys.vm to the same value as one of the existing interpreters and try to match the implementation details as well as the language spec, or else use their own name. |
|
|
| msg75391 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2008-10-30 22:34 |
| On Thu, Oct 30, 2008 at 5:33 PM, Nick Coghlan <report@bugs.python.org> wrote: > > Nick Coghlan <ncoghlan@gmail.com> added the comment: > > I personally wonder if we should be moving towards a more systematic > means of identifying the underlying Python VM than the current fairly ad > hoc use of sys.platform. I use platform.python_implementation(). |
|
|
| msg75397 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2008-10-30 23:03 |
| Interesting, I hadn't noticed that addition to the platform module for 2.6. A bit more verbose than sys.vm, but it would certainly do the trick :) In that case, I would suggest something along the lines of the following: vm = platform.python_implementation().lower() reference_vm = "cpython" def impl_detail(*vm_names): if vm_names: vm_names = [vm.lower() for vm in vm_names] else: vm_names = [reference_vm] if vm in vm_names: # Test the implementation detail else: # Skip this test |
|
|
| msg75415 - (view) |
Author: Armin Rigo (arigo) * |
Date: 2008-10-31 13:44 |
| Brett: in my experience the granularity is usually fine, and not coarse. A class decorator doesn't look too useful. A function decorator is useful, but not enough. We also need a flag that can be checked in the middle of a larger test. (See the patch for test_descr for many examples of this use case.) Nick: your impl_detail() decorator looks fine to me (except that I think it should also accept an optional reason=... keyword argument). Based on it, the way to skip only a few lines in a larger test should be with a similar helper check_impl_detail(*vm_names) which returns True or False. I agree that "impl_detail()" wasn't the best name originally, but in Nick's proposed approach, "impl_detail()" sounds like exactly the right name. I also like Nick's approach because it means that in the various little cases where, for some elegance argument, CPython is really wrong, then it can be "officialized" by writing a test that is skipped on CPython :-) |
|
|
| msg75434 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2008-10-31 23:08 |
| My idea above won't really support Armin's idea of being able to exclude certain known-broken implementations. The check function would need to be handled differently to support that use case: # In test_support.py vm = platform.python_implementation().lower() reference_vm = "cpython" def check_impl_detail(implemented=(reference_vm,), broken=()): # Skip known broken implementations if broken: broken = [vm.lower() for vm in broken] if vm in broken: return False # Only check named implementations if implemented: implemented = [vm.lower() for vm in implemented] return vm in implemented # No specific implementations named, so expect it to # work on implementations that are not known to be broken return True def impl_detail(implemented=(reference_vm,), broken=(), msg=''): if check_impl_detail(implemented, broken): # Test the implementation detail else: # Skip this test (incude 'msg' in the skip message) It would be pretty easy to build cpython_only, jython_only, pypy_only etc checks and decorators on top of that kind of infrastructure. |
|
|
| msg79744 - (view) |
Author: Armin Rigo (arigo) * |
Date: 2009-01-13 14:31 |
| Here is a summarizing implementation that accepts this interface: if check_impl_detail(): # only on CPython (default) if check_impl_detail(jython=True): # only on Jython if check_impl_detail(cpython=False): # everywhere except on CPython and similarly with the decorator: @impl_detail() # only on CPython (default) @impl_detail("reason...") # the same, with an explicit message @impl_detail(jython=True) # only on Jython @impl_detail(cpython=False) # everywhere except on CPython I think this is a nice interface, although it takes some largish number of lines to implement. |
|
|
| msg79760 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2009-01-13 18:36 |
| At the language summit I actually plan on proposing separating out the Python the language and standard library from CPython. That would make this patch mostly unneeded as the CPython-specific tests and code would simply be kept separate from the language code that PyPy and other VMs would use. Because of this I am not going to do any code review right now in hopes that my more radical proposal goes through. |
|
|
| msg79773 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2009-01-13 20:08 |
| Physically splitting the code base? Ick... I'd prefer just to flag the parts of the test suite that are optional and let the developers of other implementations pick and choose as to how much of the pure Python code they want to adopt to pass the non-optional parts of the test-suite... |
|
|
| msg79895 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2009-01-15 12:14 |
| The patch looks nice to me. (I'm curious why you call gc.collect() several times in a row, though) However, since it is an important change in the long run, perhaps the specifics could be further discussed on python-dev before taking a decision? |
|
|
| msg80266 - (view) |
Author: Stefan Behnel (scoder) * |
Date: 2009-01-20 19:49 |
| I would definitely appreciate having a well-defined set of "required tests" that Cython should pass for compliance. However, something like sys.vm won't easily work for Cython: it runs within the CPython VM but only after converting the Python code to C. Emulating platform.python_implementation() to make it return "Cython" does not sound correct. You can currently detect Cython compilation by doing this: import cython print cython.compiled Obviously, the import will fail when running as Python code without having Cython installed. However, in Cython, you would often get a compile time error in cases where implementation details apply, so checking for implementation details programmatically may not work at all. |
|
|
| msg80270 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2009-01-20 20:24 |
| Would a C API in CPython to set the value returned by sys.vm be enough to help Cython out Stefan? Such a feature would help with any CPython based variant - the implementers could either leave sys.vm as "cpython" and attempt to be fully compatible, or else set it to something different (e.g "stackless" or "cython") and target the common subset of the test suite. |
|
|
| msg80271 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2009-01-20 20:32 |
| Why would Cython be affected? This is about tests of the stdlib, which have nothing to whether you use Cython or not. |
|
|
| msg80275 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2009-01-20 20:46 |
| I got the impression from Stefan's question that he would like to be able to run the stdlib tests with Cython enabled for all of the stdlib Python modules and know which tests still needed to pass and which could be safely skipped as being specific to vanilla CPython. Without some way to change the value of sys.vm (either from Python or from C), that kind of thing wouldn't be possible. |
|
|
| msg80285 - (view) |
Author: Stefan Behnel (scoder) * |
Date: 2009-01-20 22:06 |
| @Antoine: Cython already compiles a major part of CPython's test suite successfully (although we didn't actually try to compile the stdlib itself but only the tests). It's an announced goal for Cython 1.0 to compile 'all' Python code, and it would be great to have an official subset of the test suite that would allow us to prove compliance and to know when we're done. Thinking about this some more, I guess that fairness would require us to also compile the pure Python modules in the stdlib that are being tested. I really like that idea. That would allow a Cython-enabled CPython to compile its entire stdlib into fast extension modules and to ship them right next to the pure Python code modules, so that people could still read the Python code that gets executed at C speed. Looks like all we'd need to do is to install a global import hook for .py files (but that's definitely off-topic for this bug). @Nick: It's not a technical problem. We could special case sys.vm in Cython by simply replacing it by a constant string when we find it in the source tree (that should do for 'normal' usage, although "getattr(sys, vm_string)" won't work). Being able to change the value of sys.vm programmatically isn't a good solution, as it would affect all Python code in the VM, not only code that was compiled by Cython. I'm more concerned about the semantics. It wouldn't be correct to tell code that it runs in Cython when it was actually interested in the *VM* it runs in. But some things might still behave different in Cython than in CPython, due to static compilation, exception handling nuances, the placement of reference counting calls, etc. The information about the running VM isn't enough here, whereas "platform.python_implementation()" makes at least a bit more sense by its name. The main problem seems to be that Cython has some specialties in its own right, while it inherits others from CPython. So code that branches based on "platform.python_implementation()" must be aware of both. There will definitely be cases where CPython will work but Cython compilation won't (and maybe even vice versa :) |
|
|
| msg80286 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2009-01-20 22:15 |
| > @Antoine: Cython already compiles a major part of CPython's test suite > successfully (although we didn't actually try to compile the stdlib > itself but only the tests). It's an announced goal for Cython 1.0 to > compile 'all' Python code, and it would be great to have an official > subset of the test suite that would allow us to prove compliance and to > know when we're done. Wow! I guess I was still living in the Pyrex era... That's an impressive achievement. So, let me retract what I said about Cython not being relevant to this issue. |
|
|
| msg80312 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2009-01-21 02:19 |
| Like Brett, I think the long term solution is to segregate implementation-specific tests into a separate file or subdirectory of files. Then the main directory of tests could (and I would like) constitute an executable definition-by-example for the language. (To aid this, I would also like the naming of files and tests and sequencing of tests within files to reflect the structure of the manual as much as possible -- and would help to make it so. Separate patches of course.) Would alternative implementors prefer to wait or have a *temporary* addition to test_support? If something is added, I would give it a leading underscore name and document it as something probably temporary for alternate implementations to use until a permanent solution is implemented. |
|
|
| msg80315 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2009-01-21 02:51 |
| On Tue, Jan 20, 2009 at 18:19, Terry J. Reedy <report@bugs.python.org> wrote: > > Terry J. Reedy <tjreedy@udel.edu> added the comment: > > Like Brett, I think the long term solution is to segregate > implementation-specific tests into a separate file or subdirectory of > files. Then the main directory of tests could (and I would like) > constitute an executable definition-by-example for the language. (To > aid this, I would also like the naming of files and tests and sequencing > of tests within files to reflect the structure of the manual as much as > possible -- and would help to make it so. Separate patches of course.) > > Would alternative implementors prefer to wait or have a *temporary* > addition to test_support? Well, if the idea of breaking out the language stuff into its own repository happens, then the tests will have to be crawled through anyway so decorating now to act as a marker of what has to be separated out shouldn't lead to too much extra work. > > If something is added, I would give it a leading underscore name and > document it as something probably temporary for alternate > implementations to use until a permanent solution is implemented. Well, this might be the permanent solution. Plus the entire test directory tends to be somewhat optional thanks to the Linux distros leaving it out most of the time so even if it is put in there they can just not document it. |
|
|
| msg84208 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2009-03-26 20:01 |
| Ok, I applied part of Armin's patch in r70615 modified to work with unittest's new test skipping ability. I think I will apply the test_descr part later. |
|
|
| msg84211 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2009-03-26 20:17 |
| Second patch applied in r70617. |
|
|