From 5eaabe38bcd5a7dfdc9bf3bdef0558fcf8a49af5 Mon Sep 17 00:00:00 2001 From: "Zak B. Elep" Date: Sun, 19 Jul 2026 22:53:05 +0800 Subject: [PATCH] fix(test): accept path kwarg in shutil.which mocks for agent-browser cascade _find_agent_browser's extended-PATH branch now calls shutil.which(name, path=extended_path), which broke two post_setup_gating tests mocking shutil.which with name-only lambdas. Update those mocks and two similarly-shaped chromium test mocks that were latent landmines, and add coverage for cascade branches (local node_modules/.bin, validate=False paths, and _agent_browser_candidate_present) that had none. --- .../test_browser_chromium_autoinstall.py | 17 ++ tests/tools/test_browser_chromium_check.py | 2 +- tests/tools/test_browser_homebrew_paths.py | 164 ++++++++++++++++++ 3 files changed, 182 insertions(+), 1 deletion(-) diff --git a/tests/tools/test_browser_chromium_autoinstall.py b/tests/tools/test_browser_chromium_autoinstall.py index 2385eb9407..549e2ae9b2 100644 --- a/tests/tools/test_browser_chromium_autoinstall.py +++ b/tests/tools/test_browser_chromium_autoinstall.py @@ -57,6 +57,23 @@ class TestInstall: assert captured["cmd"] == ["/x/agent-browser", "install"] assert "--with-deps" not in captured["cmd"] + def test_npx_form_is_binary_only(self, monkeypatch): + monkeypatch.setattr(bt, "_running_in_docker", lambda: False) + monkeypatch.setattr("tools.lazy_deps._allow_lazy_installs", lambda: True) + monkeypatch.setattr(bt, "_find_agent_browser", lambda: "npx agent-browser") + monkeypatch.setattr(bt, "_build_browser_env", lambda: {}) + monkeypatch.setattr(bt, "_chromium_installed", lambda: True) + monkeypatch.setattr(bt.shutil, "which", lambda _, path=None: "/usr/bin/npx") + + captured = {} + monkeypatch.setattr( + bt.subprocess, "run", + lambda cmd, **kw: captured.update(cmd=cmd) or SimpleNamespace(returncode=0, stdout="", stderr=""), + ) + + assert bt._maybe_autoinstall_chromium() is True + assert captured["cmd"] == ["/usr/bin/npx", "-y", "agent-browser", "install"] + assert "--with-deps" not in captured["cmd"] def test_nonzero_exit_returns_false(self, monkeypatch): monkeypatch.setattr(bt, "_running_in_docker", lambda: False) diff --git a/tests/tools/test_browser_chromium_check.py b/tests/tools/test_browser_chromium_check.py index f9c11051ce..1aad65e775 100644 --- a/tests/tools/test_browser_chromium_check.py +++ b/tests/tools/test_browser_chromium_check.py @@ -40,7 +40,7 @@ class TestChromiumInstalled: monkeypatch.setattr( bt.shutil, "which", - lambda name: "/usr/bin/chromium" if name == "chromium" else None, + lambda name, path=None: "/usr/bin/chromium" if name == "chromium" else None, ) assert bt._chromium_installed() is True diff --git a/tests/tools/test_browser_homebrew_paths.py b/tests/tools/test_browser_homebrew_paths.py index a5a861ece6..02751433de 100644 --- a/tests/tools/test_browser_homebrew_paths.py +++ b/tests/tools/test_browser_homebrew_paths.py @@ -2,12 +2,14 @@ import json import os +import sys from pathlib import Path from unittest.mock import patch, MagicMock, mock_open import pytest from tools.browser_tool import ( + _agent_browser_candidate_present, _discover_homebrew_node_dirs, _find_agent_browser, _run_browser_command, @@ -95,6 +97,168 @@ class TestFindAgentBrowser: with pytest.raises(FileNotFoundError, match="agent-browser CLI not found"): _find_agent_browser() + def test_finds_in_local_node_modules_bin(self): + """Should fall through to the repo's node_modules/.bin when both the + bare PATH and the extended (Homebrew/fallback) PATH miss.""" + repo_root = Path(_bt.__file__).parent.parent + local_bin_dir = repo_root / "node_modules" / ".bin" + local_bin_path = str(local_bin_dir / "agent-browser") + + def mock_which(cmd, path=None): + if cmd == "agent-browser" and path and str(local_bin_dir) in path: + return local_bin_path + return None + + original_is_dir = Path.is_dir + + def mock_is_dir(self): + if self == local_bin_dir: + return True + return original_is_dir(self) + + with patch("shutil.which", side_effect=mock_which), \ + patch("os.path.isdir", return_value=False), \ + patch.object(Path, "is_dir", mock_is_dir), \ + patch("tools.browser_tool.agent_browser_runnable", return_value=True), \ + patch( + "tools.browser_tool._discover_homebrew_node_dirs", + return_value=[], + ): + result = _find_agent_browser() + + assert result == local_bin_path + + def test_extended_path_hit_validate_false_skips_runnable_check(self, tmp_path): + """Readiness probes (validate=False, used by _has_agent_browser) must + resolve a candidate found via the extended PATH's path= kwarg lookup + without calling agent_browser_runnable — that keeps the probe a cheap + existence check with no subprocess spawn.""" + fake_binary = tmp_path / "agent-browser" + fake_binary.write_text("#!/bin/sh\n") + fake_binary.chmod(0o755) + + def mock_which(cmd, path=None): + if cmd == "agent-browser" and path: + return str(fake_binary) + return None # bare (path=None) PATH lookup misses + + with patch("shutil.which", side_effect=mock_which), \ + patch("os.path.isdir", return_value=True), \ + patch( + "tools.browser_tool.agent_browser_runnable", + side_effect=AssertionError( + "validate=False must not call agent_browser_runnable" + ), + ), \ + patch( + "tools.browser_tool._discover_homebrew_node_dirs", + return_value=["/opt/homebrew/bin"], + ): + result = _find_agent_browser(validate=False) + + assert result == str(fake_binary) + + def test_local_bin_hit_validate_false_skips_runnable_check(self, tmp_path): + """Same no-subprocess-spawn contract for the node_modules/.bin + candidate: validate=False relies on _agent_browser_candidate_present's + existence+exec-bit check instead of shelling out to --version.""" + repo_root = Path(_bt.__file__).parent.parent + local_bin_dir = repo_root / "node_modules" / ".bin" + + fake_binary = tmp_path / "agent-browser" + fake_binary.write_text("#!/bin/sh\n") + fake_binary.chmod(0o755) + + def mock_which(cmd, path=None): + if cmd == "agent-browser" and path and str(local_bin_dir) in path: + return str(fake_binary) + return None + + original_is_dir = Path.is_dir + + def mock_is_dir(self): + if self == local_bin_dir: + return True + return original_is_dir(self) + + with patch("shutil.which", side_effect=mock_which), \ + patch("os.path.isdir", return_value=False), \ + patch.object(Path, "is_dir", mock_is_dir), \ + patch( + "tools.browser_tool.agent_browser_runnable", + side_effect=AssertionError( + "validate=False must not call agent_browser_runnable" + ), + ), \ + patch( + "tools.browser_tool._discover_homebrew_node_dirs", + return_value=[], + ): + result = _find_agent_browser(validate=False) + + assert result == str(fake_binary) + + def test_npx_fallback_validate_false(self): + """The npx sentinel must resolve through the validate=False path too, + independent of the fully-mocked coverage in test_nous_subscription.py.""" + def mock_which(cmd, path=None): + if cmd == "agent-browser": + return None + if cmd == "npx": + return "/usr/bin/npx" + return None + + original_path_exists = Path.exists + + def mock_path_exists(self): + if "node_modules" in str(self) and "agent-browser" in str(self): + return False + return original_path_exists(self) + + with patch("shutil.which", side_effect=mock_which), \ + patch("os.path.isdir", return_value=False), \ + patch.object(Path, "exists", mock_path_exists), \ + patch( + "tools.browser_tool._discover_homebrew_node_dirs", + return_value=[], + ): + result = _find_agent_browser(validate=False) + + assert result == "npx agent-browser" + + +class TestAgentBrowserCandidatePresent: + """Direct unit tests for the validate=False candidate check used by every + branch of _find_agent_browser's readiness-probe (no-subprocess) mode.""" + + def test_none_is_false(self): + assert _agent_browser_candidate_present(None) is False + + def test_empty_string_is_false(self): + assert _agent_browser_candidate_present("") is False + + def test_npx_sentinel_is_true_without_touching_filesystem(self): + assert _agent_browser_candidate_present("npx agent-browser") is True + + def test_executable_file_is_true(self, tmp_path): + binary = tmp_path / "agent-browser" + binary.write_text("#!/bin/sh\n") + binary.chmod(0o755) + assert _agent_browser_candidate_present(str(binary)) is True + + @pytest.mark.skipif( + sys.platform == "win32", + reason="exec-bit is not meaningful on Windows; os.name == 'nt' short-circuits", + ) + def test_nonexecutable_file_is_false(self, tmp_path): + binary = tmp_path / "agent-browser" + binary.write_text("#!/bin/sh\n") + binary.chmod(0o644) + assert _agent_browser_candidate_present(str(binary)) is False + + def test_nonexistent_path_is_false(self, tmp_path): + assert _agent_browser_candidate_present(str(tmp_path / "missing")) is False + class TestBrowserRequirements: def test_cdp_override_does_not_require_agent_browser_cli(self, monkeypatch):