Fix a problem with DecoratorTools fiddling with the trace function and screwing us up. Now the Python trace function is simpler, with no variability of registered trace function. Fixes bugs #12 and #13.

7 files changed, 185 insertions, 72 deletions

diff --git a/alltests.cmd b/alltests.cmd
index 3a29202f..0a4888d3 100644
--- a/alltests.cmd
+++ b/alltests.cmd
@@ -1,20 +1,31 @@
 call \ned\bin\switchpy 23

 python setup.py -q develop

+set COVERAGE_TEST_TRACER=c

 nosetests

 del coverage\tracer.pyd

+set COVERAGE_TEST_TRACER=py

 nosetests

+

 call \ned\bin\switchpy 24

 python setup.py -q develop

+set COVERAGE_TEST_TRACER=c

 nosetests

 del coverage\tracer.pyd

+set COVERAGE_TEST_TRACER=py

 nosetests

+

 call \ned\bin\switchpy 25

 python setup.py -q develop

+set COVERAGE_TEST_TRACER=c

 nosetests

 del coverage\tracer.pyd

+set COVERAGE_TEST_TRACER=py

 nosetests

+

 call \ned\bin\switchpy 26

 python setup.py -q develop

+set COVERAGE_TEST_TRACER=c

 nosetests

 del coverage\tracer.pyd

+set COVERAGE_TEST_TRACER=py

 nosetests

diff --git a/coverage/cmdline.py b/coverage/cmdline.py
index 9684b925..b353efa1 100644
--- a/coverage/cmdline.py
+++ b/coverage/cmdline.py
@@ -10,11 +10,12 @@ Measure, collect, and report on code coverage in Python programs.
 
 Usage:
 
-coverage -x [-p] [-L] MODULE.py [ARG1 ARG2 ...]
+coverage -x [-p] [-L] [--timid] MODULE.py [ARG1 ARG2 ...]
     Execute the module, passing the given command-line arguments, collecting
     coverage data.  With the -p option, include the machine name and process
     ID in the .coverage file name.  With -L, measure coverage even inside the
-    Python installed library, which isn't done by default.
+    Python installed library, which isn't done by default.  With --timid, use a
+    simpler but slower trace method.
 
 coverage -e
     Erase collected coverage data.
@@ -95,8 +96,11 @@ class CoverageScript:
             '-x': 'execute',
             '-o:': 'omit=',
             }
+        # Long options with no short equivalent.
+        long_only_opts = ['timid']
+        
         short_opts = ''.join([o[1:] for o in optmap.keys()])
-        long_opts = optmap.values()
+        long_opts = optmap.values() + long_only_opts
         options, args = getopt.getopt(argv, short_opts, long_opts)
         for o, a in options:
             if optmap.has_key(o):
@@ -139,7 +143,8 @@ class CoverageScript:
         # Do something.
         self.coverage = self.covpkg.coverage(
             data_suffix = bool(settings.get('parallel-mode')),
-            cover_pylib = settings.get('pylib')
+            cover_pylib = settings.get('pylib'),
+            timid = settings.get('timid'),
             )
 
         if settings.get('erase'):
diff --git a/coverage/collector.py b/coverage/collector.py
index 940f7c75..a121193d 100644
--- a/coverage/collector.py
+++ b/coverage/collector.py
@@ -7,66 +7,72 @@ try:
     from coverage.tracer import Tracer
 except ImportError:
     # Couldn't import the C extension, maybe it isn't built.
-
-    class Tracer:
-        """Python implementation of the raw data tracer."""
-        def __init__(self):
-            self.data = None
-            self.should_trace = None
-            self.should_trace_cache = None
-            self.cur_filename = None
-            self.filename_stack = []
-            self.last_exc_back = None
-
-        def _global_trace(self, frame, event, arg_unused):
-            """The trace function passed to sys.settrace."""
-            #print "global event: %s %r" % (event, frame.f_code.co_filename)
-            if event == 'call':
-                # Entering a new function context.  Decide if we should trace
-                # in this file.
-                filename = frame.f_code.co_filename
-                tracename = self.should_trace_cache.get(filename)
-                if tracename is None:
-                    tracename = self.should_trace(filename, frame)
-                    self.should_trace_cache[filename] = tracename
-                if tracename:
-                    # We need to trace.  Push the current filename on the stack
-                    # and record the new current filename.
-                    self.filename_stack.append(self.cur_filename)
-                    self.cur_filename = tracename
-                    # Use _local_trace for tracing within this function.
-                    return self._local_trace
-                else:
-                    # No tracing in this function.
-                    return None
-            return self._global_trace
-    
-        def _local_trace(self, frame, event, arg_unused):
-            """The trace function used within a function."""
-            #print "local event: %s %r" % (event, frame.f_code.co_filename)
-            if self.last_exc_back:
-                if frame == self.last_exc_back:
-                    # Someone forgot a return event.
-                    self.cur_filename = self.filename_stack.pop()
-                self.last_exc_back = None
-                
-            if event == 'line':
-                # Record an executed line.
-                self.data[(self.cur_filename, frame.f_lineno)] = True
-            elif event == 'return':
-                # Leaving this function, pop the filename stack.
-                self.cur_filename = self.filename_stack.pop()
-            elif event == 'exception':
-                self.last_exc_back = frame.f_back
-            return self._local_trace
+    Tracer = None
     
-        def start(self):
-            """Start this Tracer."""
-            sys.settrace(self._global_trace)
+class PyTracer:
+    """Python implementation of the raw data tracer."""
     
-        def stop(self):
-            """Stop this Tracer."""
-            sys.settrace(None)
+    # Because of poor implementations of trace-function-manipulating tools,
+    # the Python trace function must be kept very simple.  In particular, there
+    # must be only one function ever set as the trace function, both through
+    # sys.settrace, and as the return value from the trace function.  Put
+    # another way, the trace function must always return itself.  It cannot
+    # swap in other functions, or return None to avoid tracing a particular
+    # frame.
+    #
+    # The trace manipulator that introduced this restriction is DecoratorTools,
+    # which sets a trace function, and then later restores the pre-existing one
+    # by calling sys.settrace with a function it found in the current frame.
+    #
+    # Systems that use DecoratorTools (or similar trace manipulations) must use
+    # PyTracer to get accurate results.  The command-line --timid argument is
+    # used to force the use of this tracer.
+
+    def __init__(self):
+        self.data = None
+        self.should_trace = None
+        self.should_trace_cache = None
+        self.cur_filename = None
+        self.filename_stack = []
+        self.last_exc_back = None
+
+    def _trace(self, frame, event, arg_unused):
+        """The trace function passed to sys.settrace."""
+        
+        #print "trace event: %s %r @%d" % (
+        #           event, frame.f_code.co_filename, frame.f_lineno)
+        
+        if self.last_exc_back:
+            if frame == self.last_exc_back:
+                # Someone forgot a return event.
+                self.cur_filename = self.filename_stack.pop()
+            self.last_exc_back = None
+            
+        if event == 'call':
+            # Entering a new function context.  Decide if we should trace
+            # in this file.
+            self.filename_stack.append(self.cur_filename)
+            filename = frame.f_code.co_filename
+            tracename = self.should_trace(filename, frame)
+            self.cur_filename = tracename
+        elif event == 'line':
+            # Record an executed line.
+            if self.cur_filename:
+                self.data[(self.cur_filename, frame.f_lineno)] = True
+        elif event == 'return':
+            # Leaving this function, pop the filename stack.
+            self.cur_filename = self.filename_stack.pop()
+        elif event == 'exception':
+            self.last_exc_back = frame.f_back
+        return self._trace
+        
+    def start(self):
+        """Start this Tracer."""
+        sys.settrace(self._trace)
+
+    def stop(self):
+        """Stop this Tracer."""
+        sys.settrace(None)
 
 
 class Collector:
@@ -89,16 +95,28 @@ class Collector:
     # the top, and resumed when they become the top again.
     _collectors = []
 
-    def __init__(self, should_trace):
+    def __init__(self, should_trace, timid=False):
         """Create a collector.
         
         `should_trace` is a function, taking a filename, and returning a
         canonicalized filename, or False depending on whether the file should
         be traced or not.
         
+        If `timid` is true, then a slower simpler trace function will be
+        used.  This is important for some environments where manipulation of
+        tracing functions make the faster more sophisticated trace function not
+        operate properly.
+        
         """
         self.should_trace = should_trace
         self.reset()
+        if timid:
+            # Being timid: use the simple Python trace function.
+            self._trace_class = PyTracer
+        else:
+            # Being fast: use the C Tracer if it is available, else the Python
+            # trace function.
+            self._trace_class = Tracer or PyTracer
 
     def reset(self):
         """Clear collected data, and prepare to collect more."""
@@ -116,7 +134,7 @@ class Collector:
 
     def _start_tracer(self):
         """Start a new Tracer object, and store it in self.tracers."""
-        tracer = Tracer()
+        tracer = self._trace_class()
         tracer.data = self.data
         tracer.should_trace = self.should_trace
         tracer.should_trace_cache = self.should_trace_cache
@@ -153,12 +171,10 @@ class Collector:
         """Stop collecting trace information."""
         assert self._collectors
         assert self._collectors[-1] is self
-        
-        for tracer in self.tracers:
-            tracer.stop()
+
+        self.pause()        
         self.tracers = []
-        threading.settrace(None)
-        
+                
         # Remove this Collector from the stack, and resume the one underneath
         # (if any).
         self._collectors.pop()
diff --git a/coverage/control.py b/coverage/control.py
index 57079fee..2cfde279 100644
--- a/coverage/control.py
+++ b/coverage/control.py
@@ -20,13 +20,14 @@ class coverage:
         
         cov = coverage()
         cov.start()
-        #.. blah blah (run your code) blah blah
+        #.. blah blah (run your code) blah blah ..
         cov.stop()
         cov.html_report(directory='covhtml')
 
     """
+
     def __init__(self, data_file=None, data_suffix=False, cover_pylib=False,
-                auto_data=False):
+                auto_data=False, timid=False):
         """Create a new coverage measurement context.
         
         `data_file` is the base name of the data file to use, defaulting to
@@ -42,6 +43,11 @@ class coverage:
         coverage measurement starts, and data will be saved automatically when
         measurement stops.
         
+        If `timid` is true, then a slower simpler trace function will be
+        used.  This is important for some environments where manipulation of
+        tracing functions make the faster more sophisticated trace function not
+        operate properly.
+        
         """
         from coverage import __version__
         
@@ -53,7 +59,11 @@ class coverage:
         
         self.file_locator = FileLocator()
         
-        self.collector = Collector(self._should_trace)
+        # Timidity: for nose users, read an environment variable.  This is a
+        # cheap hack, since the rest of the command line arguments aren't
+        # recognized, but it solves some users' problems.
+        timid = timid or ('--timid' in os.environ.get('COVERAGE_OPTIONS', ''))
+        self.collector = Collector(self._should_trace, timid=timid)
 
         # Create the data file.
         if data_suffix:
diff --git a/doc/cmd.rst b/doc/cmd.rst
index 2f8ec170..ae7282f8 100644
--- a/doc/cmd.rst
+++ b/doc/cmd.rst
@@ -57,6 +57,12 @@ Arguments after your file name are passed to your program in sys.argv.
 By default, coverage does not measure code installed with the Python interpreter.
 If you want to measure that code as well as your own, add the -L flag.
 
+If your coverage results seems to be overlooking code that you know has been
+executed, try running coverage again with the --timid flag.  This uses a simpler
+but slower trace method.  Projects that use DecoratorTools, including TurboGears,
+will need to use --timid to get correct results.  This option can also be set
+with the environment variable COVERAGE_OPTIONS set to '--timid'.
+
 
 Combining data files
 --------------------
diff --git a/test/farm/run/run_timid.py b/test/farm/run/run_timid.py
new file mode 100644
index 00000000..bbc322e2
--- /dev/null
+++ b/test/farm/run/run_timid.py
@@ -0,0 +1,50 @@
+# Test that the --timid command line argument properly swaps the tracer function
+# for a simpler one.
+#
+# This is complicated by the fact that alltests.cmd will run the test suite
+# twice for each version: once with a compiled C-based trace function, and once
+# without it, to also test the Python trace function.  So this test has to
+# examine an environment variable set in alltests.cmd to know whether to expect
+# to see the C trace function or not.
+
+import os
+
+copy("src", "out")
+run("""
+    coverage -e -x showtrace.py regular
+    coverage -e -x --timid showtrace.py timid
+    """, rundir="out", outfile="showtraceout.txt")
+
+# When running timidly, the trace function is always Python.
+contains("out/showtraceout.txt", "timid coverage.collector.PyTracer")
+
+if os.environ.get('COVERAGE_TEST_TRACER', 'c') == 'c':
+    # If the C trace function is being tested, then regular running should have
+    # the C function (shown as None in f_trace since it isn't a Python
+    # function).
+    contains("out/showtraceout.txt", "regular None")
+else:
+    # If the Python trace function is being tested, then regular running will
+    # also show the Python function.
+    contains("out/showtraceout.txt", "regular coverage.collector.PyTracer")
+
+# Try the environment variable.
+old_opts = os.environ.get('COVERAGE_OPTIONS')
+os.environ['COVERAGE_OPTIONS'] = '--timid'
+
+run("""
+    coverage -e -x showtrace.py regular
+    coverage -e -x --timid showtrace.py timid
+    """, rundir="out", outfile="showtraceout.txt")
+
+contains("out/showtraceout.txt",
+        "timid coverage.collector.PyTracer",
+        "regular coverage.collector.PyTracer"
+        )
+
+if old_opts:
+    os.environ['COVERAGE_OPTIONS'] = old_opts
+else:
+    del os.environ['COVERAGE_OPTIONS']
+
+clean("out")
diff --git a/test/farm/run/src/showtrace.py b/test/farm/run/src/showtrace.py
new file mode 100644
index 00000000..49e212e8
--- /dev/null
+++ b/test/farm/run/src/showtrace.py
@@ -0,0 +1,15 @@
+# Show the current frame's trace function, so that we can test what the
+# command-line options do to the trace function used.
+
+import sys
+
+# Print the argument as a label for the output.
+print sys.argv[1],
+
+# Show what the trace function is.  If a C-based function is used, then f_trace
+# is None.
+trace_fn = sys._getframe(0).f_trace
+if trace_fn is None:
+    print "None"
+else:
+    print trace_fn.im_class