Repository: Bachmann1234/diff_cover

Issue:
diff-quality doesn't consider untracked files
Hi,

so I just found out, that diff-quality doesn't consider untracked files. So new files which are staged are checked, but unstaged new files are ignore. The root cause is, that git-diff doesn't consider them as well. As far as I know, it's needed to include them manually by using "git ls-files ...".

I would like to suggest this as a feature (and I can do the PR of course): A new CLI flag which includes unstaged files.

Any feedback or ideas to this?

Discussion:
kasium: @Bachmann1234 as said, I created a quick and dirty PoC allowing diff-cover to see untracked files. The base is the "git ls-files" command. Afterwards these files are just added to the final list and all lines are marked as changed.

Of course I would add tests and a CLI flag to hide this feature. What do you think (It really took me only 20min or so to make this. If you think it has no benefit, let me just update the README)
Bachmann1234: not too bad. In the final This really should be behind a flag and not the default behavior IMO
kasium: Sure, I'll finalize it in the next days

Patch:
diff --git a/README.rst b/README.rst
index c09d16a..6e184e3 100644
--- a/README.rst
+++ b/README.rst
@@ -224,6 +224,15 @@ The following is executed for every changed file:
 #. if yes, check if the changed file is part of at least one include pattern
 #. check if the file is part of any exclude pattern
 
+Ignore/Include based on file status in git
+------------------------------------------
+Both ``diff-cover`` and ``diff-quality`` allow users to ignore and include files based on the git
+status: staged, unstaged, untracked:
+
+* ``--ignore-staged``: ignore all staged files (by default include them)
+* ``--ignore-unstaged``: ignore all unstaged files (by default include them)
+* ``--include-untracked``: include all untracked files (by default ignore them)
+
 Quiet mode
 ----------
 Both ``diff-cover`` and ``diff-quality`` support a quiet mode which is disable by default.
@@ -273,6 +282,11 @@ __ http://travis-ci.org
 **Solution**: ``diff-quality`` assumes you have the tool you wish to run against your diff installed. If you do not have it
 then install it with your favorite package manager.
 
+**Issue**: ``diff-quality`` reports no quality issues
+
+**Solution**: You might use a pattern like ``diff-quality --violations foo *.py``. The last argument
+is not used to specify the files but for the quality tool report. Remove it to resolve the issue
+
 License
 -------
 
diff --git a/diff_cover/diff_cover_tool.py b/diff_cover/diff_cover_tool.py
index a1299e5..b93fc1f 100644
--- a/diff_cover/diff_cover_tool.py
+++ b/diff_cover/diff_cover_tool.py
@@ -36,6 +36,7 @@ DIFF_RANGE_NOTATION_HELP = (
 )
 QUIET_HELP = "Only print errors and failures"
 SHOW_UNCOVERED = "Show uncovered lines on the console"
+INCLUDE_UNTRACKED_HELP = "Include untracked files"
 
 LOGGER = logging.getLogger(__name__)
 
@@ -119,6 +120,13 @@ def parse_coverage_args(argv):
         help=IGNORE_UNSTAGED_HELP,
     )
 
+    parser.add_argument(
+        "--include-untracked",
+        action="store_true",
+        default=False,
+        help=INCLUDE_UNTRACKED_HELP,
+    )
+
     parser.add_argument(
         "--exclude", metavar="EXCLUDE", type=str, nargs="+", help=EXCLUDE_HELP
     )
@@ -164,6 +172,7 @@ def generate_coverage_report(
     markdown_report=None,
     ignore_staged=False,
     ignore_unstaged=False,
+    include_untracked=False,
     exclude=None,
     src_roots=None,
     diff_range_notation=None,
@@ -179,6 +188,7 @@ def generate_coverage_report(
         git_diff=GitDiffTool(diff_range_notation, ignore_whitespace),
         ignore_staged=ignore_staged,
         ignore_unstaged=ignore_unstaged,
+        include_untracked=include_untracked,
         exclude=exclude,
     )
 
@@ -242,6 +252,7 @@ def main(argv=None, directory=None):
         css_file=arg_dict["external_css_file"],
         ignore_staged=arg_dict["ignore_staged"],
         ignore_unstaged=arg_dict["ignore_unstaged"],
+        include_untracked=arg_dict["include_untracked"],
         exclude=arg_dict["exclude"],
         src_roots=arg_dict["src_roots"],
         diff_range_notation=arg_dict["diff_range_notation"],
diff --git a/diff_cover/diff_quality_tool.py b/diff_cover/diff_quality_tool.py
index d8f916a..1047f28 100644
--- a/diff_cover/diff_quality_tool.py
+++ b/diff_cover/diff_quality_tool.py
@@ -22,6 +22,7 @@ from diff_cover.diff_cover_tool import (
     IGNORE_STAGED_HELP,
     IGNORE_UNSTAGED_HELP,
     IGNORE_WHITESPACE,
+    INCLUDE_UNTRACKED_HELP,
     QUIET_HELP,
 )
 from diff_cover.diff_reporter import GitDiffReporter
@@ -141,6 +142,13 @@ def parse_quality_args(argv):
         help=IGNORE_UNSTAGED_HELP,
     )
 
+    parser.add_argument(
+        "--include-untracked",
+        action="store_true",
+        default=False,
+        help=INCLUDE_UNTRACKED_HELP,
+    )
+
     parser.add_argument(
         "--exclude", metavar="EXCLUDE", type=str, nargs="+", help=EXCLUDE_HELP
     )
@@ -181,6 +189,7 @@ def generate_quality_report(
     css_file=None,
     ignore_staged=False,
     ignore_unstaged=False,
+    include_untracked=False,
     exclude=None,
     include=None,
     diff_range_notation=None,
@@ -198,6 +207,7 @@ def generate_quality_report(
         git_diff=GitDiffTool(diff_range_notation, ignore_whitespace),
         ignore_staged=ignore_staged,
         ignore_unstaged=ignore_unstaged,
+        include_untracked=include_untracked,
         supported_extensions=supported_extensions,
         exclude=exclude,
         include=include,
@@ -276,7 +286,8 @@ def main(argv=None, directory=None):
                     try:
                         input_reports.append(open(path, "rb"))
                     except OSError:
-                        LOGGER.warning("Could not load '%s'", path)
+                        LOGGER.error("Could not load report '%s'", path)
+                        return 1
                 reporter = QualityReporter(driver, input_reports, user_options)
 
             percent_passing = generate_quality_report(
@@ -286,6 +297,7 @@ def main(argv=None, directory=None):
                 css_file=arg_dict["external_css_file"],
                 ignore_staged=arg_dict["ignore_staged"],
                 ignore_unstaged=arg_dict["ignore_unstaged"],
+                include_untracked=arg_dict["include_untracked"],
                 exclude=arg_dict["exclude"],
                 include=arg_dict["include"],
                 diff_range_notation=arg_dict["diff_range_notation"],
diff --git a/diff_cover/diff_reporter.py b/diff_cover/diff_reporter.py
index 14d5320..e90fd56 100644
--- a/diff_cover/diff_reporter.py
+++ b/diff_cover/diff_reporter.py
@@ -112,6 +112,7 @@ class GitDiffReporter(BaseDiffReporter):
         git_diff=None,
         ignore_staged=None,
         ignore_unstaged=None,
+        include_untracked=False,
         supported_extensions=None,
         exclude=None,
         include=None,
@@ -126,6 +127,8 @@ class GitDiffReporter(BaseDiffReporter):
             options.append("staged")
         if not ignore_unstaged:
             options.append("unstaged")
+        if include_untracked:
+            options.append("untracked")
 
         # Branch is always present, so use as basis for name
         name = f"{compare_branch}...HEAD"
@@ -142,6 +145,7 @@ class GitDiffReporter(BaseDiffReporter):
         self._git_diff_tool = git_diff
         self._ignore_staged = ignore_staged
         self._ignore_unstaged = ignore_unstaged
+        self._include_untracked = include_untracked
         self._supported_extensions = supported_extensions
 
         # Cache diff information as a dictionary
@@ -162,6 +166,13 @@ class GitDiffReporter(BaseDiffReporter):
         # Get the diff dictionary
         diff_dict = self._git_diff()
 
+        # include untracked files
+        if self._include_untracked:
+            for path in self._git_diff_tool.untracked():
+                with open(path) as file_handle:
+                    num_lines = len(file_handle.readlines())
+                diff_dict[path] = list(range(1, num_lines + 1))
+
         # Return the changed file paths (dict keys)
         # in alphabetical order
         return sorted(diff_dict.keys(), key=lambda x: x.lower())
diff --git a/diff_cover/git_diff.py b/diff_cover/git_diff.py
index 75613b2..d3fccb1 100644
--- a/diff_cover/git_diff.py
+++ b/diff_cover/git_diff.py
@@ -102,3 +102,9 @@ class GitDiffTool:
         return execute(self._default_git_args + self._default_diff_args + ["--cached"])[
             0
         ]
+
+    def untracked(self):
+        """Return the untracked files."""
+        output = execute(["git", "ls-files", "--exclude-standard", "--others"])[0]
+        output = output.strip()
+        return output.split("\n") if output else []
diff --git a/diff_cover/git_path.py b/diff_cover/git_path.py
index 913149a..bad6949 100644
--- a/diff_cover/git_path.py
+++ b/diff_cover/git_path.py
@@ -42,9 +42,7 @@ class GitPathTool:
         # and src_path is `diff_cover/violations_reporter.py`
         # search for `violations_reporter.py`
         root_rel_path = os.path.relpath(cls._cwd, cls._root)
-        rel_path = os.path.relpath(git_diff_path, root_rel_path)
-
-        return rel_path
+        return os.path.relpath(git_diff_path, root_rel_path)
 
     @classmethod
     def absolute_path(cls, src_path):
diff --git a/pyproject.toml b/pyproject.toml
index c99dc90..7452977 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -2,10 +2,11 @@
 line-length = 88
 target-version = ['py37']
 include = '\.pyi?$'
-exclude = "diff_cover/tests/fixtures/*"
+exclude = "tests/fixtures/*"
 
 [tool.isort]
 profile = "black"
+extend_skip = "tests/fixtures/"
 
 [tool.pylint.master]
 max-line-length = 100
diff --git a/tox.ini b/tox.ini
index cb6b0a3..b5c9913 100644
--- a/tox.ini
+++ b/tox.ini
@@ -7,10 +7,10 @@ whitelist_externals =
 deps =
     -r{toxinidir}/test-requirements.txt
 commands =
-    black diff_cover
-    isort diff_cover
+    black diff_cover tests
+    isort diff_cover tests
     python -m pytest --cov --cov-report=xml tests
     git fetch origin main:refs/remotes/origin/main
-    diff-cover coverage.xml
-    diff-quality --violation=flake8
-    diff-quality --violation=pylint
+    diff-cover coverage.xml --include-untracked
+    diff-quality --violations flake8 --include-untracked
+    diff-quality --violations pylint --include-untracked


Test patch:
diff --git a/tests/test_diff_reporter.py b/tests/test_diff_reporter.py
index 25d4697..7d7b626 100644
--- a/tests/test_diff_reporter.py
+++ b/tests/test_diff_reporter.py
@@ -56,6 +56,13 @@ class GitDiffReporterTest(unittest.TestCase):
             "origin/main...HEAD",
         )
 
+    def test_name_include_untracked(self):
+        # Override the default branch
+        self.assertEqual(
+            GitDiffReporter(git_diff=self._git_diff, include_untracked=True).name(),
+            "origin/main...HEAD, staged, unstaged and untracked changes",
+        )
+
     def test_git_path_selection(self):
         for include, exclude, expected in [
             # no include/exclude --> use all paths
@@ -589,7 +596,26 @@ class GitDiffReporterTest(unittest.TestCase):
         sentinel = object()
         self.assertTrue(self.diff._fnmatch("file.py", [], default=sentinel) is sentinel)
 
-    def _set_git_diff_output(self, committed_diff, staged_diff, unstaged_diff):
+    def test_include_untracked(self):
+        self.diff = GitDiffReporter(git_diff=self._git_diff, include_untracked=True)
+
+        diff = git_diff_output(
+            {"subdir/file1.py": line_numbers(3, 10) + line_numbers(34, 47)}
+        )
+        self._set_git_diff_output(staged_diff=diff, untracked=["foo.py"])
+
+        open_mock = mock.mock_open(read_data="1\n2\n3\n")
+        with mock.patch("diff_cover.diff_reporter.open", open_mock):
+            changed = self.diff.src_paths_changed()
+
+        self.assertEqual(2, len(changed))
+        assert "subdir/file1.py" in changed
+        assert "foo.py" in changed
+        assert self.diff.lines_changed("foo.py") == [1, 2, 3]
+
+    def _set_git_diff_output(
+        self, committed_diff="", staged_diff="", unstaged_diff="", untracked=None
+    ):
         """
         Configure the git diff tool to return `committed_diff`,
         `staged_diff`, and `unstaged_diff` as outputs from
@@ -599,3 +625,4 @@ class GitDiffReporterTest(unittest.TestCase):
         self._git_diff.diff_committed.return_value = committed_diff
         self._git_diff.diff_staged.return_value = staged_diff
         self._git_diff.diff_unstaged.return_value = unstaged_diff
+        self._git_diff.untracked.return_value = untracked
diff --git a/tests/test_git_diff.py b/tests/test_git_diff.py
index 3cedb09..2521ec0 100644
--- a/tests/test_git_diff.py
+++ b/tests/test_git_diff.py
@@ -181,3 +181,17 @@ def test_errors(set_git_diff_output, tool):
 
     with pytest.raises(CommandError):
         tool.diff_unstaged()
+
+
+@pytest.mark.parametrize(
+    "output,expected",
+    [
+        ("", []),
+        ("\n", []),
+        ("a.py\n", ["a.py"]),
+        ("a.py\nb.py\n", ["a.py", "b.py"]),
+    ],
+)
+def test_untracked(tool, set_git_diff_output, output, expected):
+    set_git_diff_output(output, b"")
+    assert tool.untracked() == expected
diff --git a/tests/test_git_path.py b/tests/test_git_path.py
index ea70fe3..abd6a9f 100644
--- a/tests/test_git_path.py
+++ b/tests/test_git_path.py
@@ -1,81 +1,97 @@
-import unittest
-from unittest import mock
+# pylint: disable=missing-function-docstring
+
+"""Test for diff_cover.git_path"""
+
+import pytest
 
 from diff_cover.git_path import GitPathTool
 
 
-class TestGitPathTool(unittest.TestCase):
-    def setUp(self):
-        # Create mock subprocess to simulate `git rev-parse`
-        self.process = mock.Mock()
-        self.process.returncode = 0
-        self.subprocess = mock.patch("diff_cover.command_runner.subprocess").start()
-        self.subprocess.Popen.return_value = self.process
-        self.addCleanup(mock.patch.stopall)
+@pytest.fixture(autouse=True)
+def patch_git_path_tool(mocker):
+    mocker.patch.object(GitPathTool, "_root", None)
+    mocker.patch.object(GitPathTool, "_cwd", None)
+
+
+@pytest.fixture
+def process(mocker):
+    process_ = mocker.Mock()
+    process_.returncode = 0
+    return process_
+
+
+@pytest.fixture(autouse=True)
+def subprocess(mocker, process):
+    subprocess_ = mocker.patch("diff_cover.command_runner.subprocess")
+    subprocess_.Popen.return_value = process
+    return subprocess_
+
+
+def test_project_root_command(process, subprocess):
+    process.communicate.return_value = (b"/phony/path", b"")
+
+    GitPathTool.set_cwd(b"/phony/path")
+
+    # Expect that the correct command was executed
+    expected = ["git", "rev-parse", "--show-toplevel", "--encoding=utf-8"]
+    subprocess.Popen.assert_called_with(
+        expected, stdout=subprocess.PIPE, stderr=subprocess.PIPE
+    )
+
+
+def test_relative_path(process):
+    process.communicate.return_value = (b"/home/user/work/diff-cover", b"")
+
+    expected = "violations_reporter.py"
+    cwd = "/home/user/work/diff-cover/diff_cover"
 
-    def tearDown(self):
-        # Reset static class members
-        GitPathTool._root = None
-        GitPathTool._cwd = None
+    GitPathTool.set_cwd(cwd)
+    path = GitPathTool.relative_path("diff_cover/violations_reporter.py")
 
-    def test_project_root_command(self):
-        self._set_git_root(b"/phony/path")
+    # Expect relative path from diff_cover
+    assert path == expected
 
-        GitPathTool.set_cwd(b"/phony/path")
 
-        # Expect that the correct command was executed
-        expected = ["git", "rev-parse", "--show-toplevel", "--encoding=utf-8"]
-        self.subprocess.Popen.assert_called_with(
-            expected, stdout=self.subprocess.PIPE, stderr=self.subprocess.PIPE
-        )
+def test_absolute_path(process):
+    process.communicate.return_value = (
+        b"/home/user/work dir/diff-cover\n--encoding=utf-8\n",
+        b"",
+    )
 
-    def test_relative_path(self):
-        self._set_git_root(b"/home/user/work/diff-cover")
-        expected = "violations_reporter.py"
-        cwd = "/home/user/work/diff-cover/diff_cover"
+    expected = "/home/user/work dir/diff-cover/other_package/file.py"
+    cwd = "/home/user/work dir/diff-cover/diff_cover"
 
-        GitPathTool.set_cwd(cwd)
-        path = GitPathTool.relative_path("diff_cover/violations_reporter.py")
+    GitPathTool.set_cwd(cwd)
+    path = GitPathTool.absolute_path("other_package/file.py")
 
-        # Expect relative path from diff_cover
-        self.assertEqual(path, expected)
+    # Expect absolute path to file.py
+    assert path == expected
 
-    def test_absolute_path(self):
-        self._set_git_root(b"/home/user/work dir/diff-cover\n--encoding=utf-8\n")
-        expected = "/home/user/work dir/diff-cover/other_package/file.py"
-        cwd = "/home/user/work dir/diff-cover/diff_cover"
 
-        GitPathTool.set_cwd(cwd)
-        path = GitPathTool.absolute_path("other_package/file.py")
+def test_set_cwd_unicode(process):
+    process.communicate.return_value = (b"\xe2\x94\xbb\xe2\x94\x81\xe2\x94\xbb", b"")
 
-        # Expect absolute path to file.py
-        self.assertEqual(path, expected)
+    expected = "\u253b\u2501\u253b/other_package/file.py"
+    cwd = "\\u253b\\u2501\\u253b/diff_cover\n--encoding=utf-8\n"
 
-    def test_set_cwd_unicode(self):
-        self._set_git_root(b"\xe2\x94\xbb\xe2\x94\x81\xe2\x94\xbb")
-        expected = "\u253b\u2501\u253b/other_package/file.py"
-        cwd = "\\u253b\\u2501\\u253b/diff_cover\n--encoding=utf-8\n"
+    GitPathTool.set_cwd(cwd)
+    path = GitPathTool.absolute_path("other_package/file.py")
 
-        GitPathTool.set_cwd(cwd)
-        path = GitPathTool.absolute_path("other_package/file.py")
+    # Expect absolute path to file.py
+    assert path == expected
 
-        # Expect absolute path to file.py
-        self.assertEqual(path, expected)
 
-    def test_set_cwd_unicode_byte_passed_in_for_cwd(self):
-        self._set_git_root(b"\xe2\x94\xbb\xe2\x94\x81\xe2\x94\xbb\n--encoding=utf-8\n")
-        expected = "\u253b\u2501\u253b/other_package/file.py"
-        cwd = b"\\u253b\\u2501\\u253b/diff_cover"
+def test_set_cwd_unicode_byte_passed_in_for_cwd(process):
+    process.communicate.return_value = (
+        b"\xe2\x94\xbb\xe2\x94\x81\xe2\x94\xbb\n--encoding=utf-8\n",
+        b"",
+    )
 
-        GitPathTool.set_cwd(cwd)
-        path = GitPathTool.absolute_path("other_package/file.py")
+    expected = "\u253b\u2501\u253b/other_package/file.py"
+    cwd = b"\\u253b\\u2501\\u253b/diff_cover"
 
-        # Expect absolute path to file.py
-        self.assertEqual(path, expected)
+    GitPathTool.set_cwd(cwd)
+    path = GitPathTool.absolute_path("other_package/file.py")
 
-    def _set_git_root(self, git_root):
-        """
-        Configure the process mock to output `stdout`
-        to a given git project root.
-        """
-        self.process.communicate.return_value = (git_root, b"")
+    # Expect absolute path to file.py
+    assert path == expected
diff --git a/tests/test_integration.py b/tests/test_integration.py
index 4d4c2cf..9d08974 100644
--- a/tests/test_integration.py
+++ b/tests/test_integration.py
@@ -18,8 +18,8 @@ from diff_cover.diff_cover_tool import main as diff_cover_main
 from diff_cover.diff_quality_tool import QUALITY_DRIVERS
 from diff_cover.diff_quality_tool import main as diff_quality_main
 from diff_cover.git_path import GitPathTool
-from tests.helpers import assert_long_str_equal, fixture_path
 from diff_cover.violationsreporters.base import QualityDriver
+from tests.helpers import assert_long_str_equal, fixture_path
 
 
 class ToolsIntegrationBase(unittest.TestCase):
@@ -540,7 +540,7 @@ class DiffQualityIntegrationTest(ToolsIntegrationBase):
         self._check_console_report(
             "git_diff_violations.txt",
             "pyflakes_violations_report.txt",
-            ["diff-quality", "--violations=pyflakes", "pyflakes_report.txt"],
+            ["diff-quality", "--violations=pyflakes", "pyflakes_violations_report.txt"],
         )
 
     def test_pre_generated_pylint_report(self):
@@ -571,7 +571,9 @@ class DiffQualityIntegrationTest(ToolsIntegrationBase):
         """
         with open("git_diff_add.txt", encoding="utf-8") as git_diff_file:
             self._set_git_diff_output(git_diff_file.read(), "")
-        argv = ["diff-quality", f"--violations={tool_name}", report_arg]
+        argv = ["diff-quality", f"--violations={tool_name}"]
+        if report_arg:
+            argv.append(report_arg)
 
         with patch("diff_cover.diff_quality_tool.LOGGER") as logger:
             exit_value = diff_quality_main(argv)
@@ -592,7 +594,7 @@ class DiffQualityIntegrationTest(ToolsIntegrationBase):
             self._call_quality_expecting_error(
                 "not_installed",
                 ("Failure: '%s'", "not_installed is not installed"),
-                report_arg="",
+                report_arg=None,
             )
         finally:
             # Cleaning is good for the soul... and other tests
diff --git a/tests/test_report_generator.py b/tests/test_report_generator.py
index 339cd9b..4a4fe52 100644
--- a/tests/test_report_generator.py
+++ b/tests/test_report_generator.py
@@ -13,11 +13,11 @@ from diff_cover.report_generator import (
     StringReportGenerator,
     TemplateReportGenerator,
 )
-from tests.helpers import assert_long_str_equal, load_fixture
 from diff_cover.violationsreporters.violations_reporter import (
     BaseViolationReporter,
     Violation,
 )
+from tests.helpers import assert_long_str_equal, load_fixture
 
 
 class SimpleReportGenerator(BaseReportGenerator):