From 76b030ce538df81d137ca04a1bf42606c8872749 Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Sun, 28 Jun 2009 17:43:05 -0400 Subject: Epic bug: pyexpat fiddles incorrectly with the systrace function. This is a hack to make it behave correctly with coverage.py. Fixes bug #10. --- CHANGES.txt | 3 +++ coverage/tracer.c | 19 +++++++++++++++++++ test/test_coverage.py | 47 ++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 68 insertions(+), 1 deletion(-) diff --git a/CHANGES.txt b/CHANGES.txt index 488ab3d..9f4ec39 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -12,6 +12,9 @@ Version 3.next mode just like Python does. This lets it run Windows files on Mac, for example. +- Fixed a bizarre problem involving pyexpat, whereby lines following XML parser + invocations could be overlooked. + Version 3.0, 13 June 2009 ------------------------- diff --git a/coverage/tracer.c b/coverage/tracer.c index 50a25be..15a17b2 100644 --- a/coverage/tracer.c +++ b/coverage/tracer.c @@ -198,6 +198,25 @@ Tracer_trace(Tracer *self, PyFrameObject *frame, int what, PyObject *arg) break; } + /* UGLY HACK: for some reason, pyexpat invokes the systrace function directly. + It uses "pyexpat.c" as the filename, which is strange enough, but it calls + it incorrectly: when an exception passes through the C code, it calls trace + with an EXCEPTION, but never calls RETURN. This throws off our bookkeeping. + To make things right, if this is an EXCEPTION from pyexpat.c, then inject + a RETURN event also. If the bug in pyexpat.c gets fixed someday, we'll + either have to put a version check here, or do something more sophisticated + to detect the EXCEPTION-without-RETURN case that has to be fixed up. + */ + if (what == PyTrace_EXCEPTION) { + if (strstr(PyString_AS_STRING(frame->f_code->co_filename), "pyexpat.c")) { + /* Stupid pyexpat: pretend it gave us the RETURN it should have. */ + SHOWLOG(self->depth, frame->f_lineno, frame->f_code->co_filename, "wrongexc"); + if (Tracer_trace(self, frame, PyTrace_RETURN, arg) < 0) { + return -1; + } + } + } + return 0; } diff --git a/test/test_coverage.py b/test/test_coverage.py index 30ac4b8..2d08281 100644 --- a/test/test_coverage.py +++ b/test/test_coverage.py @@ -1704,6 +1704,51 @@ class RecursionTest(CoverageTest): [1,2,3,5,7], "") +class PyexpatTest(CoverageTest): + """Pyexpat screws up tracing. Make sure we've counter-defended properly.""" + def testPyexpat(self): + # pyexpat calls the trace function explicitly (inexplicably), and does + # it wrong for exceptions. Parsing a DOCTYPE for some reason throws + # an exception internally, and triggers its wrong behavior. This test + # checks that our fake PyTrace_RETURN hack in tracer.c works. It will + # also detect if the pyexpat bug is fixed unbeknownst to us, meaning + # we'd see two RETURNs where there should only be one. + + self.makeFile("trydom.py", """\ + import xml.dom.minidom + + XML = '''\\ + + + ''' + + def foo(): + dom = xml.dom.minidom.parseString(XML) + assert len(dom.getElementsByTagName('child')) == 2 + print "Parsed" + + foo() + """) + + self.makeFile("outer.py", "\n"*100 + "import trydom\nprint 'done'\n") + + cov = coverage.coverage() + cov.erase() + + # Import the python file, executing it. + cov.start() + self.importModule("outer") + cov.stop() + + _, statements, missing, _ = cov.analysis("trydom.py") + self.assertEqual(statements, [1,3,8,9,10,11,13]) + self.assertEqual(missing, []) + + _, statements, missing, _ = cov.analysis("outer.py") + self.assertEqual(statements, [101,102]) + self.assertEqual(missing, []) + + if __name__ == '__main__': print "Testing under Python version: %s" % sys.version unittest.main() @@ -1714,4 +1759,4 @@ if __name__ == '__main__': # in an expression!) # TODO: Generator comprehensions? # TODO: Constant if tests ("if 1:"). Py 2.4 doesn't execute them. -# TODO: There are no tests for analysis2 directly. \ No newline at end of file +# TODO: There are no tests for analysis2 directly. -- cgit v1.2.1