| msg85770 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2009-04-08 15:43 |
| Looking at the tests it seems that the pure-Python paths of json are partly untested. In particular, py_make_scanner (as oppose to c_make_scanner). |
|
|
| msg85771 - (view) |
Author: Bob Ippolito (bob.ippolito) * |
Date: 2009-04-08 15:51 |
| Is this high priority? The pure-Python code paths don't even run in cpython. I test them manually with simplejson by just deleting the extension and then running the tests again. There doesn't seem to be a very good way to do this sort of thing |
|
|
| msg85772 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2009-04-08 16:01 |
| > Is this high priority? The pure-Python code paths don't even run in > cpython. I test them manually with simplejson by just deleting the > extension and then running the tests again. There doesn't seem to be a > very good way to do this sort of thing The main reason I've put it as "high priority" is that right now I'm porting the new json to py3k, and I can't know whether the pure Python paths are ported correctly. That probably won't refrain us from committing it, especially if you say that they are never run with CPython. |
|
|
| msg85773 - (view) |
Author: Walter Dörwald (doerwalter) * |
Date: 2009-04-08 16:04 |
| test_quopri has a decorator that calls a test using both the C and Python version of the tested function. This decorator looks like this: def withpythonimplementation(testfunc): def newtest(self): # Test default implementation testfunc(self) # Test Python implementation if quopri.b2a_qp is not None or quopri.a2b_qp is not None: oldencode = quopri.b2a_qp olddecode = quopri.a2b_qp try: quopri.b2a_qp = None quopri.a2b_qp = None testfunc(self) finally: quopri.b2a_qp = oldencode quopri.a2b_qp = olddecode newtest.__name__ = testfunc.__name__ return newtest Adding such a decorator to every test method might solve the problem. |
|
|
| msg85774 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2009-04-08 16:07 |
| It is a priority because we need solid test coverage in order to successfully port 2.7 to 3.1 without breaking code or changing semantics. The original 3.0 port was done badly. |
|
|
| msg85837 - (view) |
Author: Bob Ippolito (bob.ippolito) * |
Date: 2009-04-10 00:50 |
| I don't think the decorator approach would work for the doctests, it looks like it could be an interesting approach though. I have a feeling that it's going to have to be done in some kind of ugly subclass though, I'll dig into unittest deeper this weekend to see how that might be done. |
|
|
| msg85843 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2009-04-11 09:09 |
| Hi, > I don't think the decorator approach would work for the doctests, it looks > like it could be an interesting approach though. I have a feeling that > it's going to have to be done in some kind of ugly subclass though, I'll > dig into unittest deeper this weekend to see how that might be done. Doctests will be annoying indeed. I never use doctests so I can't suggest you anything. As for standard unit tests, the common idiom is something like: class JSONEncodingTests: def test_encode1(self): self.assertEquals(self.encode("foo"), "bar") # etc. class CJSONEncodingTests(JSONEncodingTests, unittest.TestCase): encode = json.c_encode class PyJSONEncodingTests(JSONEncodingTests, unittest.TestCase): encode = json.py_encode (I'm CC'ing you since bugs.python.org looks down) Regards Antoine. |
|
|
| msg110101 - (view) |
Author: Fred Drake (fdrake) |
Date: 2010-07-12 15:48 |
| This lack of tests is an issue for Python 2.6 as well. Issue 9233 might have been avoided were the pure-Python implementation tested. |
|
|
| msg133644 - (view) |
Author: Ezio Melotti (ezio.melotti) * |
Date: 2011-04-13 07:18 |
| Some tests for py_make_scanner have been added in c3ad883b940b. I agree that having the tested method as an attribute of the class and changing it on a different subclass is the best approach, but it's not currently done by the json tests. Do you think the test should be refactored to use this approach? This will also make easier to skip _json-specific tests when _json is not available and for other Python implementations. |
|
|
| msg133932 - (view) |
Author: Bob Ippolito (bob.ippolito) * |
Date: 2011-04-17 17:01 |
| I did this some time ago in simplejson by defining a TestSuite subclass and instrumenting simplejson so that speedups can be enabled and disabled easily with a private API. https://github.com/simplejson/simplejson/blob/master/simplejson/tests/__init__.py |
|
|
| msg135817 - (view) |
Author: Ezio Melotti (ezio.melotti) * |
Date: 2011-05-12 05:45 |
| Attached patch refactors the tests to use import_fresh_module and different subclasses for Python and C tests. It also includes a fix to import_fresh_module to make it work with packages (it can be committed separately). |
|
|
| msg135826 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2011-05-12 11:31 |
| Comments: - I don't like the fact that skip_unless_cjson() uses unittest internals. Why can't you write something like: skip_unless_cjson = skipUnless(...) - instead of "self.mod", "self.json" would be nicer - you could also export "self.loads", "self.dumps" for easier access - you could also have two base classes exporting all this instead of repeating the attribute-setting for every test class |
|
|
| msg135858 - (view) |
Author: Ezio Melotti (ezio.melotti) * |
Date: 2011-05-12 20:02 |
| > Why can't you write something like:skip_unless_cjson = skipUnless(...) This indeed works -- using unittest internals was just a temporary workaround because the example in the unittest doc didn't seem to work. > - instead of "self.mod", "self.json" would be nicer I thought about using self.json, but then opted for 'mod' because is what the other modules seem to use, but I will fix it. > - you could also export "self.loads", "self.dumps" for easier access Usually they are not called more than a couple of times for each test, and each test class usually has 1-2 tests methods, so I'm not sure it's worth it. - you could also have two base classes exporting all this instead of repeating the attribute-setting for every test class I considered this too, but since the C test classes currently inherit from the Python classes, the C base class would have to be a mixin that overrides the effect of the Python base class -- unless I move all the tests in separate base classes and create two separate subclasses for each C/Python test that inherit from the base test classes and either the C or Python base classes. So the two base test classes will be in __init__: class CTest(TestCase): self.json = cjson; self.loads = cjson.loads; ... class PyTest(TestCase): self.json = pyjson; self.loads = pyjson.loads; ... and the other test files will use either: class TestPySomething(PyTest): def test_something(self): ... class TestCSomething(TestPySomething, CTest): pass or: class TestSomething(TestCase): def test_something(self): ... class TestPySomething(TestSomething, PyTest): pass class TestCSomething(TestSomething, CTest): pass Another option is to have a single base class that sets self.loads/dumps in the __init__ but that will still require the module to be set in the subclasses, something like: class JsonTestCase(TestCase): def __init__(self): self.loads = self.json.loads self.dumps = self.json.dumps and then use: class TestPySomething(JsonTestCase): json = pyjson def test_something(self): ... class TestCSomething(TestPySomething): json = cjson I'm not sure any of these options is better than what we have now though. |
|
|
| msg135872 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2011-05-12 21:56 |
| > class TestSomething(TestCase): > def test_something(self): ... > class TestPySomething(TestSomething, PyTest): pass > class TestCSomething(TestSomething, CTest): pass I was thinking about that. That looks clean and explicit to me. |
|
|
| msg135880 - (view) |
Author: Ezio Melotti (ezio.melotti) * |
Date: 2011-05-12 22:35 |
| With this approach is necessary to exclude the base class from the tests, either by listing all the Python/C tests explicitly or doing some automatic check to find these base classes. Listing all the tests is a bad idea because it needs to be updated manually and it's easy to forget about that and end up with tests that are never run. Checking and skipping the base classes is not very elegant IMHO. It also requires an extra base class, and even if it's more flexible because it makes possible to add Python-specific tests easily, that's not necessary with json because all the tests run unchanged on both pyjson and cjson. |
|
|
| msg135881 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2011-05-12 22:47 |
| > With this approach is necessary to exclude the base class from the > tests, either by listing all the Python/C tests explicitly or doing > some automatic check to find these base classes. It just needs a small change then: class PyTest(TestCase): ... class CTest(TestCase): ... class TestSomething: def test_something(self): ... class TestPySomething(TestSomething, PyTest): pass class TestCSomething(TestSomething, CTest): pass |
|
|
| msg135883 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2011-05-12 23:23 |
| My usual pattern (adopted from examples in the stdlib tests) is this: TestSomethingBase: tests PyTestSomething(TestSomethingBase, TestCase): stuff CTestSomething(TestSomethingBase, TestCase): stuff Is there a reason that won't work in your case? |
|
|
| msg135884 - (view) |
Author: Ezio Melotti (ezio.melotti) * |
Date: 2011-05-12 23:32 |
| Technically they both work, they are just two different approaches that offer more or less the same compromise between features and verbosity. Your approach requires an extra class for each test but saves you from setting the module attribute and the skip, mine is the other way around. |
|
|
| msg135889 - (view) |
Author: Ezio Melotti (ezio.melotti) * |
Date: 2011-05-13 05:39 |
| Attached patch uses the approach described in . |
|
|
| msg135952 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2011-05-14 03:53 |
| New changeset 5b0fecd2eba0 by Ezio Melotti in branch '2.7': #5723: Improve json tests to be executed with and without accelerations. http://hg.python.org/cpython/rev/5b0fecd2eba0 New changeset c2853a54b29e by Ezio Melotti in branch '3.1': #5723: Improve json tests to be executed with and without accelerations. http://hg.python.org/cpython/rev/c2853a54b29e New changeset 63fb2b811c9d by Ezio Melotti in branch '3.2': #5723: merge with 3.1. http://hg.python.org/cpython/rev/63fb2b811c9d New changeset afdc06f2552f by Ezio Melotti in branch 'default': #5723: merge with 3.2. http://hg.python.org/cpython/rev/afdc06f2552f |
|
|