Improve installing, add back tests for util

cpp_linter_hooks/clang_format.py

@@ -2,6 +2,7 @@
 from pathlib import Path
 import sys
 from argparse import ArgumentParser
+from typing import Tuple
 
 from .util import ensure_installed, DEFAULT_CLANG_VERSION
 
@@ -12,28 +13,28 @@
 parser.add_argument("--version", default=DEFAULT_CLANG_VERSION)
 
 
-def run_clang_format(version, args) -> int:
- command = [f'{BIN_PATH}/clang-format-{version}', '-i']
+def run_clang_format(version, args) -> Tuple[int, str]:
+ path = ensure_installed("clang-format", version)
+ command = [str(path), '-i']
  command.extend(args)
 
  retval = 0
  output = ""
  try:
  if "--dry-run" in command:
- sp = subprocess.run(command, stdout=subprocess.PIPE)
+ sp = subprocess.run(command, stdout=subprocess.PIPE, encoding="utf-8")
  retval = -1 # Not a fail just identify it's a dry-run.
- output = sp.stdout.decode("utf-8")
+ output = sp.stdout
  else:
  retval = subprocess.run(command, stdout=subprocess.PIPE).returncode
  return retval, output
  except FileNotFoundError as stderr:
  retval = 1
- return retval, stderr
+ return retval, str(stderr)
 
 
 def main() -> int:
  main_args, other_args = parser.parse_known_args()
- ensure_installed("clang-format", main_args.version)
  retval, output = run_clang_format(version=main_args.version, args=other_args)
  if retval != 0:
  print(output)

cpp_linter_hooks/clang_tidy.py

@@ -2,6 +2,7 @@
 from pathlib import Path
 import sys
 from argparse import ArgumentParser
+from typing import Tuple
 
 from .util import ensure_installed, DEFAULT_CLANG_VERSION
 
@@ -13,28 +14,28 @@
 
 
 
-def run_clang_tidy(version, args) -> int:
- command = [f'{BIN_PATH}/clang-tidy-{version}']
+def run_clang_tidy(version, args) -> Tuple[int, str]:
+ path = ensure_installed("clang-tidy", version)
+ command = [str(path)]
 
  command.extend(args)
 
  retval = 0
  output = ""
  try:
- sp = subprocess.run(command, stdout=subprocess.PIPE)
+ sp = subprocess.run(command, stdout=subprocess.PIPE, encoding='utf-8')
  retval = sp.returncode
- output = sp.stdout.decode("utf-8")
+ output = sp.stdout
  if "warning:" in output or "error:" in output:
  retval = 1
  return retval, output
  except FileNotFoundError as stderr:
  retval = 1
- return retval, stderr
+ return retval, str(stderr)
 
 
 def main() -> int:
  main_args, other_args = parser.parse_known_args()
- ensure_installed("clang-tidy", main_args.version)
  retval, output = run_clang_tidy(version=main_args.version, args=other_args)
  if retval != 0:
  print(output)

cpp_linter_hooks/util.py

@@ -1,8 +1,9 @@
 import sys
 from pathlib import Path
 import logging
+from typing import Optional
 
-from clang_tools.install import is_installed, install_tool
+from clang_tools.install import is_installed as _is_installed, install_tool
 
 
 LOG = logging.getLogger(__name__)
@@ -11,11 +12,37 @@
 DEFAULT_CLANG_VERSION = "13"
 
 
-def ensure_installed(tool_name: str, version: str = DEFAULT_CLANG_VERSION):
+def is_installed(tool_name: str, version: str) -> Optional[Path]:
+ """Check if tool is installed.
+
+ Checks the current python prefix and PATH via clang_tools.install.is_installed.
+ """
+ # check in current python prefix (usual situation when we installed into pre-commit venv)
+ directory = Path(sys.executable).parent
+ path = (directory / f"{tool_name}-{version}")
+ if path.is_file():
+ return path
+
+ # also check using clang_tools
+ path = _is_installed(tool_name, version)
+ if path is not None:
+ return Path(path)
+
+ # not found
+ return None
+
+
+def ensure_installed(tool_name: str, version: str = DEFAULT_CLANG_VERSION) -> Path:
+ """
+ Ensure tool is available at given version.
+ """
  LOG.info("Checking for %s, version %s", tool_name, version)
- if not is_installed(tool_name, version):
- LOG.info("Installing %s, version %s", tool_name, version)
- directory = Path(sys.executable).parent
- install_tool(tool_name, version, directory=str(directory), no_progress_bar=True)
- else:
+ path = is_installed(tool_name, version)
+ if path is not None:
  LOG.info("%s, version %s is already installed", tool_name, version)
+ return path
+
+ LOG.info("Installing %s, version %s", tool_name, version)
+ directory = Path(sys.executable).parent
+ install_tool(tool_name, version, directory=str(directory), no_progress_bar=True)
+ return directory / f"{tool_name}-{version}"

testing/main.c

@@ -1,8 +1,2 @@
 #include <stdio.h>
-int main()
-{
- for (;;)
- break;
- printf("Hello world!\n");
- return 0;
-}
+int main() {for (;;) break; printf("Hello world!\n");return 0;}

testing/run.sh

@@ -1,9 +1,10 @@
 rm -f result.txt
+git restore testing/main.c
 
 for config in testing/pre-commit-config.yaml testing/pre-commit-config-version.yaml; do
- git restore testing/main.c
  pre-commit clean
  pre-commit run -c $config --files testing/main.c | tee -a result.txt || true
+ git restore testing/main.c
 done
 
 failed_cases=`grep -c "Failed" result.txt`

tests/test_util.py

@@ -1,52 +1,38 @@
-from unittest.mock import patch
-
+import logging
+import sys
 import pytest
+from itertools import product
 
-from cpp_linter_hooks.util import check_installed
-from cpp_linter_hooks.util import get_expect_version
-from cpp_linter_hooks.util import install_clang_tools
-
-
-@pytest.mark.parametrize(('tool', 'expected_retval'), (('clang-format', 0), ('clang-tidy', 0),),)
-@patch('cpp_linter_hooks.util.subprocess.run')
-def test_check_installed(mock_subprocess_run, tool, expected_retval):
- mock_subprocess_run.return_value = expected_retval
- ret = check_installed(tool)
- assert ret == expected_retval
-
+from cpp_linter_hooks.util import ensure_installed, DEFAULT_CLANG_VERSION
 
-@pytest.mark.parametrize(('tool', 'version', 'expected_retval'), (('clang-format', '14', 0), ('clang-tidy', '14', 0),),)
-@patch('cpp_linter_hooks.util.subprocess.run')
-def test_check_installed_with_version(mock_subprocess_run, tool, version, expected_retval):
- mock_subprocess_run.return_value = expected_retval
- ret = check_installed(tool, version=version)
- assert ret == expected_retval
 
+VERSIONS = [None, "16"]
+TOOLS = ["clang-format", "clang-tidy"]
 
-@pytest.mark.parametrize(('tool', 'version', 'expected_retval'), (('non-exist-cmd', '14', 0),),)
-@patch('cpp_linter_hooks.util.subprocess.run')
-def test_check_installed_with_except(mock_subprocess_run, tool, version, expected_retval):
- mock_subprocess_run.return_value = expected_retval
- ret = check_installed(tool, version=version)
- assert ret == expected_retval
+@pytest.mark.parametrize(("tool", "version"), list(product(TOOLS, VERSIONS)))
+def test_ensure_installed(tool, version, tmp_path, monkeypatch, caplog):
 
+ bin_path = tmp_path / "bin"
+ with monkeypatch.context() as m:
+ m.setattr(sys, "executable", str(bin_path / "python"))
 
-@pytest.mark.parametrize(('version', 'expected_retval'), (('100', 1),),)
-@patch('cpp_linter_hooks.util.subprocess.run')
-def test_install_clang_tools(mock_subprocess_run, version, expected_retval):
- mock_subprocess_run.return_value = expected_retval
- try:
- ret = install_clang_tools(version)
- assert ret == expected_retval
- except Exception:
- pass
+ for run in range(2):
+ # clear any existing log messages
+ caplog.clear()
+ caplog.set_level(logging.INFO, logger="cpp_linter_hooks.util")
 
+ if version is not None:
+ ensure_installed(tool, version=version)
+ else:
+ ensure_installed(tool)
 
-def test_get_expect_version():
- args = ['clang-format', '--version 14']
- version = get_expect_version(args)
- assert version == '14'
+ bin_version = version or DEFAULT_CLANG_VERSION
+ assert (bin_path / f"{tool}-{bin_version}").is_file()
 
- args = ['clang-format', '--version=14']
- version = get_expect_version(args)
- assert version == '14'
+ # first run should install
+ assert caplog.record_tuples[0][2] == f"Checking for {tool}, version {bin_version}"
+ if run == 0:
+ assert caplog.record_tuples[1][2] == f"Installing {tool}, version {bin_version}"
+ # second run should just confirm it's already installed
+ else:
+ assert caplog.record_tuples[1][2] == f"{tool}, version {bin_version} is already installed"