mirror of
https://github.com/instructkr/claw-code.git
synced 2026-10-18 00:00:11 +08:00
test: enforce bash lru_cache clearance and expose specific skip reasons
Adds a conftest.py pytest fixture to explicitly clear the lru_cache for bash detection, addressing the monkeypatching hazard in a way that doesn't rely solely on comments. (Note: CI currently runs python -m unittest, but this keeps local pytest runners safe.) Also updates the bash check to run the execution probe directly inside the cached _check_bash_state, exposing the difference between 'no bash found' and 'bash present but execution failed' directly to the unittest skip reason.
This commit is contained in:
1 parent
c31c5e27dd
commit
86cf10a43a
4 files changed
+40
-23
No files matched your search
+24
-15
@@ -3,13 +3,20 @@ import os
|
||||
import shutil
|
||||
import subprocess
|
||||
|
||||
# Warning: get_bash_executable and require_bash use @functools.lru_cache.
|
||||
# Warning: _check_bash_state uses @functools.lru_cache.
|
||||
# Do not monkeypatch os.environ['PATH'] or shutil.which in tests and expect
|
||||
# these functions to re-evaluate. If you must patch them, call .cache_clear()
|
||||
# before and after the test.
|
||||
# before and after the test, or use the _clear_bash_cache pytest fixture.
|
||||
|
||||
def _probe_bash(bash_path: str) -> bool:
|
||||
try:
|
||||
res = subprocess.run([bash_path, '-c', 'echo 1'], capture_output=True, text=True, timeout=2)
|
||||
return res.returncode == 0
|
||||
except Exception:
|
||||
return False
|
||||
|
||||
@functools.lru_cache(maxsize=1)
|
||||
def get_bash_executable() -> str | None:
|
||||
def _check_bash_state() -> tuple[str | None, str]:
|
||||
if os.name == 'nt':
|
||||
for candidate in (
|
||||
r'C:\Program Files\Git\bin\bash.exe',
|
||||
@@ -18,19 +25,21 @@ def get_bash_executable() -> str | None:
|
||||
os.path.expandvars(r'%LOCALAPPDATA%\Programs\Git\bin\bash.exe'),
|
||||
):
|
||||
if os.path.exists(candidate):
|
||||
return candidate
|
||||
if _probe_bash(candidate):
|
||||
return candidate, ""
|
||||
return None, f"bash present at {candidate} but unusable"
|
||||
bash = shutil.which('bash')
|
||||
if bash and 'WindowsApps' not in bash:
|
||||
return bash
|
||||
return None
|
||||
if _probe_bash(bash):
|
||||
return bash, ""
|
||||
return None, f"bash present at {bash} but unusable"
|
||||
return None, "Requires bash"
|
||||
|
||||
def get_bash_executable() -> str | None:
|
||||
return _check_bash_state()[0]
|
||||
|
||||
@functools.lru_cache(maxsize=1)
|
||||
def require_bash() -> bool:
|
||||
bash = get_bash_executable()
|
||||
if not bash:
|
||||
return False
|
||||
try:
|
||||
res = subprocess.run([bash, '-c', 'echo 1'], capture_output=True, text=True, timeout=2)
|
||||
return res.returncode == 0
|
||||
except Exception:
|
||||
return False
|
||||
return get_bash_executable() is not None
|
||||
|
||||
def bash_skip_reason() -> str:
|
||||
return _check_bash_state()[1]
|
||||
@@ -0,0 +1,8 @@
|
||||
import pytest
|
||||
from tests._bash import _check_bash_state
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _clear_bash_cache():
|
||||
_check_bash_state.cache_clear()
|
||||
yield
|
||||
_check_bash_state.cache_clear()
|
||||
@@ -5,7 +5,7 @@ import subprocess
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
|
||||
from tests._bash import get_bash_executable, require_bash
|
||||
from tests._bash import get_bash_executable, require_bash, bash_skip_reason
|
||||
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parents[1]
|
||||
@@ -13,7 +13,7 @@ PRE_PUSH_HOOK = REPO_ROOT / '.github' / 'hooks' / 'pre-push'
|
||||
|
||||
|
||||
class PrePushHookContractTests(unittest.TestCase):
|
||||
@unittest.skipUnless(require_bash(), 'Requires bash')
|
||||
@unittest.skipUnless(require_bash(), bash_skip_reason())
|
||||
def test_skip_escape_hatch_exits_successfully_with_stderr_notice(self) -> None:
|
||||
bash_cmd = get_bash_executable() or 'bash'
|
||||
env = os.environ.copy()
|
||||
@@ -32,7 +32,7 @@ class PrePushHookContractTests(unittest.TestCase):
|
||||
self.assertIn('SKIP_CLAW_PRE_PUSH_BUILD=1', result.stderr)
|
||||
self.assertIn('skipping cargo workspace build', result.stderr)
|
||||
|
||||
@unittest.skipUnless(require_bash(), 'Requires bash')
|
||||
@unittest.skipUnless(require_bash(), bash_skip_reason())
|
||||
def test_default_build_gate_uses_workspace_locked_cargo_build(self) -> None:
|
||||
hook = PRE_PUSH_HOOK.read_text()
|
||||
|
||||
|
||||
@@ -14,7 +14,7 @@ REPO_ROOT = Path(__file__).resolve().parents[1]
|
||||
NEXT_ID = REPO_ROOT / 'scripts' / 'roadmap-next-id.sh'
|
||||
DOGFOOD_PROBE = REPO_ROOT / 'scripts' / 'dogfood-probe.py'
|
||||
|
||||
from tests._bash import get_bash_executable, require_bash
|
||||
from tests._bash import get_bash_executable, require_bash, bash_skip_reason
|
||||
|
||||
|
||||
def run_next_id(roadmap: Path, script: Path = NEXT_ID) -> subprocess.CompletedProcess[str]:
|
||||
@@ -40,7 +40,7 @@ def run_dogfood_probe(args: list[str]) -> subprocess.CompletedProcess[str]:
|
||||
|
||||
|
||||
class RoadmapHelperTests(unittest.TestCase):
|
||||
@unittest.skipUnless(require_bash(), 'Requires bash')
|
||||
@unittest.skipUnless(require_bash(), bash_skip_reason())
|
||||
def test_roadmap_next_id_prints_only_next_id_after_duplicate_check(self) -> None:
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
roadmap = Path(temp_dir) / 'ROADMAP.md'
|
||||
@@ -52,7 +52,7 @@ class RoadmapHelperTests(unittest.TestCase):
|
||||
self.assertEqual('725\n', result.stdout)
|
||||
self.assertEqual('', result.stderr)
|
||||
|
||||
@unittest.skipUnless(require_bash(), 'Requires bash')
|
||||
@unittest.skipUnless(require_bash(), bash_skip_reason())
|
||||
def test_roadmap_next_id_fails_fast_on_helper_era_duplicate(self) -> None:
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
roadmap = Path(temp_dir) / 'ROADMAP.md'
|
||||
@@ -66,7 +66,7 @@ class RoadmapHelperTests(unittest.TestCase):
|
||||
self.assertIn('999', result.stderr)
|
||||
self.assertNotIn('1000', result.stdout)
|
||||
|
||||
@unittest.skipUnless(require_bash(), 'Requires bash')
|
||||
@unittest.skipUnless(require_bash(), bash_skip_reason())
|
||||
def test_roadmap_next_id_fails_when_explicit_roadmap_path_is_missing(self) -> None:
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
roadmap = Path(temp_dir) / 'missing-ROADMAP.md'
|
||||
@@ -78,7 +78,7 @@ class RoadmapHelperTests(unittest.TestCase):
|
||||
self.assertIn('ROADMAP not found', result.stderr)
|
||||
self.assertIn(str(roadmap), result.stderr)
|
||||
|
||||
@unittest.skipUnless(require_bash(), 'Requires bash')
|
||||
@unittest.skipUnless(require_bash(), bash_skip_reason())
|
||||
def test_roadmap_next_id_fails_closed_when_checker_is_unavailable(self) -> None:
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
script_dir = Path(temp_dir) / 'scripts'
|
||||
|
||||
Reference in new issue
Block a user