diff --git a/CHANGES.md b/CHANGES.md index 187bb9fc..40cdfc69 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,12 @@ ## Unreleased +* Improve `GitFileSystem` to support symbolic links by using `git cat-file + --batch --follow-symlinks`. +* Improve `GitFileSystem` to support source files located in submodules. +* Use a slightly darker yellow in the HTML report for the numbers of + partially covered lines, for readability. + * Add support for Python 3.12 and 3.13. ## 4.0.0 (2025-04-13) diff --git a/pycobertura/filesystem.py b/pycobertura/filesystem.py index 42624976..2ea8b22a 100644 --- a/pycobertura/filesystem.py +++ b/pycobertura/filesystem.py @@ -3,7 +3,6 @@ import io import zipfile import subprocess -import shlex from contextlib import contextmanager @@ -81,56 +80,172 @@ def __init__(self, repo_folder, ref): # the report may have been collected in a subfolder of the repository # root. Each file path shall thus be completed by the prefix. self.prefix = self.repository.replace(self.repository_root, "").lstrip("/") + # Cache submodule paths and commit SHAs for the provided ref + self._submodules = self._discover_submodules() + + def _git_cat_file_check(self, repo_root, spec): + """ + Call `git cat-file --batch-check --follow-symlinks` + and return existence as bool. + """ + args = ["git", "cat-file", "--batch-check", "--follow-symlinks"] + input_data = f"{spec}\n".encode() + try: + process = subprocess.Popen( + args, + cwd=repo_root, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + output, _ = process.communicate(input=input_data) + return_code = process.wait() + except (OSError, subprocess.CalledProcessError): + return False + return return_code == 0 and not output.endswith(b"missing\n") + + def _git_cat_file_read(self, repo_root, spec): + """ + Call `git cat-file --batch --follow-symlinks` and return blob content as bytes. + Raises FileNotFound if the object is missing or on error. + """ + args = ["git", "cat-file", "--batch", "--follow-symlinks"] + input_data = f"{spec}\n".encode() + try: + process = subprocess.Popen( + args, + cwd=repo_root, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + output, _ = process.communicate(input=input_data) + return_code = process.wait() + except (OSError, subprocess.CalledProcessError): + raise self.FileNotFound(spec) + + if return_code != 0 or output.endswith(b"missing\n"): + raise self.FileNotFound(spec) + lines = output.split(b"\n", 1) + if len(lines) < 2: + raise self.FileNotFound(spec) + return lines[1] def real_filename(self, filename): - prefix = "{}/".format(self.prefix) if self.prefix else "" - return "{ref}:{prefix}{filename}".format( - prefix=prefix, ref=self.ref, filename=filename - ) + """ + Constructs the Git path for a given filename. + This method should NOT resolve symlinks on the local disk. + """ + prefix = f"{self.prefix}/" if self.prefix else "" + return f"{self.ref}:{prefix}{filename}" def has_file(self, filename): - command = "git --no-pager show {}".format(self.real_filename(filename)) - return_code = subprocess.call( - command, - cwd=self.repository, - shell=True, - stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL, - ) - return not bool(return_code) + """ + Check for a file's existence in the specified commit's tree. + """ + # If the file is within a submodule, query the submodule repository + submodule_ctx = self._resolve_submodule_ctx(filename) + if submodule_ctx is not None: + submodule_root, sub_commit, rel_path = submodule_ctx + return self._git_cat_file_check(submodule_root, f"{sub_commit}:{rel_path}") + + real_filename = self.real_filename(filename) + return self._git_cat_file_check(self.repository_root, real_filename) def _get_root_path(self, repository_folder): - command = "git rev-parse --show-toplevel" - command_tokens = shlex.split(command) + command = ["git", "rev-parse", "--show-toplevel"] try: - output = subprocess.check_output(command_tokens, cwd=repository_folder) + output = subprocess.check_output(command, cwd=repository_folder) except (OSError, subprocess.CalledProcessError): raise ValueError( - "The folder {} is not " - "a valid git repository.".format(repository_folder) + f"The folder {repository_folder} is not a valid git repository." ) - return output.decode("utf-8").rstrip() @contextmanager def open(self, filename): """ - Yield a file-like object for file `filename`. + Yield a file-like object for the given filename, + following symlinks if necessary. This function is a context manager. """ - filename = self.real_filename(filename) + # If the file is within a submodule, + # read from the submodule repository at the pinned commit + submodule_ctx = self._resolve_submodule_ctx(filename) + if submodule_ctx is not None: + submodule_root, sub_commit, rel_path = submodule_ctx + content = self._git_cat_file_read( + submodule_root, f"{sub_commit}:{rel_path}" + ) + yield io.StringIO(content.decode("utf-8")) + return - command = "git --no-pager show {}".format(filename) - command_tokens = shlex.split(command) + real_filename = self.real_filename(filename) + content = self._git_cat_file_read(self.repository_root, real_filename) + yield io.StringIO(content.decode("utf-8")) + def _discover_submodules(self): + """ + Discover submodule paths and SHAs for the given ref by inspecting the tree. + Returns a mapping of submodule path -> commit SHA. + """ try: - output = subprocess.check_output(command_tokens, cwd=self.repository) + output = subprocess.check_output( + [ + "git", + "ls-tree", + "-r", + "--full-tree", + self.ref, + ], + cwd=self.repository_root, + ) except (OSError, subprocess.CalledProcessError): - raise self.FileNotFound(filename) - - output = output.decode("utf-8").rstrip() - yield io.StringIO(output) + return {} + + submodules = {} + for line in output.decode("utf-8", errors="replace").splitlines(): + # Expected format: "160000 commit \t" + try: + meta, path = line.split("\t", 1) + except ValueError: + continue + parts = meta.split() + if len(parts) < 3: + continue + mode, obj_type, sha = parts[0], parts[1], parts[2] + if mode == "160000" and obj_type == "commit": + submodules[path] = sha + return submodules + + def _resolve_submodule_ctx(self, filename): + """ + If the path points into a submodule, return a tuple of + (submodule_root_abs_path, submodule_commit_sha, relative_path_inside_submodule). + Otherwise, return None. + """ + # Find the longest matching submodule path that prefixes the filename + matching = [ + p + for p in self._submodules.keys() + if filename == p or filename.startswith(p + "/") + ] + if not matching: + return None + # Use the longest (deepest) match in case of nested submodules + sub_path = max(matching, key=len) + sub_sha = self._submodules.get(sub_path) + if not sub_sha: + return None + + rel_path = filename.removeprefix(sub_path).lstrip("/") + submodule_root = os.path.join(self.repository_root, sub_path) + if not os.path.isdir(submodule_root): + # Submodule not checked out; + # we cannot read without local checkout of objects + return None + return (submodule_root, sub_sha, rel_path) def filesystem_factory(source, source_prefix=None, ref=None): diff --git a/pycobertura/templates/html-delta.jinja2 b/pycobertura/templates/html-delta.jinja2 index 3ec62172..ab46610a 100644 --- a/pycobertura/templates/html-delta.jinja2 +++ b/pycobertura/templates/html-delta.jinja2 @@ -7,7 +7,7 @@ {% include 'skeleton.css' %} .red {color: red} .green {color: green} -.yellow {color: yellow} +.yellow {color: #FFD700} .hit {background-color: #E6FFEC} .miss {background-color: #FFEBE9} .partial {background-color: #FFFECD} diff --git a/tests/test_filesystem.py b/tests/test_filesystem.py index 2e9f4db5..af4ab535 100644 --- a/tests/test_filesystem.py +++ b/tests/test_filesystem.py @@ -1,5 +1,6 @@ import subprocess from unittest.mock import patch, MagicMock +import pytest FIRST_PYCOBERTURA_COMMIT_SHA = "d1fe88da6b18340762b24bb1f89067a3439c4041" @@ -121,21 +122,30 @@ def test_filesystem_git(): import pycobertura.filesystem as fsm branch, folder, filename = "master", "tests/dummy", "test-file" + repo_root = "/tmp/repo" with patch.object(fsm, "subprocess") as subprocess_mock: - subprocess_mock.check_output = MagicMock(return_value=b"") + # Mock for _get_root_path + subprocess_mock.check_output.return_value = repo_root.encode('utf-8') + + # Mock for open + mock_process = MagicMock() + mock_process.communicate.return_value = ('some_hash blob 12\n'.encode(), b'') + mock_process.wait.return_value = 0 + subprocess_mock.Popen.return_value = mock_process fs = fsm.GitFileSystem(folder, branch) with fs.open(filename) as f: assert hasattr(f, 'read') - expected_git_filename = "master:tests/dummy/test-file" + expected_git_filename = f"{branch}:{folder}/{filename}" git_filename = fs.real_filename(filename) assert git_filename == expected_git_filename - expected_command = ["git", "--no-pager", "show", git_filename] - subprocess_mock.check_output.assert_called_with(expected_command, cwd=folder) + expected_command = ["git", "cat-file", "--batch", "--follow-symlinks"] + subprocess_mock.Popen.assert_called_with(expected_command, cwd=repo_root, stdin=subprocess_mock.PIPE, + stdout=subprocess_mock.PIPE, stderr=subprocess_mock.PIPE) def test_filesystem_git_integration(): @@ -184,22 +194,6 @@ def test_filesystem_git__git_not_found(): assert folder in str(e) -def test_filesystem_git_integration(): - from pycobertura.filesystem import GitFileSystem - - fs = GitFileSystem(".", FIRST_PYCOBERTURA_COMMIT_SHA) - - # Files included in pycobertura's first commit. - source_files = [ - "README.md", - ".gitignore", - ] - - for source_file in source_files: - with fs.open(source_file) as f: - assert hasattr(f, "read") - - def test_filesystem_git_has_file_integration(): from pycobertura.filesystem import GitFileSystem @@ -222,11 +216,11 @@ def test_filesystem_git_integration__not_found(): dummy_source_file = "CHANGES.md" - try: + with pytest.raises(GitFileSystem.FileNotFound) as excinfo: with fs.open(dummy_source_file) as f: pass - except GitFileSystem.FileNotFound as fnf: - assert fnf.path == fs.real_filename(dummy_source_file) + + assert excinfo.value.path == fs.real_filename(dummy_source_file) def test_filesystem_git_has_file_integration__not_found(): diff --git a/tests/test_reporters.py b/tests/test_reporters.py index ce5a0e6a..fce69d52 100644 --- a/tests/test_reporters.py +++ b/tests/test_reporters.py @@ -528,7 +528,7 @@ def test_html_report_delta(): html_output = report_delta.generate() assert '.red {color: red}' in html_output assert '.green {color: green}' in html_output - assert '.yellow {color: yellow}' in html_output + assert '.yellow {color: #FFD700}' in html_output assert "normalize.css" in html_output assert "Skeleton V2.0" in html_output