A new warning for files already imported before coverage starts

7 files changed, 90 insertions, 29 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index f2fcfc63..126ced94 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -18,7 +18,9 @@ Change history for Coverage.py
 Unreleased
 ----------
 
-None yet
+- A new warning (already-imported) is issued if measurable files have already
+  been imported before coverage.py started measurement.  See
+  :ref:`cmd_warnings` for more information.
 
 
 .. _changes_451:
diff --git a/coverage/control.py b/coverage/control.py
index b82c8047..daa00bd0 100644
--- a/coverage/control.py
+++ b/coverage/control.py
@@ -74,6 +74,10 @@ class Coverage(object):
         cov.html_report(directory='covhtml')
 
     """
+    # A global to know if we have ever checked for files imported before
+    # coverage has been started.
+    _checked_preimported = False
+
     def __init__(
         self, data_file=None, data_suffix=None, cover_pylib=None,
         auto_data=False, timid=None, branch=None, config_file=True,
@@ -164,6 +168,7 @@ class Coverage(object):
         # Is it ok for no data to be collected?
         self._warn_no_data = True
         self._warn_unimported_source = True
+        self._warn_preimported_source = True
 
         # A record of all the warnings that have been issued.
         self._warnings = []
@@ -434,7 +439,7 @@ class Coverage(object):
         else:
             return dunder_name
 
-    def _should_trace_internal(self, filename, frame):
+    def _should_trace_internal(self, filename, frame=None):
         """Decide whether to trace execution in `filename`, with a reason.
 
         This function is called from the trace function.  As each new file name
@@ -452,22 +457,23 @@ class Coverage(object):
             disp.reason = reason
             return disp
 
-        # Compiled Python files have two file names: frame.f_code.co_filename is
-        # the file name at the time the .pyc was compiled.  The second name is
-        # __file__, which is where the .pyc was actually loaded from.  Since
-        # .pyc files can be moved after compilation (for example, by being
-        # installed), we look for __file__ in the frame and prefer it to the
-        # co_filename value.
-        dunder_file = frame.f_globals and frame.f_globals.get('__file__')
-        if dunder_file:
-            filename = source_for_file(dunder_file)
-            if original_filename and not original_filename.startswith('<'):
-                orig = os.path.basename(original_filename)
-                if orig != os.path.basename(filename):
-                    # Files shouldn't be renamed when moved. This happens when
-                    # exec'ing code.  If it seems like something is wrong with
-                    # the frame's file name, then just use the original.
-                    filename = original_filename
+        if frame is not None:
+            # Compiled Python files have two file names: frame.f_code.co_filename is
+            # the file name at the time the .pyc was compiled.  The second name is
+            # __file__, which is where the .pyc was actually loaded from.  Since
+            # .pyc files can be moved after compilation (for example, by being
+            # installed), we look for __file__ in the frame and prefer it to the
+            # co_filename value.
+            dunder_file = frame.f_globals and frame.f_globals.get('__file__')
+            if dunder_file:
+                filename = source_for_file(dunder_file)
+                if original_filename and not original_filename.startswith('<'):
+                    orig = os.path.basename(original_filename)
+                    if orig != os.path.basename(filename):
+                        # Files shouldn't be renamed when moved. This happens when
+                        # exec'ing code.  If it seems like something is wrong with
+                        # the frame's file name, then just use the original.
+                        filename = original_filename
 
         if not filename:
             # Empty string is pretty useless.
@@ -534,22 +540,21 @@ class Coverage(object):
                     "Plugin %r didn't set source_filename for %r" %
                     (plugin, disp.original_filename)
                 )
-            reason = self._check_include_omit_etc_internal(
-                disp.source_filename, frame,
-            )
+            module_globals = frame.f_globals if frame is not None else {}
+            reason = self._check_include_omit_etc_internal(disp.source_filename, module_globals)
             if reason:
                 nope(disp, reason)
 
         return disp
 
-    def _check_include_omit_etc_internal(self, filename, frame):
+    def _check_include_omit_etc_internal(self, filename, module_globals):
         """Check a file name against the include, omit, etc, rules.
 
         Returns a string or None.  String means, don't trace, and is the reason
         why.  None means no reason found to not trace.
 
         """
-        modulename = self._name_for_module(frame.f_globals, filename)
+        modulename = self._name_for_module(module_globals, filename)
 
         # If the user specified source or include, then that's authoritative
         # about the outer bound of what to measure and we don't have to apply
@@ -599,7 +604,8 @@ class Coverage(object):
         Returns a boolean: True if the file should be traced, False if not.
 
         """
-        reason = self._check_include_omit_etc_internal(filename, frame)
+        module_globals = frame.f_globals if frame is not None else {}
+        reason = self._check_include_omit_etc_internal(filename, module_globals)
         if self.debug.should('trace'):
             if not reason:
                 msg = "Including %r" % (filename,)
@@ -698,9 +704,31 @@ class Coverage(object):
         if self._auto_load:
             self.load()
 
+        # See if we think some code that would eventually be measured has already been imported.
+        if not Coverage._checked_preimported and self._warn_preimported_source:
+            if self.include or self.source or self.source_pkgs:
+                self._check_for_already_imported_files()
+                Coverage._checked_preimported = True
+
         self.collector.start()
         self._started = True
 
+    def _check_for_already_imported_files(self):
+        """Examine sys.modules looking for files that will be measured."""
+        warned = set()
+        for mod in list(sys.modules.values()):
+            filename = getattr(mod, "__file__", None)
+            if filename is None:
+                continue
+            if filename in warned:
+                continue
+
+            disp = self._should_trace_internal(filename)
+            if disp.trace:
+                msg = "Already imported a file that will be measured: {0}".format(filename)
+                self._warn(msg, slug="already-imported")
+                warned.add(filename)
+
     def stop(self):
         """Stop measuring code coverage."""
         if self._started:
@@ -1277,10 +1305,11 @@ def process_startup():
 
     cov = Coverage(config_file=cps)
     process_startup.coverage = cov
-    cov.start()
     cov._warn_no_data = False
     cov._warn_unimported_source = False
+    cov._warn_preimported_source = False
     cov._auto_save = True
+    cov.start()
 
     return cov
 
diff --git a/coverage/multiproc.py b/coverage/multiproc.py
index fe837318..986ee9d4 100644
--- a/coverage/multiproc.py
+++ b/coverage/multiproc.py
@@ -33,6 +33,7 @@ class ProcessWithCoverage(OriginalProcess):
         from coverage import Coverage       # avoid circular import
         rcfile = os.environ[COVERAGE_RCFILE_ENV]
         cov = Coverage(data_suffix=True, config_file=rcfile)
+        cov._warn_preimported_source = False
         cov.start()
         debug = cov.debug
         try:
diff --git a/doc/cmd.rst b/doc/cmd.rst
index ef4c1135..baf1ca08 100644
--- a/doc/cmd.rst
+++ b/doc/cmd.rst
@@ -171,6 +171,13 @@ could affect the measurement process.  The possible warnings include:
   when coverage started.  This meant coverage.py couldn't monitor its
   execution.
 
+* "Already imported a file that will be measured: XXX (already-imported)"
+
+  File XXX had already been imported when coverage.py started measurement. Your
+  setting for ``--source`` or ``--include`` indicates that you wanted to
+  measure that file.  Lines will be missing from the coverage report since the
+  execution during import hadn't been measured.
+
 * "--include is ignored because --source is set (include-ignored)"
 
   Both ``--include`` and ``--source`` were specified while running code.  Both
diff --git a/igor.py b/igor.py
index 43ce3303..3f5ce12b 100644
--- a/igor.py
+++ b/igor.py
@@ -122,11 +122,8 @@ def run_tests_with_coverage(tracer, *runner_args):
 
     import coverage
     cov = coverage.Coverage(config_file="metacov.ini", data_suffix=False)
-    # Cheap trick: the coverage.py code itself is excluded from measurement,
-    # but if we clobber the cover_prefix in the coverage object, we can defeat
-    # the self-detection.
-    cov.cover_prefix = "Please measure coverage.py!"
     cov._warn_unimported_source = False
+    cov._warn_preimported_source = False
     cov.start()
 
     try:
diff --git a/tests/test_api.py b/tests/test_api.py
index b461c503..7c2672d8 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -5,6 +5,7 @@
 
 import fnmatch
 import os
+import os.path
 import sys
 import textwrap
 import warnings
diff --git a/tests/test_process.py b/tests/test_process.py
index 35dddd07..70329b59 100644
--- a/tests/test_process.py
+++ b/tests/test_process.py
@@ -585,6 +585,30 @@ class ProcessTest(CoverageTest):
 
         self.assertIn("Trace function changed", out)
 
+    def test_warn_preimported(self):
+        self.make_file("hello.py", """\
+            import goodbye
+            import coverage
+            cov = coverage.Coverage(include=["good*"])
+            cov.start()
+            print(goodbye.f())
+            cov.stop()
+            """)
+        self.make_file("goodbye.py", """\
+            def f():
+                return "Goodbye!"
+            """)
+        goodbye_path = os.path.abspath("goodbye.py")
+
+        out = self.run_command("python hello.py")
+        self.assertIn("Goodbye!", out)
+
+        msg = (
+            "Coverage.py warning: "
+            "Already imported a file that will be measured: {0} "
+            "(already-imported)").format(goodbye_path)
+        self.assertIn(msg, out)
+
     def test_note(self):
         if env.PYPY and env.PY3 and env.PYPYVERSION[:3] == (5, 10, 0):
             # https://bitbucket.org/pypy/pypy/issues/2729/pypy3-510-incorrectly-decodes-astral-plane