diff --git a/.github/workflows/ci-nightly.yml b/.github/workflows/ci-nightly.yml index 0188ebf2524..bf9d42e9f56 100644 --- a/.github/workflows/ci-nightly.yml +++ b/.github/workflows/ci-nightly.yml @@ -46,7 +46,7 @@ jobs: run: | python -m pip install pytest # Standalone CI tool tests; skip repo-root conftest.py (imports cuda.pathfinder). - python -m pytest -v --noconftest ci/tools/tests + python -m pytest -v --noconftest ci/tools/tests toolshed/tests find-wheels: runs-on: ubuntu-latest diff --git a/toolshed/check_precommit_installed.py b/toolshed/check_precommit_installed.py index 19d7cfc20b6..f6145ca58df 100644 --- a/toolshed/check_precommit_installed.py +++ b/toolshed/check_precommit_installed.py @@ -20,25 +20,39 @@ MARKER = b"File generated by pre-commit" -def _git_hooks_dir() -> str: - result = subprocess.run( - ["git", "rev-parse", "--git-path", "hooks"], # noqa: S607 - capture_output=True, - check=True, - text=True, - ) - return result.stdout.strip() +def _git_hooks_dir() -> str | None: + """Resolve the git hooks directory, or None if git cannot tell us. + + Anything that stops git from answering -- not inside a work tree, git + missing from PATH, a broken repository -- leaves us unable to confirm the + hook is installed, which is the same outcome as "not installed". This + check is advisory (see the module docstring), so it must never turn a + `pre-commit run` into a failure. + """ + try: + result = subprocess.run( + ["git", "rev-parse", "--git-path", "hooks"], # noqa: S607 + capture_output=True, + check=True, + text=True, + ) + except (OSError, subprocess.SubprocessError): + return None + return result.stdout.strip() or None def main() -> int: hooks_dir = _git_hooks_dir() - hook_path = f"{hooks_dir}/pre-commit" - try: - with open(hook_path, "rb") as f: - installed = MARKER in f.read() - except FileNotFoundError: - installed = False + installed = False + if hooks_dir is not None: + try: + with open(f"{hooks_dir}/pre-commit", "rb") as f: + installed = MARKER in f.read() + except OSError: + # Unreadable, a directory, a dangling symlink: all mean we cannot + # confirm an installed hook, and none of them justify failing. + installed = False if not installed: print( diff --git a/toolshed/tests/test_check_precommit_installed.py b/toolshed/tests/test_check_precommit_installed.py new file mode 100644 index 00000000000..d204ec7a2d4 --- /dev/null +++ b/toolshed/tests/test_check_precommit_installed.py @@ -0,0 +1,86 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +from __future__ import annotations + +import os +import subprocess +import sys + +import pytest + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) +import check_precommit_installed as mod + +WARNING_TEXT = "pre-commit git hook is not installed" + + +def _write_hook(hooks_dir, body=mod.MARKER): + hooks_dir.mkdir(parents=True, exist_ok=True) + (hooks_dir / "pre-commit").write_bytes(body) + + +@pytest.mark.agent_authored(model="claude-opus-5") +def test_installed_hook_is_quiet(tmp_path, monkeypatch, capsys): + hooks = tmp_path / "hooks" + _write_hook(hooks, b"#!/bin/sh\n# File generated by pre-commit\n") + monkeypatch.setattr(mod, "_git_hooks_dir", lambda: str(hooks)) + + assert mod.main() == 0 + assert WARNING_TEXT not in capsys.readouterr().err + + +@pytest.mark.agent_authored(model="claude-opus-5") +def test_missing_hook_warns_and_succeeds(tmp_path, monkeypatch, capsys): + monkeypatch.setattr(mod, "_git_hooks_dir", lambda: str(tmp_path / "hooks")) + + assert mod.main() == 0 + assert WARNING_TEXT in capsys.readouterr().err + + +@pytest.mark.agent_authored(model="claude-opus-5") +def test_foreign_hook_warns_and_succeeds(tmp_path, monkeypatch, capsys): + """A hook installed by something other than pre-commit lacks the marker.""" + hooks = tmp_path / "hooks" + _write_hook(hooks, b"#!/bin/sh\necho hand-written hook\n") + monkeypatch.setattr(mod, "_git_hooks_dir", lambda: str(hooks)) + + assert mod.main() == 0 + assert WARNING_TEXT in capsys.readouterr().err + + +@pytest.mark.agent_authored(model="claude-opus-5") +def test_outside_a_git_work_tree_warns_and_succeeds(tmp_path, monkeypatch, capsys): + """`git rev-parse` exits 128 outside a work tree. + + check=True turned that into a CalledProcessError traceback and a non-zero + exit, even though this check is advisory and main() returns 0 on both of + its own branches. + """ + monkeypatch.chdir(tmp_path) + + assert mod.main() == 0 + assert WARNING_TEXT in capsys.readouterr().err + + +@pytest.mark.agent_authored(model="claude-opus-5") +def test_git_missing_from_path_warns_and_succeeds(monkeypatch, capsys): + def no_git(*args, **kwargs): + raise FileNotFoundError(2, "No such file or directory: 'git'") + + monkeypatch.setattr(subprocess, "run", no_git) + + assert mod.main() == 0 + assert WARNING_TEXT in capsys.readouterr().err + + +@pytest.mark.agent_authored(model="claude-opus-5") +def test_unreadable_hook_path_warns_and_succeeds(tmp_path, monkeypatch, capsys): + """The open() guard only caught FileNotFoundError, so anything else that + makes the path unreadable escaped as a traceback.""" + hooks = tmp_path / "hooks" + (hooks / "pre-commit").mkdir(parents=True) # a directory, not a file + monkeypatch.setattr(mod, "_git_hooks_dir", lambda: str(hooks)) + + assert mod.main() == 0 + assert WARNING_TEXT in capsys.readouterr().err