New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Incomplete json tests #49973
Comments
Looking at the tests it seems that the pure-Python paths of json are |
Is this high priority? The pure-Python code paths don't even run in |
The main reason I've put it as "high priority" is that right now I'm |
test_quopri has a decorator that calls a test using both the C and 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. |
It is a priority because we need solid test coverage in order to |
I don't think the decorator approach would work for the doctests, it looks |
Hi,
Doctests will be annoying indeed. I never use doctests so I can't 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. |
This lack of tests is an issue for Python 2.6 as well. bpo-9233 might have been avoided were the pure-Python implementation tested. |
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. |
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 |
Attached patch refactors the tests to use import_fresh_module and different subclasses for Python and C tests. |
Comments:
|
This indeed works -- using unittest internals was just a temporary workaround because the example in the unittest doc didn't seem to work.
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.
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.
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__: and the other test files will use either: or: 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: and then use: I'm not sure any of these options is better than what we have now though. |
I was thinking about that. That looks clean and explicit to me. |
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. |
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 |
My usual pattern (adopted from examples in the stdlib tests) is this: TestSomethingBase:
PyTestSomething(TestSomethingBase, TestCase): CTestSomething(TestSomethingBase, TestCase): Is there a reason that won't work in your case? |
Technically they both work, they are just two different approaches that offer more or less the same compromise between features and verbosity. |
Attached patch uses the approach described in msg135881. |
New changeset 5b0fecd2eba0 by Ezio Melotti in branch '2.7': New changeset c2853a54b29e by Ezio Melotti in branch '3.1': New changeset 63fb2b811c9d by Ezio Melotti in branch '3.2': New changeset afdc06f2552f by Ezio Melotti in branch 'default': |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: