diff options
| -rw-r--r-- | CHANGES.txt | 7 | ||||
| -rw-r--r-- | coverage/collector.py | 12 | ||||
| -rw-r--r-- | coverage/control.py | 8 | ||||
| -rw-r--r-- | coverage/tracer.c | 6 | ||||
| -rw-r--r-- | test/test_oddball.py | 49 |
5 files changed, 78 insertions, 4 deletions
diff --git a/CHANGES.txt b/CHANGES.txt index 4604489f..3c04d3b3 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -9,6 +9,13 @@ Version 3.5 - A little bit of Jython support: `coverage run` can now measure Jython execution by adapting when $py.class files are traced. Thanks, Adi Roiban. +- Pathological code execution could disable the trace function behind our + backs, leading to incorrect code measurement. Now if this happens, + coverage.py will issue a warning, at least alerting you to the problem. + Closes `issue 93`_. Thanks to Marius Gedminas for the idea. + +.. _issue 93: http://bitbucket.org/ned/coveragepy/issue/93/copying-a-mock-object-breaks-coverage + Version 3.4 --- 19 September 2010 --------------------------------- diff --git a/coverage/collector.py b/coverage/collector.py index 6b196e99..9c40d16c 100644 --- a/coverage/collector.py +++ b/coverage/collector.py @@ -33,6 +33,7 @@ class PyTracer(object): self.data = None self.should_trace = None self.should_trace_cache = None + self.warn = None self.cur_file_data = None self.last_line = 0 self.data_stack = [] @@ -109,6 +110,10 @@ class PyTracer(object): def stop(self): """Stop this Tracer.""" + if hasattr(sys, "gettrace") and self.warn: + if sys.gettrace() != self._trace: + msg = "Trace function changed, measurement is likely wrong: %r" + self.warn(msg % sys.gettrace()) sys.settrace(None) def get_stats(self): @@ -137,7 +142,7 @@ class Collector(object): # the top, and resumed when they become the top again. _collectors = [] - def __init__(self, should_trace, timid, branch): + def __init__(self, should_trace, timid, branch, warn): """Create a collector. `should_trace` is a function, taking a filename, and returning a @@ -153,8 +158,12 @@ class Collector(object): collecting data on which statements followed each other (arcs). Use `get_arc_data` to get the arc data. + `warn` is a warning function, taking a single string message argument, + to be used if a warning needs to be issued. + """ self.should_trace = should_trace + self.warn = warn self.branch = branch self.reset() @@ -194,6 +203,7 @@ class Collector(object): tracer.arcs = self.branch tracer.should_trace = self.should_trace tracer.should_trace_cache = self.should_trace_cache + tracer.warn = self.warn fn = tracer.start() self.tracers.append(tracer) return fn diff --git a/coverage/control.py b/coverage/control.py index cdaf9721..4fae198c 100644 --- a/coverage/control.py +++ b/coverage/control.py @@ -69,6 +69,9 @@ class coverage(object): """ from coverage import __version__ + # A record of all the warnings that have been issued. + self._warnings = [] + # Build our configuration from a number of sources: # 1: defaults: self.config = CoverageConfig() @@ -120,7 +123,7 @@ class coverage(object): self.collector = Collector( self._should_trace, timid=self.config.timid, - branch=self.config.branch + branch=self.config.branch, warn=self._warn ) # Suffixes are a bit tricky. We want to use the data suffix only when @@ -274,7 +277,8 @@ class coverage(object): def _warn(self, msg): """Use `msg` as a warning.""" - sys.stderr.write("Coverage.py warning: " + msg + "\n") + self._warnings.append(msg) + sys.stderr.write("Coverage.py warning: %s\n" % msg) def _abs_files(self, files): """Return a list of absolute file names for the names in `files`.""" diff --git a/coverage/tracer.c b/coverage/tracer.c index 9eb3cca1..e046596a 100644 --- a/coverage/tracer.c +++ b/coverage/tracer.c @@ -63,6 +63,7 @@ typedef struct { /* Python objects manipulated directly by the Collector class. */ PyObject * should_trace; + PyObject * warn; PyObject * data; PyObject * should_trace_cache; PyObject * arcs; @@ -134,6 +135,7 @@ Tracer_init(Tracer *self, PyObject *args_unused, PyObject *kwds_unused) #endif /* COLLECT_STATS */ self->should_trace = NULL; + self->warn = NULL; self->data = NULL; self->should_trace_cache = NULL; self->arcs = NULL; @@ -166,6 +168,7 @@ Tracer_dealloc(Tracer *self) } Py_XDECREF(self->should_trace); + Py_XDECREF(self->warn); Py_XDECREF(self->data); Py_XDECREF(self->should_trace_cache); @@ -539,6 +542,9 @@ Tracer_members[] = { { "should_trace", T_OBJECT, offsetof(Tracer, should_trace), 0, PyDoc_STR("Function indicating whether to trace a file.") }, + { "warn", T_OBJECT, offsetof(Tracer, warn), 0, + PyDoc_STR("Function for issuing warnings.") }, + { "data", T_OBJECT, offsetof(Tracer, data), 0, PyDoc_STR("The raw dictionary of trace data.") }, diff --git a/test/test_oddball.py b/test/test_oddball.py index 3d5bf097..e94e2bad 100644 --- a/test/test_oddball.py +++ b/test/test_oddball.py @@ -63,8 +63,9 @@ class RecursionTest(CoverageTest): return recur(n-1)+1 recur(495) # We can get at least this many stack frames. + i = 8 # and this line will be traced """, - [1,2,3,5,7], "") + [1,2,3,5,7,8], "") def test_long_recursion(self): # We can't finish a very deep recursion, but we don't crash. @@ -80,6 +81,52 @@ class RecursionTest(CoverageTest): """, [1,2,3,5,7], "") + def test_long_recursion_recovery(self): + # Test the core of bug 93: http://bitbucket.org/ned/coveragepy/issue/93 + # When recovering from a stack overflow, the Python trace function is + # disabled, but the C trace function is not. So if we're using a + # Python trace function, we won't trace anything after the stack + # overflow, and there should be a warning about it. If we're using + # the C trace function, only line 3 will be missing, and all else + # will be traced. + + self.make_file("recur.py", """\ + def recur(n): + if n == 0: + return 0 # never hit + else: + return recur(n-1)+1 + + try: + recur(100000) # This is definitely too many frames. + except RuntimeError: + i = 10 + i = 11 + """) + + cov = coverage.coverage() + cov.start() + self.import_local_file("recur") + cov.stop() + + pytrace = (cov.collector.tracer_name() == "PyTracer") + expected_missing = [3] + if pytrace: + expected_missing += [9,10,11] + + _, statements, missing, _ = cov.analysis("recur.py") + self.assertEqual(statements, [1,2,3,5,7,8,9,10,11]) + self.assertEqual(missing, expected_missing) + + # We can get a warning about the stackoverflow effect on the tracing + # function only if we have sys.gettrace + if pytrace and hasattr(sys, "gettrace"): + self.assertEqual(cov._warnings, + ["Trace function changed, measurement is likely wrong: None"] + ) + else: + self.assertEqual(cov._warnings, []) + class MemoryLeakTest(CoverageTest): """Attempt the impossible: test that memory doesn't leak.""" |