From 1052cd93eac5237c48a0f94217606c08a5245771 Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Tue, 2 Jan 2024 02:52:48 -0800 Subject: [PATCH] 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 | 3 +- cpp_linter/common_fs.py | 18 +++----- cpp_linter/git/__init__.py | 2 +- cpp_linter/loggers.py | 2 +- pyproject.toml | 4 +- .../capture_tools_output/test_tools_output.py | 44 ++++++++++++++++++ tests/test_git_str.py | 45 +++++++++++-------- tests/test_misc.py | 7 ++- 8 files changed, 86 insertions(+), 39 deletions(-) diff --git a/cpp_linter/clang_tools/clang_tidy.py b/cpp_linter/clang_tools/clang_tidy.py index 0d442a2..11d8dea 100644 --- a/cpp_linter/clang_tools/clang_tidy.py +++ b/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) diff --git a/cpp_linter/common_fs.py b/cpp_linter/common_fs.py index 6b91029..5b25962 100644 --- a/cpp_linter/common_fs.py +++ b/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( diff --git a/cpp_linter/git/__init__.py b/cpp_linter/git/__init__.py index 4fafb6b..b23bb8d 100644 --- a/cpp_linter/git/__init__.py +++ b/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" ) diff --git a/cpp_linter/loggers.py b/cpp_linter/loggers.py index 6460d40..39e2380 100644 --- a/cpp_linter/loggers.py +++ b/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 diff --git a/pyproject.toml b/pyproject.toml index 5fcbc3b..454f612 100644 --- a/pyproject.toml +++ b/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", ] diff --git a/tests/capture_tools_output/test_tools_output.py b/tests/capture_tools_output/test_tools_output.py index cdf6602..02d90b8 100644 --- a/tests/capture_tools_output/test_tools_output.py +++ b/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" 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] diff --git a/tests/test_git_str.py b/tests/test_git_str.py index 837be45..4c20467 100644 --- a/tests/test_git_str.py +++ b/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 ", + "+#include ", + " ", + " ", + " \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 ", - "+#include ", - " ", - " ", - " \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 diff --git a/tests/test_misc.py b/tests/test_misc.py index 7bae06b..f9eeeda 100644 --- a/tests/test_misc.py +++ b/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", [