bring coverage back up to previously on main

only check format comment if checks failed

not all versions of clang-format trigger on our test case for using a .clang-format config file

cpp_linter/clang_tools/clang_tidy.py

@@ -135,7 +135,8 @@ def run_clang_tidy(
     if len(extra_args) == 1 and " " in extra_args[0]:
         extra_args = extra_args[0].split()
     for extra_arg in extra_args:
-        cmds.append(f"--extra-arg={extra_arg}")
+        arg = extra_arg.strip('"')
+        cmds.append(f"--extra-arg=\"{arg}\"")
     cmds.append(filename)
     logger.info('Running "%s"', " ".join(cmds))
     results = subprocess.run(cmds, capture_output=True)

cpp_linter/common_fs.py

@@ -105,9 +105,7 @@ def is_file_in_list(paths: List[str], file_name: str, prompt: str) -> bool:
         - False if ``file_name`` is not in the ``paths`` list.
     """
     for path in paths:
-        result = commonpath(
-            [PurePath(path).as_posix(), PurePath(file_name).as_posix()]
-        )
+        result = commonpath([PurePath(path).as_posix(), PurePath(file_name).as_posix()])
         if result == path:
             logger.debug(
                 '"./%s" is %s as specified in the domain "./%s"',
@@ -118,6 +116,7 @@ def is_file_in_list(paths: List[str], file_name: str, prompt: str) -> bool:
             return True
     return False
 
+
 def is_source_or_ignored(
     file_name: str,
     ext_list: List[str],
@@ -136,15 +135,10 @@ def is_source_or_ignored(
         True if there are files to check. False will invoke a early exit (in
         `main()`) when no files to be checked.
     """
-    if (
-        PurePath(file_name).suffix.lstrip(".") in ext_list
-        and (
-            not is_file_in_list(ignored, file_name, "ignored")
-            or is_file_in_list(not_ignored, file_name, "not ignored")
-        )
-    ):
-        return True
-    return False
+    return PurePath(file_name).suffix.lstrip(".") in ext_list and (
+        not is_file_in_list(ignored, file_name, "ignored")
+        or is_file_in_list(not_ignored, file_name, "not ignored")
+    )
 
 
 def list_source_files(

cpp_linter/git/__init__.py

@@ -72,7 +72,7 @@ def get_diff(parents: int = 1) -> Diff:
         diff_name = f"{head.short_id}...{base.short_id}"
 
     logger.info("getting diff between %s", diff_name)
-    if logger.getEffectiveLevel() <= logging.DEBUG:
+    if logger.getEffectiveLevel() <= logging.DEBUG:  # pragma: no cover
         Path(CACHE_PATH, f"{diff_name}.diff").write_text(
             diff_obj.patch or "", encoding="utf-8"
         )

cpp_linter/loggers.py

@@ -3,7 +3,7 @@
 from requests import Response
 
 FOUND_RICH_LIB = False
-try:
+try:  # pragma: no cover
     from rich.logging import RichHandler  # type: ignore
 
     FOUND_RICH_LIB = True

pyproject.toml

@@ -92,8 +92,8 @@ exclude_lines = [
     "def main",
     # ignore any branch that makes the module executable
     'if __name__ == "__main__"',
-    # ignore branches specific to type checking
-    "if TYPE_CHECKING",
+    # ignore missing implementations in an abstract class
+    "raise NotImplementedError",
     # ignore the local secific debug statement related to not having rich installed
     "if not FOUND_RICH_LIB",
 ]

tests/capture_tools_output/test_tools_output.py

@@ -20,6 +20,7 @@
 from cpp_linter.clang_tools import capture_clang_tools_output
 from cpp_linter.loggers import log_commander, logger
 from cpp_linter.rest_api.github_api import GithubApiClient
+from cpp_linter.cli import cli_arg_parser
 
 CLANG_VERSION = os.getenv("CLANG_VERSION", "16")
 
@@ -254,6 +255,15 @@ def test_format_annotations(
 
     caplog.set_level(logging.INFO, logger=log_commander.name)
     log_commander.propagate = True
+
+    # check thread comment
+    comment, format_checks_failed, _ = gh_client.make_comment(
+        files, format_advice, tidy_advice
+    )
+    if format_checks_failed:
+        assert f"{format_checks_failed} file(s) not formatted</strong>" in comment
+
+    # check annotations
     gh_client.make_annotations(files, format_advice, tidy_advice, style)
     for message in [r.message for r in caplog.records if r.levelno == logging.INFO]:
         if FORMAT_RECORD.search(message) is not None:
@@ -427,3 +437,37 @@ def test_parse_diff(
         assert files
     else:
         assert not files
+
+
+@pytest.mark.parametrize(
+    "input",
+    [["-std=c++17", "-Wall"], ["-std=c++17 -Wall"]],
+    ids=["separate", "unified"],
+)
+def test_tidy_extra_args(caplog: pytest.LogCaptureFixture, input: List[str]):
+    """Just make sure --extra-arg is passed to clang-tidy properly"""
+    cli_in = []
+    for a in input:
+        cli_in.append(f'--extra-arg="{a}"')
+    caplog.set_level(logging.INFO, logger=logger.name)
+    args = cli_arg_parser.parse_args(cli_in)
+    assert len(input) == len(args.extra_arg)
+    _, _ = capture_clang_tools_output(
+        files=[FileObj("test/demo/demo.cpp", [], [])],
+        version=CLANG_VERSION,
+        checks="",  # use .clang-tidy config
+        style="",  # disable clang-format
+        lines_changed_only=0,
+        database="",
+        extra_args=args.extra_arg,
+    )
+    messages = [
+        r.message
+        for r in caplog.records
+        if r.levelno == logging.INFO and r.message.startswith("Running")
+    ]
+    assert messages
+    if len(input) == 1 and " " in input[0]:
+        input = input[0].split()
+    for a in input:
+        assert f'--extra-arg="{a}"' in messages[0]

tests/test_git_str.py

@@ -5,6 +5,24 @@
 from cpp_linter.git.git_str import parse_diff as parse_diff_str
 
 
+TYPICAL_DIFF = "\n".join(
+    [
+        "diff --git a/path/for/Some file.cpp b/path/to/Some file.cpp",
+        "--- a/path/for/Some file.cpp",
+        "+++ b/path/to/Some file.cpp",
+        "@@ -3,7 +3,7 @@",
+        " ",
+        " ",
+        " ",
+        "-#include <some_lib/render/animation.hpp>",
+        "+#include <some_lib/render/animations.hpp>",
+        " ",
+        " ",
+        " \n",
+    ]
+)
+
+
 def test_pygit2_bug1260(caplog: pytest.LogCaptureFixture):
     """This test the legacy approach of parsing a diff str using pure python regex
     patterns.
@@ -29,24 +47,8 @@ def test_pygit2_bug1260(caplog: pytest.LogCaptureFixture):
 
 def test_typical_diff():
     """For coverage completeness. Also tests for files with spaces in the names."""
-    diff_str = "\n".join(
-        [
-            "diff --git a/path/for/Some file.cpp b/path/to/Some file.cpp",
-            "--- a/path/for/Some file.cpp",
-            "+++ b/path/to/Some file.cpp",
-            "@@ -3,7 +3,7 @@",
-            " ",
-            " ",
-            " ",
-            "-#include <some_lib/render/animation.hpp>",
-            "+#include <some_lib/render/animations.hpp>",
-            " ",
-            " ",
-            " \n",
-        ]
-    )
-    from_c = parse_diff(diff_str, ["cpp"], [], [])
-    from_py = parse_diff_str(diff_str, ["cpp"], [], [])
+    from_c = parse_diff(TYPICAL_DIFF, ["cpp"], [], [])
+    from_py = parse_diff_str(TYPICAL_DIFF, ["cpp"], [], [])
     assert [f.serialize() for f in from_c] == [f.serialize() for f in from_py]
     for file_obj in from_c:
         # file name should have spaces
@@ -65,3 +67,10 @@ def test_binary_diff():
     files = parse_diff_str(diff_str, ["cpp"], [], [])
     # binary files are ignored during parsing
     assert not files
+
+
+def test_ignored_diff():
+    """For coverage completeness"""
+    files = parse_diff_str(TYPICAL_DIFF, ["hpp"], [], [])
+    # binary files are ignored during parsing
+    assert not files

tests/test_misc.py

@@ -80,7 +80,7 @@ def test_response_logs(url: str):
 @pytest.mark.parametrize(
     "extensions",
     [
-        (["cpp", "hpp"]),
+        (["cpp", "hpp", "yml"]),  # yml included to traverse .github folder
         pytest.param(["cxx"], marks=pytest.mark.xfail),
     ],
 )
@@ -89,15 +89,14 @@ def test_list_src_files(
     caplog: pytest.LogCaptureFixture,
     extensions: List[str],
 ):
-    """List the source files in the demo folder of this repo."""
-    monkeypatch.chdir(Path(__file__).parent.as_posix())
+    """List the source files in the root folder of this repo."""
+    monkeypatch.chdir(Path(__file__).parent.parent.as_posix())
     caplog.set_level(logging.DEBUG, logger=logger.name)
     files = list_source_files(ext_list=extensions, ignored_paths=[], not_ignored=[])
     assert files
     for file in files:
         assert Path(file.name).suffix.lstrip(".") in extensions
 
-
 @pytest.mark.parametrize(
     "pseudo,expected_url,fake_runner",
     [