AI review changes

Author: 2bndy5

cpp_linter_hooks/util.py

@@ -60,17 +60,18 @@ def parse_version(v: str):
 
 
 def _install_tool(tool: str, version: str) -> Optional[Path]:
-    """Install a tool using pip, suppressing output."""
+    """Install a tool using pip, logging output on failure."""
     result = subprocess.run(
         [sys.executable, "-m", "pip", "install", f"{tool}=={version}"],
         capture_output=True,
+        text=True,
     )
     if result.returncode == 0:
         return shutil.which(tool)
     else:
         LOG.error("pip failed to install %s %s", tool, version)
-        LOG.error(result.stdout.decode(encoding="utf-8"))
-        LOG.error(result.stderr.decode(encoding="utf-8"))
+        LOG.error(result.stdout)
+        LOG.error(result.stderr)
         return None
 
 

tests/test_util.py

@@ -136,6 +136,7 @@ def patched_run(*args, **kwargs):
         mock_run.assert_called_once_with(
             [sys.executable, "-m", "pip", "install", "clang-format==20.1.7"],
             capture_output=True,
+            text=True,
         )
 
 
@@ -145,7 +146,7 @@ def test_install_tool_failure():
 
     def patched_run(*args, **kwargs):
         return subprocess.CompletedProcess(
-            args, returncode=1, stderr=b"Error", stdout=b"Installation failed"
+            args, returncode=1, stderr="Error", stdout="Installation failed"
         )
 
     with (
@@ -159,7 +160,14 @@ def patched_run(*args, **kwargs):
 @pytest.mark.benchmark
 def test_install_tool_success_but_not_found():
     """Test _install_tool when install succeeds but tool not found in PATH."""
-    with patch("subprocess.run"), patch("shutil.which", return_value=None):
+
+    def patched_run(*args, **kwargs):
+        return subprocess.CompletedProcess(args, returncode=0)
+
+    with (
+        patch("subprocess.run", side_effect=patched_run),
+        patch("shutil.which", return_value=None),
+    ):
         result = _install_tool("clang-format", "20.1.7")
         assert result is None