[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-06-24 Thread Eric Snow
Eric Snow added the comment: The gains here aren't worth the hassle in my mind, and I've lost interest in pushing this forward any further. If anyone else wants to pick this up, feel free to re-open the issue. -- assignee: eric.snow -> resolution: -> rejected status: open -> closed

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-04-16 Thread Zachary Ware
Changes by Zachary Ware : -- nosy: +zach.ware ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.pytho

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-02-07 Thread Eric Snow
Eric Snow added the comment: Fine with me. -- ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.pytho

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-02-07 Thread Brett Cannon
Brett Cannon added the comment: At this point let's just start with the helper class which takes the arguments as necessary to do the proper importing of both the pure Python and accelerated versions of the module and optionally the name of the attribute the test classes expect (otherwise just

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-02-06 Thread Eric Snow
Eric Snow added the comment: First of all, I agree that the API for import_fresh_module() isn't ideal and could stand some improvement. I've created issue 17146 for pursuing that avenue. More importantly for this issue, simplifying import_fresh_module() is not nearly enough of a win for me r

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-02-06 Thread Eli Bendersky
Eli Bendersky added the comment: On Wed, Feb 6, 2013 at 4:17 AM, Ezio Melotti wrote: > > Ezio Melotti added the comment: > > I'm still a bit skeptic about this. > > To summarize my position, I am: > +0 about adding something like this to test.support and use it for new > tests; > -1 about mass-

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-02-06 Thread Ezio Melotti
Ezio Melotti added the comment: I'm still a bit skeptic about this. To summarize my position, I am: +0 about adding something like this to test.support and use it for new tests; -1 about mass-rewriting the existing tests to use this (in particular for test_json); +1 about simplifying the API of

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-02-06 Thread Eric Snow
Eric Snow added the comment: Any objections to what Brett proposed? Any preferences on a name? How about DualImplementationTests? Here's a patch with tests. I added a pure_python_only() decorator in addition to accelerated_only(). I also made both work as decorators for classes and methods

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-01-28 Thread Eric Snow
Eric Snow added the comment: +1 on requires_accelerator(). It could also be used for individual test methods. Similar decorators-as-methods could be added later, where appropriate, for other special cases, like handling the pickle situation described in #16817. -- _

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-01-28 Thread Eric Snow
Eric Snow added the comment: FWIW, here's a little more explanation on my original thoughts, none of which I'm married to. Most of the magic in my patch came messing with the globals (swap out the module and module attrs; add in the two new classes; ditch the original class). That was a cons

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-01-28 Thread Eric Snow
Eric Snow added the comment: +1 to where Brett's taking this. I really like having the PEP399Tests class encapsulating the various boilerplate parts, with some of them as methods, rather than trying to pack them all into one all-powerful decorator. It would be worth raising an exception in `

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-01-28 Thread Brett Cannon
Brett Cannon added the comment: True, the current idiom needs to still be used in those cases, although we could introduce another method to help with this situation as well: # Could also be named use_accelerator to be less hostile-sounding. def requires_accelerator(self, cls): if self.accele

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-01-27 Thread Eli Bendersky
Eli Bendersky added the comment: etree's doctest tests are a painful relic of the past, which will hopefully go away one day (issue #15083), at which point conformance to PEP 399 shouldn't be an issue. More generally, and this applies to etree's test too, a lot of trouble is being caused by p

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-01-27 Thread Ezio Melotti
Ezio Melotti added the comment: Some of the exceptions among the modules you listed are: Lib/test/test_warnings.py:420 has a class specific for the C module. Lib/test/json_tests/test_dump.py:34 has an additional test specific for the C module. Lib/test/json_tests/test_speedups.py is specific f

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-01-27 Thread Brett Cannon
Brett Cannon added the comment: I took a closer look at test_datetime and it probably should be using this approach, but because the tests seemed to have been copied from Zope the tests were left as-is and instead the testing approach we have been using was actually forced upon the test module

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-01-27 Thread Brett Cannon
Brett Cannon added the comment: To prevent speculation, the files that mention import_fresh_module are: # Without changes to test classes Lib/test/test_bisect.py: would work if you made the attribute configurable (e.g. self.module instead of self.bisect Lib/test/test_heapq.py: would work Lib/te

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-01-27 Thread Ezio Melotti
Ezio Melotti added the comment: >> This is also not so uncommon. I think the skip on the accelerated >> version might not be necessary in some situations. > >Then don't use this approach. It doesn't have to apply to every single >test >class in a module. It just needs to help in the simple case

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-01-27 Thread Brett Cannon
Brett Cannon added the comment: On Sun, Jan 27, 2013 at 3:13 PM, Ezio Melotti wrote: > > Ezio Melotti added the comment: > > Note that while this works for the simple case, it doesn't work whenever > you need additional tests or attributes that are specific for one of the > two classes, Sure it

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-01-27 Thread Ezio Melotti
Ezio Melotti added the comment: That would work if we use something Brett's proposal, so that we can create a default setUp that raises SkipTest. My comment was about using module = import_fresh_module() directly in the class body of the subclasses as they are currently documented in PEP 399.

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-01-27 Thread Eli Bendersky
Eli Bendersky added the comment: > That wouldn't work if there is a skip decorator that checks that the C module is available :) Yes, but that's easily amended with just raising SkipTest in the setUp instead. Not polluting the global namespace has its merits, especially when the import sequence

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-01-27 Thread Antoine Pitrou
Antoine Pitrou added the comment: FWIW, I like Brett's approach much better. Also, I agree with Ezio that "pep399" shouldn't be in the name. -- ___ Python tracker ___ ___

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-01-27 Thread Ezio Melotti
Ezio Melotti added the comment: That wouldn't work if there is a skip decorator that checks that the C module is available :) -- ___ Python tracker ___ _

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-01-27 Thread Eli Bendersky
Eli Bendersky added the comment: Brett - you approach is certainly less magical. While you're at it, how about PEP 399 recommending the import_fresh_modules calls not being in the global context, but rather class/instance attributes of the test cases? This is what both yours and Eric's decorat

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-01-27 Thread Ezio Melotti
Ezio Melotti added the comment: Note that while this works for the simple case, it doesn't work whenever you need additional tests or attributes that are specific for one of the two classes, or when you have additional base classes. This is also not so uncommon. I think the skip on the accel

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-01-27 Thread Brett Cannon
Brett Cannon added the comment: OK, let's take a step back here and look at what exactly we are trying to simplify (which is now the updated example in PEP 399):: from test.support import import_fresh_module import unittest c_heapq = import_fresh_module('heapq', fresh=['_heapq'])

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-01-26 Thread Eric Snow
Eric Snow added the comment: > I think something similar has already been proposed and rejected on another > issue. Wouldn't surprise me. I'll look for it. I know there's been a bunch of traffic relative to PEP 399 and to test discovery, so I'm guessing I missed it in there. Thanks for the

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-01-26 Thread Ezio Melotti
Ezio Melotti added the comment: I think something similar has already been proposed and rejected on another issue. -- nosy: +ezio.melotti ___ Python tracker ___

[issue17037] Use a test.support helper to wrap the PEP 399 boilerplate

2013-01-26 Thread Eric Snow
Eric Snow added the comment: (changing the title to disassociate it from a particular solution) >> * make sure the original test case does not get used (#16835), > > PEP 399 dictates that the base class does not inherit from > unittest.TestCase, so why would it be used? Sorry for any confusion.