fix: remapping paths during combining needs to follow relative_files=True. #1147

Author: nedbat

CHANGES.rst

@@ -22,6 +22,10 @@ This list is detailed and covers changes in each pre-release version.
 Unreleased
 ----------
 
+- Fix: When remapping file paths through the ``[paths]`` setting while
+  combining, the ``[run] relative_files`` setting was ignored, resulting in
+  absolute paths for remapped file names (`issue 1147`_).  This is now fixed.
+
 - Fix: Complex conditionals over excluded lines could have incorrectly reported
   a missing branch (`issue 1271`_). This is now fixed.
 
@@ -33,6 +37,7 @@ Unreleased
   I'd rather not "fix" unsupported interfaces, it's actually nicer with a
   default value.
 
+.. _issue 1147: https://github.com/nedbat/coveragepy/issues/1147
 .. _issue 1271: https://github.com/nedbat/coveragepy/issues/1271
 .. _issue 1273: https://github.com/nedbat/coveragepy/issues/1273
 

coverage/control.py

@@ -706,7 +706,7 @@ def combine(self, data_paths=None, strict=False, keep=False):
 
         aliases = None
         if self.config.paths:
-            aliases = PathAliases()
+            aliases = PathAliases(relative=self.config.relative_files)
             for paths in self.config.paths.values():
                 result = paths[0]
                 for pattern in paths[1:]:

coverage/files.py

@@ -326,11 +326,13 @@ class PathAliases:
     map a path through those aliases to produce a unified path.
 
     """
-    def __init__(self):
+    def __init__(self, relative=False):
         self.aliases = []
+        self.relative = relative
 
     def pprint(self):       # pragma: debugging
         """Dump the important parts of the PathAliases, for debugging."""
+        print(f"Aliases (relative={self.relative}):")
         for regex, result in self.aliases:
             print(f"{regex.pattern!r} --> {result!r}")
 
@@ -393,7 +395,8 @@ def map(self, path):
             if m:
                 new = path.replace(m.group(0), result)
                 new = new.replace(sep(path), sep(result))
-                new = canonical_filename(new)
+                if not self.relative:
+                    new = canonical_filename(new)
                 return new
         return path
 

tests/test_api.py

@@ -23,7 +23,8 @@
 from coverage.misc import import_local_file
 
 from tests.coveragetest import CoverageTest, TESTS_DIR, UsingModulesMixin
-from tests.helpers import assert_count_equal, assert_coverage_warnings, change_dir, nice_file
+from tests.helpers import assert_count_equal, assert_coverage_warnings
+from tests.helpers import change_dir, nice_file, os_sep
 
 
 class ApiTest(CoverageTest):
@@ -1151,16 +1152,19 @@ def test_moving_stuff_with_relative(self):
             assert res == 100
 
     def test_combine_relative(self):
-        self.make_file("dir1/foo.py", "a = 1")
-        self.make_file("dir1/.coveragerc", """\
+        self.make_file("foo.py", """\
+            import mod
+            a = 1
+            """)
+        self.make_file("lib/mod/__init__.py", "x = 1")
+        self.make_file(".coveragerc", """\
             [run]
             relative_files = true
             """)
-        with change_dir("dir1"):
-            cov = coverage.Coverage(source=["."], data_suffix=True)
-            self.start_import_stop(cov, "foo")
-            cov.save()
-            shutil.move(glob.glob(".coverage.*")[0], "..")
+        sys.path.append("lib")
+        cov = coverage.Coverage(source=["."], data_suffix=True)
+        self.start_import_stop(cov, "foo")
+        cov.save()
 
         self.make_file("dir2/bar.py", "a = 1")
         self.make_file("dir2/.coveragerc", """\
@@ -1176,17 +1180,22 @@ def test_combine_relative(self):
         self.make_file(".coveragerc", """\
             [run]
             relative_files = true
+            [paths]
+            source =
+                modsrc
+                */mod
             """)
         cov = coverage.Coverage()
         cov.combine()
         cov.save()
 
         self.make_file("foo.py", "a = 1")
         self.make_file("bar.py", "a = 1")
+        self.make_file("modsrc/__init__.py", "x = 1")
 
         cov = coverage.Coverage()
         cov.load()
         files = cov.get_data().measured_files()
-        assert files == {'foo.py', 'bar.py'}
+        assert files == {'foo.py', 'bar.py', os_sep('modsrc/__init__.py')}
         res = cov.report()
         assert res == 100

tests/test_files.py

@@ -227,66 +227,72 @@ def test_fnmatch_windows_paths(self):
         self.assertMatches(fnm, r"dir\foo.py", True)
 
 
+@pytest.fixture(params=[False, True], name="rel_yn")
+def relative_setting(request):
+    """Parameterized fixture to choose whether PathAliases is relative or not."""
+    return request.param
+
+
 class PathAliasesTest(CoverageTest):
     """Tests for coverage/files.py:PathAliases"""
 
     run_in_temp_dir = False
 
-    def assert_mapped(self, aliases, inp, out):
+    def assert_mapped(self, aliases, inp, out, relative=False):
         """Assert that `inp` mapped through `aliases` produces `out`.
 
-        `out` is canonicalized first, since aliases always produce
-        canonicalized paths.
+        `out` is canonicalized first, since aliases produce canonicalized
+        paths by default.
 
         """
-        aliases.pprint()
-        print(inp)
-        print(out)
-        assert aliases.map(inp) == files.canonical_filename(out)
+        mapped = aliases.map(inp)
+        expected = files.canonical_filename(out) if not relative else out
+        assert mapped == expected
 
     def assert_unchanged(self, aliases, inp):
         """Assert that `inp` mapped through `aliases` is unchanged."""
         assert aliases.map(inp) == inp
 
-    def test_noop(self):
-        aliases = PathAliases()
+    def test_noop(self, rel_yn):
+        aliases = PathAliases(relative=rel_yn)
         self.assert_unchanged(aliases, '/ned/home/a.py')
 
-    def test_nomatch(self):
-        aliases = PathAliases()
+    def test_nomatch(self, rel_yn):
+        aliases = PathAliases(relative=rel_yn)
         aliases.add('/home/*/src', './mysrc')
         self.assert_unchanged(aliases, '/home/foo/a.py')
 
-    def test_wildcard(self):
-        aliases = PathAliases()
+    def test_wildcard(self, rel_yn):
+        aliases = PathAliases(relative=rel_yn)
         aliases.add('/ned/home/*/src', './mysrc')
-        self.assert_mapped(aliases, '/ned/home/foo/src/a.py', './mysrc/a.py')
+        self.assert_mapped(aliases, '/ned/home/foo/src/a.py', './mysrc/a.py', relative=rel_yn)
 
-        aliases = PathAliases()
+        aliases = PathAliases(relative=rel_yn)
         aliases.add('/ned/home/*/src/', './mysrc')
-        self.assert_mapped(aliases, '/ned/home/foo/src/a.py', './mysrc/a.py')
+        self.assert_mapped(aliases, '/ned/home/foo/src/a.py', './mysrc/a.py', relative=rel_yn)
 
-    def test_no_accidental_match(self):
-        aliases = PathAliases()
+    def test_no_accidental_match(self, rel_yn):
+        aliases = PathAliases(relative=rel_yn)
         aliases.add('/home/*/src', './mysrc')
         self.assert_unchanged(aliases, '/home/foo/srcetc')
 
-    def test_multiple_patterns(self):
-        aliases = PathAliases()
+    def test_multiple_patterns(self, rel_yn):
+        aliases = PathAliases(relative=rel_yn)
         aliases.add('/home/*/src', './mysrc')
         aliases.add('/lib/*/libsrc', './mylib')
-        self.assert_mapped(aliases, '/home/foo/src/a.py', './mysrc/a.py')
-        self.assert_mapped(aliases, '/lib/foo/libsrc/a.py', './mylib/a.py')
-
-    def test_cant_have_wildcard_at_end(self):
+        self.assert_mapped(aliases, '/home/foo/src/a.py', './mysrc/a.py', relative=rel_yn)
+        self.assert_mapped(aliases, '/lib/foo/libsrc/a.py', './mylib/a.py', relative=rel_yn)
+
+    @pytest.mark.parametrize("badpat", [
+        "/ned/home/*",
+        "/ned/home/*/",
+        "/ned/home/*/*/",
+    ])
+    def test_cant_have_wildcard_at_end(self, badpat):
         aliases = PathAliases()
         msg = "Pattern must not end with wildcards."
         with pytest.raises(CoverageException, match=msg):
-            aliases.add("/ned/home/*", "fooey")
-        with pytest.raises(CoverageException, match=msg):
-            aliases.add("/ned/home/*/", "fooey")
-        with pytest.raises(CoverageException, match=msg):
-            aliases.add("/ned/home/*/*/", "fooey")
+            aliases.add(badpat, "fooey")
 
     def test_no_accidental_munging(self):
         aliases = PathAliases()
@@ -295,94 +301,104 @@ def test_no_accidental_munging(self):
         self.assert_mapped(aliases, r'c:\Zoo\boo\foo.py', 'src/foo.py')
         self.assert_mapped(aliases, r'/home/ned$/foo.py', 'src/foo.py')
 
-    def test_paths_are_os_corrected(self):
-        aliases = PathAliases()
+    def test_paths_are_os_corrected(self, rel_yn):
+        aliases = PathAliases(relative=rel_yn)
         aliases.add('/home/ned/*/src', './mysrc')
         aliases.add(r'c:\ned\src', './mysrc')
-        self.assert_mapped(aliases, r'C:\Ned\src\sub\a.py', './mysrc/sub/a.py')
+        self.assert_mapped(aliases, r'C:\Ned\src\sub\a.py', './mysrc/sub/a.py', relative=rel_yn)
 
-        aliases = PathAliases()
+        aliases = PathAliases(relative=rel_yn)
         aliases.add('/home/ned/*/src', r'.\mysrc')
         aliases.add(r'c:\ned\src', r'.\mysrc')
-        self.assert_mapped(aliases, r'/home/ned/foo/src/sub/a.py', r'.\mysrc\sub\a.py')
+        self.assert_mapped(
+            aliases,
+            r'/home/ned/foo/src/sub/a.py',
+            r'.\mysrc\sub\a.py',
+            relative=rel_yn,
+        )
 
-    def test_windows_on_linux(self):
+    def test_windows_on_linux(self, rel_yn):
         # https://github.com/nedbat/coveragepy/issues/618
         lin = "*/project/module/"
         win = "*\\project\\module\\"
 
         # Try the paths in both orders.
         for paths in [[lin, win], [win, lin]]:
-            aliases = PathAliases()
+            aliases = PathAliases(relative=rel_yn)
             for path in paths:
                 aliases.add(path, "project/module")
             self.assert_mapped(
                 aliases,
                 "C:\\a\\path\\somewhere\\coveragepy_test\\project\\module\\tests\\file.py",
-                "project/module/tests/file.py"
+                "project/module/tests/file.py",
+                relative=rel_yn,
             )
 
-    def test_linux_on_windows(self):
+    def test_linux_on_windows(self, rel_yn):
         # https://github.com/nedbat/coveragepy/issues/618
         lin = "*/project/module/"
         win = "*\\project\\module\\"
 
         # Try the paths in both orders.
         for paths in [[lin, win], [win, lin]]:
-            aliases = PathAliases()
+            aliases = PathAliases(relative=rel_yn)
             for path in paths:
                 aliases.add(path, "project\\module")
             self.assert_mapped(
                 aliases,
                 "C:/a/path/somewhere/coveragepy_test/project/module/tests/file.py",
-                "project\\module\\tests\\file.py"
+                "project\\module\\tests\\file.py",
+                relative=rel_yn,
             )
 
-    def test_multiple_wildcard(self):
-        aliases = PathAliases()
+    def test_multiple_wildcard(self, rel_yn):
+        aliases = PathAliases(relative=rel_yn)
         aliases.add('/home/jenkins/*/a/*/b/*/django', './django')
         self.assert_mapped(
             aliases,
             '/home/jenkins/xx/a/yy/b/zz/django/foo/bar.py',
-            './django/foo/bar.py'
+            './django/foo/bar.py',
+            relative=rel_yn,
         )
 
-    def test_windows_root_paths(self):
-        aliases = PathAliases()
+    def test_windows_root_paths(self, rel_yn):
+        aliases = PathAliases(relative=rel_yn)
         aliases.add('X:\\', '/tmp/src')
         self.assert_mapped(
             aliases,
             "X:\\a\\file.py",
-            "/tmp/src/a/file.py"
+            "/tmp/src/a/file.py",
+            relative=rel_yn,
         )
         self.assert_mapped(
             aliases,
             "X:\\file.py",
-            "/tmp/src/file.py"
+            "/tmp/src/file.py",
+            relative=rel_yn,
         )
 
-    def test_leading_wildcard(self):
-        aliases = PathAliases()
+    def test_leading_wildcard(self, rel_yn):
+        aliases = PathAliases(relative=rel_yn)
         aliases.add('*/d1', './mysrc1')
         aliases.add('*/d2', './mysrc2')
-        self.assert_mapped(aliases, '/foo/bar/d1/x.py', './mysrc1/x.py')
-        self.assert_mapped(aliases, '/foo/bar/d2/y.py', './mysrc2/y.py')
-
-    def test_dot(self):
-        cases = ['.', '..', '../other']
-        if not env.WINDOWS:
-            # The root test case was added for the manylinux Docker images,
-            # and I'm not sure how it should work on Windows, so skip it.
-            cases += ['/']
-        for d in cases:
-            aliases = PathAliases()
-            aliases.add(d, '/the/source')
-            the_file = os.path.join(d, 'a.py')
-            the_file = os.path.expanduser(the_file)
-            the_file = os.path.abspath(os.path.realpath(the_file))
-
-            assert '~' not in the_file  # to be sure the test is pure.
-            self.assert_mapped(aliases, the_file, '/the/source/a.py')
+        self.assert_mapped(aliases, '/foo/bar/d1/x.py', './mysrc1/x.py', relative=rel_yn)
+        self.assert_mapped(aliases, '/foo/bar/d2/y.py', './mysrc2/y.py', relative=rel_yn)
+
+    # The root test case was added for the manylinux Docker images,
+    # and I'm not sure how it should work on Windows, so skip it.
+    cases = [".", "..", "../other"]
+    if not env.WINDOWS:
+        cases += ["/"]
+    @pytest.mark.parametrize("dirname", cases)
+    def test_dot(self, dirname):
+        aliases = PathAliases()
+        aliases.add(dirname, '/the/source')
+        the_file = os.path.join(dirname, 'a.py')
+        the_file = os.path.expanduser(the_file)
+        the_file = os.path.abspath(os.path.realpath(the_file))
+
+        assert '~' not in the_file  # to be sure the test is pure.
+        self.assert_mapped(aliases, the_file, '/the/source/a.py')
 
 
 class FindPythonFilesTest(CoverageTest):