diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 68c4aeb255..0117c0e202 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -7,7 +7,7 @@ - [ ] Tested locally with `uv run specify --help` -- [ ] Ran existing tests with `uv sync && uv run pytest` +- [ ] Ran existing tests with `uv sync && uv run pytest` (optionally `uv run pytest --parallel --parallel-tier medium`) - [ ] Tested with a sample project (if applicable) ## AI Disclosure @@ -17,6 +17,7 @@ - [ ] I **did not** use AI assistance for this contribution - [ ] I **did** use AI assistance (describe below) +- [ ] If AI posted PR comments on my behalf, each comment includes explicit "Posted on behalf of @ by (model: )" attribution diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 12b095f5fc..239b1609c3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -38,6 +38,7 @@ On [GitHub Codespaces](https://github.com/features/codespaces) it's even simpler 1. Fork and clone the repository 1. Configure and install the dependencies: `uv sync --extra test` 1. Make sure the CLI works on your machine: `uv run specify --help` +1. Run tests: `uv run pytest` (optional faster path: `uv run pytest --parallel`) 1. Create a new branch: `git checkout -b /-` (see [Branch naming](#branch-naming) below) 1. Make your change, add tests, and make sure everything still works 1. Test the CLI functionality with a sample project if relevant @@ -87,6 +88,32 @@ For the smoothest review experience, validate changes in this order: ### Automated checks +#### Optional parallel test execution + +```bash +uv run pytest --parallel +``` + +`--parallel` is opt-in and auto-selects a conservative worker count using CPU, memory, and OS caps. Use `--parallel-max-workers N` to set a stricter upper bound. + +Worker settings are calculated from effective CPU capacity (including affinity/container quotas where available) and currently available memory, then bounded by platform caps. + +Use `--parallel-tier low|medium|high` to tune aggressiveness: + +- `low` keeps more headroom (best for laptops or multitasking) +- `medium` is the default balance +- `high` favors throughput on dedicated dev/CI machines + +Recommended starting points: + +| Environment | Suggested tier | Example command | +|---|---|---| +| Laptop / shared desktop | low | `uv run pytest --parallel --parallel-tier low` | +| Developer workstation | medium | `uv run pytest --parallel --parallel-tier medium` | +| Dedicated CI runner | high | `uv run pytest --parallel --parallel-tier high` | + +If system load is high or tests become unstable, step down one tier and/or set `--parallel-max-workers`. + #### Agent configuration and wiring consistency ```bash @@ -190,6 +217,12 @@ That being said, if you are using any kind of AI assistance (e.g., agents, ChatG If your PR responses or comments are being generated by an AI, disclose that as well. +When AI-generated PR comments are posted on your behalf, use an explicit attribution line in the comment body, for example: + +> Posted on behalf of @ by GitHub Copilot (model: GPT-5.3-Codex). + +Keep one top-level review-round summary comment per round (instead of replying to every thread), and do not resolve reviewer conversations yourself. + As an exception, trivial spacing or typo fixes don't need to be disclosed, so long as the changes are limited to small parts of the code or short phrases. An example disclosure: diff --git a/pyproject.toml b/pyproject.toml index 2b7cdc5bc1..aa293cf7b8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -51,6 +51,7 @@ packages = ["src/specify_cli"] test = [ "pytest>=7.0", "pytest-cov>=4.0", + "pytest-xdist>=3.6.1", ] [tool.pytest.ini_options] diff --git a/scripts/bash/common.sh b/scripts/bash/common.sh index 9d7dd21edf..980fab0995 100644 --- a/scripts/bash/common.sh +++ b/scripts/bash/common.sh @@ -433,12 +433,16 @@ resolve_template() { local presets_dir="$repo_root/.specify/presets" if [ -d "$presets_dir" ]; then local registry_file="$presets_dir/.registry" - if [ -f "$registry_file" ] && command -v python3 >/dev/null 2>&1; then + if [ -f "$registry_file" ] && (command -v python3 >/dev/null 2>&1 || command -v python >/dev/null 2>&1); then # Read preset IDs sorted by priority (lower number = higher precedence). - # The python3 call is wrapped in an if-condition so that set -e does not - # abort the function when python3 exits non-zero (e.g. invalid JSON). + # The python call is wrapped in an if-condition so that set -e does not + # abort the function when the interpreter exits non-zero (e.g. invalid JSON). local sorted_presets="" - if sorted_presets=$(SPECKIT_REGISTRY="$registry_file" python3 -c " + local python_cmd="python3" + if ! command -v "$python_cmd" >/dev/null 2>&1; then + python_cmd="python" + fi + if sorted_presets=$(SPECKIT_REGISTRY="$registry_file" "$python_cmd" -c " import json, sys, os try: with open(os.environ['SPECKIT_REGISTRY']) as f: @@ -451,28 +455,29 @@ except Exception: sys.exit(1) " 2>/dev/null); then if [ -n "$sorted_presets" ]; then - # python3 succeeded and returned preset IDs — search in priority order + # Python interpreter succeeded and returned preset IDs — search in priority order while IFS= read -r preset_id; do + preset_id="${preset_id%$'\r'}" local candidate="$presets_dir/$preset_id/templates/${template_name}.md" [ -f "$candidate" ] && echo "$candidate" && return 0 done <<< "$sorted_presets" fi - # python3 succeeded but registry has no presets — nothing to search + # Python interpreter succeeded but registry has no presets — nothing to search else - # python3 failed (missing, or registry parse error) — fall back to unordered directory scan - for preset in "$presets_dir"/*/; do + # Interpreter invocation failed (missing, or registry parse error) — fall back to deterministic directory scan + while IFS= read -r preset; do [ -d "$preset" ] || continue local candidate="$preset/templates/${template_name}.md" [ -f "$candidate" ] && echo "$candidate" && return 0 - done + done < <(find "$presets_dir" -mindepth 1 -maxdepth 1 -type d 2>/dev/null | LC_ALL=C sort) fi else - # Fallback: alphabetical directory order (no python3 available) - for preset in "$presets_dir"/*/; do + # Fallback: alphabetical directory order (no usable python interpreter available) + while IFS= read -r preset; do [ -d "$preset" ] || continue local candidate="$preset/templates/${template_name}.md" [ -f "$candidate" ] && echo "$candidate" && return 0 - done + done < <(find "$presets_dir" -mindepth 1 -maxdepth 1 -type d 2>/dev/null | LC_ALL=C sort) fi fi diff --git a/tests/_parallel.py b/tests/_parallel.py new file mode 100644 index 0000000000..1ff5429fec --- /dev/null +++ b/tests/_parallel.py @@ -0,0 +1,293 @@ +"""Parallel-test worker sizing helpers for pytest.""" + +from __future__ import annotations + +import ctypes +import os +import sys +from dataclasses import dataclass +from typing import Literal + + +ParallelTier = Literal["low", "medium", "high"] + + +def _read_text(path: str) -> str | None: + try: + with open(path, "r", encoding="utf-8") as f: + return f.read().strip() + except OSError: + return None + + +def _read_meminfo_available_bytes() -> int | None: + raw = _read_text("/proc/meminfo") + if not raw: + return None + for line in raw.splitlines(): + if line.startswith("MemAvailable:"): + parts = line.split() + if len(parts) >= 2: + try: + return int(parts[1]) * 1024 + except ValueError: + return None + return None + + +def _detect_cgroup_available_memory_bytes() -> int | None: + # cgroup v2 + limit_raw = _read_text("/sys/fs/cgroup/memory.max") + used_raw = _read_text("/sys/fs/cgroup/memory.current") + + if limit_raw and used_raw and limit_raw != "max": + try: + limit = int(limit_raw) + used = int(used_raw) + if limit > 0: + return max(0, limit - used) + except ValueError: + pass + + # cgroup v1 + limit_raw = _read_text("/sys/fs/cgroup/memory/memory.limit_in_bytes") + used_raw = _read_text("/sys/fs/cgroup/memory/memory.usage_in_bytes") + if limit_raw and used_raw: + try: + limit = int(limit_raw) + used = int(used_raw) + if limit > 0 and limit < (1 << 60): # ignore effectively-unlimited sentinel values + return max(0, limit - used) + except ValueError: + pass + + return None + + +def _detect_cgroup_cpu_quota_count() -> int | None: + # cgroup v2 + quota_raw = _read_text("/sys/fs/cgroup/cpu.max") + if quota_raw: + parts = quota_raw.split() + if len(parts) == 2 and parts[0] != "max": + try: + quota = int(parts[0]) + period = int(parts[1]) + if quota > 0 and period > 0: + return max(1, quota // period) + except ValueError: + pass + + # cgroup v1 + # Some distros/runtimes mount under /sys/fs/cgroup/cpu/, while others use + # /sys/fs/cgroup/cpu,cpuacct/. + quota_candidates = ( + "/sys/fs/cgroup/cpu/cpu.cfs_quota_us", + "/sys/fs/cgroup/cpu,cpuacct/cpu.cfs_quota_us", + "/sys/fs/cgroup/cpuacct,cpu/cpu.cfs_quota_us", + ) + period_candidates = ( + "/sys/fs/cgroup/cpu/cpu.cfs_period_us", + "/sys/fs/cgroup/cpu,cpuacct/cpu.cfs_period_us", + "/sys/fs/cgroup/cpuacct,cpu/cpu.cfs_period_us", + ) + + for quota_path, period_path in zip(quota_candidates, period_candidates): + quota_raw = _read_text(quota_path) + period_raw = _read_text(period_path) + if not quota_raw or not period_raw: + continue + try: + quota = int(quota_raw) + period = int(period_raw) + # cgroup v1 uses -1 for unlimited quota. + if quota > 0 and period > 0: + return max(1, quota // period) + except ValueError: + continue + + return None + + +def detect_effective_cpu_count() -> int: + """Best-effort effective CPU count considering affinity and container quotas.""" + cpus = max(1, int(os.cpu_count() or 1)) + + if hasattr(os, "sched_getaffinity"): + try: + cpus = min(cpus, max(1, len(os.sched_getaffinity(0)))) + except OSError: + pass + + cgroup_cpus = _detect_cgroup_cpu_quota_count() + if cgroup_cpus is not None: + cpus = min(cpus, cgroup_cpus) + + return max(1, cpus) + + +def detect_total_memory_bytes() -> int | None: + """Best-effort total system memory in bytes, or None if unavailable.""" + if sys.platform == "win32": + class MEMORYSTATUSEX(ctypes.Structure): + _fields_ = [ + ("dwLength", ctypes.c_ulong), + ("dwMemoryLoad", ctypes.c_ulong), + ("ullTotalPhys", ctypes.c_ulonglong), + ("ullAvailPhys", ctypes.c_ulonglong), + ("ullTotalPageFile", ctypes.c_ulonglong), + ("ullAvailPageFile", ctypes.c_ulonglong), + ("ullTotalVirtual", ctypes.c_ulonglong), + ("ullAvailVirtual", ctypes.c_ulonglong), + ("ullAvailExtendedVirtual", ctypes.c_ulonglong), + ] + + stats = MEMORYSTATUSEX() + stats.dwLength = ctypes.sizeof(MEMORYSTATUSEX) + if ctypes.windll.kernel32.GlobalMemoryStatusEx(ctypes.byref(stats)) == 0: + return None + return int(stats.ullTotalPhys) + + if hasattr(os, "sysconf"): + page_size = None + for key in ("SC_PAGE_SIZE", "SC_PAGESIZE"): + try: + value = int(os.sysconf(key)) + except (ValueError, OSError): + continue + if value > 0: + page_size = value + break + if page_size is not None: + try: + pages = int(os.sysconf("SC_PHYS_PAGES")) + if pages > 0: + return page_size * pages + except (ValueError, OSError): + return None + + return None + + +def detect_available_memory_bytes() -> int | None: + """Best-effort currently available memory in bytes, or None if unavailable.""" + if sys.platform == "win32": + class MEMORYSTATUSEX(ctypes.Structure): + _fields_ = [ + ("dwLength", ctypes.c_ulong), + ("dwMemoryLoad", ctypes.c_ulong), + ("ullTotalPhys", ctypes.c_ulonglong), + ("ullAvailPhys", ctypes.c_ulonglong), + ("ullTotalPageFile", ctypes.c_ulonglong), + ("ullAvailPageFile", ctypes.c_ulonglong), + ("ullTotalVirtual", ctypes.c_ulonglong), + ("ullAvailVirtual", ctypes.c_ulonglong), + ("ullAvailExtendedVirtual", ctypes.c_ulonglong), + ] + + stats = MEMORYSTATUSEX() + stats.dwLength = ctypes.sizeof(MEMORYSTATUSEX) + if ctypes.windll.kernel32.GlobalMemoryStatusEx(ctypes.byref(stats)) == 0: + return None + return int(stats.ullAvailPhys) + + mem_available = _read_meminfo_available_bytes() + cgroup_available = _detect_cgroup_available_memory_bytes() + + if mem_available is not None and cgroup_available is not None: + return min(mem_available, cgroup_available) + if mem_available is not None: + return mem_available + if cgroup_available is not None: + return cgroup_available + + return None + + +@dataclass(frozen=True) +class ParallelSettings: + tier: ParallelTier + workers: int + cpu_cap: int + memory_cap: int + os_cap: int + effective_cpus: int + total_memory_bytes: int | None + available_memory_bytes: int | None + memory_per_worker_gib: float + + +@dataclass(frozen=True) +class ParallelTierConfig: + cpu_reserve: int + memory_per_worker_gib: float + os_cap_by_platform: dict[str, int] + + +TIER_CONFIGS: dict[ParallelTier, ParallelTierConfig] = { + "low": ParallelTierConfig( + cpu_reserve=2, + memory_per_worker_gib=2.5, + os_cap_by_platform={"win32": 2, "darwin": 4, "linux": 6}, + ), + "medium": ParallelTierConfig( + cpu_reserve=1, + memory_per_worker_gib=1.5, + os_cap_by_platform={"win32": 4, "darwin": 6, "linux": 8}, + ), + "high": ParallelTierConfig( + cpu_reserve=0, + memory_per_worker_gib=1.0, + os_cap_by_platform={"win32": 6, "darwin": 10, "linux": 16}, + ), +} + + +def compute_recommended_workers( + *, + cpu_count: int, + total_memory_bytes: int | None, + available_memory_bytes: int | None, + platform_name: str, + max_workers: int | None, + tier: ParallelTier = "medium", +) -> ParallelSettings: + """Compute parallel worker settings from detected system constraints.""" + cfg = TIER_CONFIGS[tier] + cpus = max(1, int(cpu_count)) + cpu_cap = max(1, cpus - cfg.cpu_reserve) + + # Bound workers by currently available memory to avoid swap thrash. + memory_cap = cpu_cap + if available_memory_bytes is not None: + memory_basis = available_memory_bytes + else: + memory_basis = total_memory_bytes + if memory_basis is not None and memory_basis > 0: + gib = memory_basis / (1024 ** 3) + memory_cap = max(1, int(gib // cfg.memory_per_worker_gib)) + elif memory_basis is not None: + memory_cap = 1 + + os_cap = cfg.os_cap_by_platform.get(platform_name) + if os_cap is None: + # Unknown platforms should default to the most permissive known cap, + # not the strictest Windows cap. + os_cap = max(cfg.os_cap_by_platform.values()) + + workers = min(cpu_cap, memory_cap, os_cap) + + if max_workers is not None: + workers = min(workers, max(1, int(max_workers))) + + return ParallelSettings( + tier=tier, + workers=max(1, workers), + cpu_cap=cpu_cap, + memory_cap=max(1, memory_cap), + os_cap=os_cap, + effective_cpus=cpus, + total_memory_bytes=total_memory_bytes, + available_memory_bytes=available_memory_bytes, + memory_per_worker_gib=cfg.memory_per_worker_gib, + ) diff --git a/tests/_path_utils.py b/tests/_path_utils.py new file mode 100644 index 0000000000..2f48f2a69c --- /dev/null +++ b/tests/_path_utils.py @@ -0,0 +1,79 @@ +"""Shared shell-path normalization helpers for cross-platform tests.""" + +from __future__ import annotations + +import os +import re +import tempfile +from pathlib import Path + + +def normalize_path_text(path_value: str) -> str: + """Normalize slashes and repeated separators for string checks.""" + normalized = path_value.replace("\\", "/") + if path_value.startswith("\\\\") or (normalized.startswith("//") and not normalized.startswith("///")): + unc_tail = normalized.lstrip("/") + return "//" + re.sub(r"/{2,}", "/", unc_tail) + return re.sub(r"/{2,}", "/", normalized) + + +def normalized_parts(path_value: str) -> list[str]: + """Return normalized path components, ignoring Windows drive prefixes.""" + normalized = normalize_path_text(path_value.strip().strip("'\"")) + normalized = re.sub(r"^[A-Za-z]:", "", normalized) + return [p for p in normalized.split("/") if p] + + +def assert_normalized_path_equal(actual: str, expected: Path | str) -> None: + """Assert paths are equivalent after cross-platform shell normalization.""" + actual_raw = actual.strip().strip("'\"") + expected_raw = str(expected).strip().strip("'\"") + if os.name == "nt": + actual_raw = str(path_from_bash_output(actual_raw)) + expected_raw = str(path_from_bash_output(expected_raw)) + if actual_raw == expected_raw: + return + if normalized_parts(actual_raw) == normalized_parts(expected_raw): + return + raise AssertionError(f"Path mismatch. actual={actual_raw!r} expected={expected_raw!r}") + + +def assert_shell_path_matches(actual: str, expected: Path) -> None: + """Assert shell-emitted path matches expected path with Windows-only relaxations.""" + actual_raw = actual.strip().strip("'\"") + expected_raw = str(expected) + if os.name == "nt": + nested_drive = re.search(r"[\\/][A-Za-z]:[\\/]", actual_raw) + if nested_drive: + actual_raw = actual_raw[nested_drive.start() + 1:] + actual_raw = str(path_from_bash_output(actual_raw)) + expected_raw = str(path_from_bash_output(expected_raw)) + + if actual_raw == expected_raw: + return + + actual_parts = normalized_parts(actual_raw) + expected_parts = normalized_parts(expected_raw) + + def trim_to_pytest(parts: list[str]) -> list[str]: + for idx, part in enumerate(parts): + if part.startswith("pytest-"): + return parts[idx:] + return parts + + if os.name == "nt" and trim_to_pytest(actual_parts) == trim_to_pytest(expected_parts): + return + + raise AssertionError(f"Path mismatch. actual={actual_raw!r} expected={expected_raw!r}") + + +def path_from_bash_output(path_value: str) -> Path: + """Normalize bash-emitted paths for assertions on Windows/Git Bash.""" + path_value = path_value.strip().strip("'\"") + if os.name == "nt": + if path_value.startswith("/tmp/"): + return Path(tempfile.gettempdir()) / path_value[len("/tmp/"):] + match = re.match(r"^/([a-zA-Z])/(.*)$", path_value) + if match: + return Path(f"{match.group(1).upper()}:/{match.group(2)}") + return Path(path_value) diff --git a/tests/conftest.py b/tests/conftest.py index 4ef643e121..2650c2a00d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,12 +5,171 @@ import shutil import subprocess import sys +import importlib.util import pytest +from tests._parallel import ( + compute_recommended_workers, + detect_available_memory_bytes, + detect_effective_cpu_count, + detect_total_memory_bytes, +) + _ANSI_ESCAPE_RE = re.compile(r"\x1b\[[0-?]*[ -/]*[@-~]") +def _args_before_double_dash(args: list[str]) -> list[str]: + """Return only option-parsed args before '--' positional sentinel.""" + if "--" in args: + return args[:args.index("--")] + return args + + +def _has_xdist_installed() -> bool: + """Return whether pytest-xdist is importable in this environment.""" + return importlib.util.find_spec("xdist") is not None + + +def _is_plugin_autoload_disabled() -> bool: + """Return True when pytest plugin autoload is explicitly disabled.""" + value = os.environ.get("PYTEST_DISABLE_PLUGIN_AUTOLOAD", "") + return value.strip().lower() in {"1", "true", "yes", "on"} + + +def _has_numprocesses_arg(args: list[str]) -> bool: + """Return True when users explicitly pass -n/--numprocesses.""" + args = _args_before_double_dash(args) + idx = 0 + while idx < len(args): + arg = args[idx] + if arg in ("-n", "--numprocesses"): + return True + if arg.startswith("--numprocesses="): + return True + # Support compact forms like -n2 or -nauto + if arg.startswith("-n") and arg != "-n": + return True + idx += 1 + return False + + +def _has_dist_arg(args: list[str]) -> bool: + """Return True when users explicitly pass --dist.""" + args = _args_before_double_dash(args) + return any(arg == "--dist" or arg.startswith("--dist=") for arg in args) + + +def _is_xdist_disabled(args: list[str]) -> bool: + """Return True when users explicitly disable xdist with -p no:xdist.""" + args = _args_before_double_dash(args) + idx = 0 + while idx < len(args): + arg = args[idx] + if arg == "-p": + if idx + 1 < len(args) and args[idx + 1].startswith("no:xdist"): + return True + idx += 2 + continue + if arg.startswith("-pno:xdist"): + return True + idx += 1 + return False + + +def _is_xdist_explicitly_enabled(args: list[str]) -> bool: + """Return True when users explicitly enable xdist via -p xdist.""" + + def _is_xdist_plugin_name(name: str) -> bool: + return name == "xdist" or name.startswith("xdist.") + + args = _args_before_double_dash(args) + idx = 0 + while idx < len(args): + arg = args[idx] + if arg == "-p": + if idx + 1 < len(args): + plugin_name = args[idx + 1] + if not plugin_name.startswith("no:") and _is_xdist_plugin_name(plugin_name): + return True + idx += 2 + continue + if arg.startswith("-p") and arg != "-p": + plugin_name = arg[2:] + if plugin_name and not plugin_name.startswith("no:") and _is_xdist_plugin_name(plugin_name): + return True + idx += 1 + return False + + +def _extract_cli_option(args: list[str], option: str, default: str | None = None) -> str | None: + """Extract option value from --opt value or --opt=value forms.""" + args = _args_before_double_dash(args) + prefix = f"{option}=" + idx = 0 + while idx < len(args): + arg = args[idx] + if arg == option: + if idx + 1 < len(args): + return args[idx + 1] + return default + if arg.startswith(prefix): + return arg[len(prefix):] + idx += 1 + return default + + +def _compute_parallel_settings_from_args(args: list[str]): + """Compute parallel worker settings from CLI args using shared detection.""" + tier = _extract_cli_option(args, "--parallel-tier", "medium") + max_workers_raw = _extract_cli_option(args, "--parallel-max-workers", None) + max_workers = None + if max_workers_raw not in (None, ""): + try: + max_workers = int(max_workers_raw) + except ValueError: + max_workers = None + + return compute_recommended_workers( + cpu_count=detect_effective_cpu_count(), + total_memory_bytes=detect_total_memory_bytes(), + available_memory_bytes=detect_available_memory_bytes(), + platform_name=sys.platform, + max_workers=max_workers, + tier=tier if tier in ("low", "medium", "high") else "medium", + ) + + +def _build_parallel_injected_args(args: list[str], workers: int) -> list[str]: + """Build xdist args to inject for parallel execution.""" + injected_args = ["-n", str(workers)] + if not _has_dist_arg(args): + injected_args.extend(["--dist", "worksteal"]) + return injected_args + + +def pytest_load_initial_conftests(early_config, parser, args): + """Inject xdist flags early so --parallel actually runs with workers.""" + if "--parallel" not in _args_before_double_dash(args): + return + if not _has_xdist_installed(): + return + if _is_plugin_autoload_disabled() and not _is_xdist_explicitly_enabled(args): + return + if _is_xdist_disabled(args): + return + if _has_numprocesses_arg(args): + return + + settings = _compute_parallel_settings_from_args(args) + injected_args = _build_parallel_injected_args(args, settings.workers) + if "--" in args: + idx = args.index("--") + args[idx:idx] = injected_args + else: + args.extend(injected_args) + + def _has_working_bash() -> bool: """Check whether a functional native bash is available. @@ -68,6 +227,103 @@ def strip_ansi(text: str) -> str: return _ANSI_ESCAPE_RE.sub("", text) +def pytest_addoption(parser): + """Add Spec Kit parallel-test controls on top of pytest-xdist.""" + group = parser.getgroup("spec-kit") + group.addoption( + "--parallel", + action="store_true", + default=False, + help="Run tests in parallel using a system-aware worker limit.", + ) + group.addoption( + "--parallel-max-workers", + action="store", + type=int, + default=None, + help="Upper bound for --parallel worker count.", + ) + group.addoption( + "--parallel-tier", + action="store", + choices=("low", "medium", "high"), + default="medium", + help="Parallel aggressiveness tier: low, medium, or high (default: medium).", + ) + + +def pytest_configure(config): + """Enable bounded xdist parallelism only when --parallel is requested.""" + if not config.getoption("--parallel"): + return + + invocation_args = list(config.invocation_params.args) + if _is_xdist_disabled(invocation_args): + raise pytest.UsageError("--parallel cannot be used with '-p no:xdist'.") + + max_workers = config.getoption("--parallel-max-workers") + tier = config.getoption("--parallel-tier") + if max_workers is not None and max_workers < 1: + raise pytest.UsageError("--parallel-max-workers must be >= 1") + + if not hasattr(config.option, "numprocesses"): + if _is_plugin_autoload_disabled(): + raise pytest.UsageError( + "--parallel requires pytest-xdist plugin loading. Unset PYTEST_DISABLE_PLUGIN_AUTOLOAD or enable xdist explicitly." + ) + raise pytest.UsageError( + "--parallel requires pytest-xdist. Install test extras with `uv sync --extra test`." + ) + + settings = compute_recommended_workers( + cpu_count=detect_effective_cpu_count(), + total_memory_bytes=detect_total_memory_bytes(), + available_memory_bytes=detect_available_memory_bytes(), + platform_name=sys.platform, + max_workers=max_workers, + tier=tier, + ) + + # Respect explicit -n values from CLI; otherwise keep the early-injected value. + requested_numprocesses = getattr(config.option, "numprocesses", None) + if requested_numprocesses in (None, 0): + config.option.numprocesses = settings.workers + if hasattr(config.option, "dist") and not _has_dist_arg(invocation_args): + config.option.dist = "worksteal" + + setattr(config, "_spec_kit_parallel_settings", settings) + setattr(config, "_spec_kit_parallel_effective_workers", getattr(config.option, "numprocesses", settings.workers)) + + +def pytest_report_header(config): + """Display resolved system-aware parallel settings in pytest header.""" + settings = getattr(config, "_spec_kit_parallel_settings", None) + if settings is None: + return None + + effective_workers = getattr(config, "_spec_kit_parallel_effective_workers", settings.workers) + + total_gib = ( + f"{settings.total_memory_bytes / (1024 ** 3):.1f}GiB" + if settings.total_memory_bytes is not None + else "unknown" + ) + avail_gib = ( + f"{settings.available_memory_bytes / (1024 ** 3):.1f}GiB" + if settings.available_memory_bytes is not None + else "unknown" + ) + return ( + "[spec-kit] --parallel settings: " + f"tier={settings.tier}, " + f"workers={effective_workers} " + f"(cpu_cap={settings.cpu_cap}, mem_cap={settings.memory_cap}, os_cap={settings.os_cap}), " + f"effective_cpus={settings.effective_cpus}, " + f"avail_mem={avail_gib}, total_mem={total_gib}, " + f"mem_per_worker={settings.memory_per_worker_gib:.1f}GiB" + ) + + # --------------------------------------------------------------------------- # Auth config isolation — prevents tests from reading ~/.specify/auth.json # --------------------------------------------------------------------------- diff --git a/tests/integrations/test_cli.py b/tests/integrations/test_cli.py index 460db4897e..f2e66e542e 100644 --- a/tests/integrations/test_cli.py +++ b/tests/integrations/test_cli.py @@ -3,6 +3,8 @@ import io import json import os +from pathlib import Path +from unittest.mock import patch import pytest import yaml @@ -569,7 +571,6 @@ def test_shared_infra_skip_warning_uses_posix_paths(self, tmp_path): assert ".specify/scripts/bash/nested/deep.sh" in output assert ".specify/templates/plan-template.md" in output - @pytest.mark.skipif(os.name == "nt", reason="POSIX mode bits are not stable on Windows") def test_shared_template_writes_are_not_world_writable(self, tmp_path): """Shared template writes use a safe default mode instead of chmod 666.""" from specify_cli.shared_infra import install_shared_infra @@ -582,18 +583,37 @@ def test_shared_template_writes_are_not_world_writable(self, tmp_path): templates_src.mkdir(parents=True) (templates_src / "plan-template.md").write_text("# plan\n", encoding="utf-8") - install_shared_infra( - project, - "sh", - version="test", - core_pack=core_pack, - repo_root=tmp_path / "unused", - console=_NoopConsole(), - force=True, - ) + with patch("specify_cli.shared_infra.Path.chmod", autospec=True, wraps=Path.chmod) as chmod_spy: + install_shared_infra( + project, + "sh", + version="test", + core_pack=core_pack, + repo_root=tmp_path / "unused", + console=_NoopConsole(), + force=True, + ) written = project / ".specify" / "templates" / "plan-template.md" - assert written.stat().st_mode & 0o777 == 0o644 + if os.name == "nt": + assert written.is_file() + + def chmod_call_matches(call) -> bool: + target = call.args[0] if call.args else call.kwargs.get("self") + mode = call.args[1] if len(call.args) > 1 else call.kwargs.get("mode") + if target is None or mode != 0o644: + return False + target_path = Path(target) + if target_path.parent != written.parent: + return False + return target_path.name == written.name or target_path.name.startswith(f".{written.name}.") + + assert any( + chmod_call_matches(call) + for call in chmod_spy.call_args_list + ) + else: + assert written.stat().st_mode & 0o777 == 0o644 def test_shared_infra_no_warning_when_forced(self, tmp_path, capsys): """No skip warning when force=True (all files overwritten).""" diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index fd9eada5cc..37d891b1af 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -2,6 +2,9 @@ import json import os +import sys + +import pytest from typer.testing import CliRunner @@ -12,6 +15,23 @@ runner = CliRunner() +def _can_create_dir_symlink(tmp_path) -> bool: + target = tmp_path / "symlink-target" + link = tmp_path / "symlink-link" + target.mkdir(exist_ok=True) + try: + link.symlink_to(target, target_is_directory=True) + return True + except (OSError, NotImplementedError): + return False + finally: + try: + if link.exists() or link.is_symlink(): + link.unlink() + except OSError: + pass + + def _init_project(tmp_path, integration="copilot"): """Helper: init a spec-kit project with the given integration.""" project = tmp_path / "proj" @@ -1079,10 +1099,8 @@ def test_switch_skips_symlinked_parent_directory(self, tmp_path): Copilot follow-up on #2375: leaf-only symlink check let writes escape when an *ancestor* directory was symlinked outside the project root. """ - import sys - if sys.platform.startswith("win"): - import pytest as _pytest - _pytest.skip("Symlink creation typically requires admin on Windows") + if sys.platform.startswith("win") and not _can_create_dir_symlink(tmp_path): + pytest.skip("Directory symlink creation is not available in this Windows environment") project = _init_project(tmp_path, "claude") bash_dir = project / ".specify" / "scripts" / "bash" diff --git a/tests/test_authentication.py b/tests/test_authentication.py index 5d75355a09..e0ac6c4e8e 100644 --- a/tests/test_authentication.py +++ b/tests/test_authentication.py @@ -16,6 +16,8 @@ import base64 import json import os +from types import SimpleNamespace +from unittest.mock import patch import pytest @@ -268,17 +270,19 @@ def test_valid_star_dot_host_accepted(self, tmp_path): entries = load_auth_config(cfg) assert entries[0].hosts == ("*.visualstudio.com",) - @pytest.mark.skipif(os.name == "nt", reason="POSIX permission bits not supported on Windows") - def test_world_readable_warns(self, tmp_path): + def test_world_readable_warns(self, tmp_path, monkeypatch): import stat + import specify_cli.authentication.config as auth_config cfg = tmp_path / "auth.json" cfg.write_text(json.dumps({ "providers": [{"hosts": ["github.com"], "provider": "github", "auth": "bearer", "token_env": "GH_TOKEN"}] })) - cfg.chmod(stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH) - with pytest.warns(UserWarning, match="readable by group"): - load_auth_config(cfg) + monkeypatch.setattr(auth_config.os, "name", "posix", raising=False) + fake_mode = stat.S_IFREG | stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH + with patch("specify_cli.authentication.config.Path.stat", return_value=SimpleNamespace(st_mode=fake_mode)): + with pytest.warns(UserWarning, match="readable by group"): + load_auth_config(cfg) # --------------------------------------------------------------------------- diff --git a/tests/test_parallel_workers.py b/tests/test_parallel_workers.py new file mode 100644 index 0000000000..1d21c306b0 --- /dev/null +++ b/tests/test_parallel_workers.py @@ -0,0 +1,360 @@ +"""Tests for system-aware parallel worker sizing.""" + +from types import SimpleNamespace + +from tests import _parallel +from tests._parallel import compute_recommended_workers, detect_effective_cpu_count +from tests.conftest import ( + _extract_cli_option, + _args_before_double_dash, + _has_dist_arg, + _has_numprocesses_arg, + _is_plugin_autoload_disabled, + _is_xdist_explicitly_enabled, + _is_xdist_disabled, + pytest_load_initial_conftests, + pytest_report_header, +) + + +def test_worker_count_cpu_bound_when_memory_is_large(): + settings = compute_recommended_workers( + cpu_count=8, + total_memory_bytes=64 * 1024 ** 3, + available_memory_bytes=16 * 1024 ** 3, + platform_name="linux", + max_workers=None, + tier="medium", + ) + # cpu_count - 1, capped by linux platform max (8) + assert settings.workers == 7 + + +def test_worker_count_memory_bound_on_low_memory_system(): + settings = compute_recommended_workers( + cpu_count=8, + total_memory_bytes=3 * 1024 ** 3, + available_memory_bytes=3 * 1024 ** 3, + platform_name="linux", + max_workers=None, + tier="medium", + ) + # 3 GiB => floor(3 / 1.5) == 2 workers + assert settings.workers == 2 + + +def test_worker_count_platform_cap_on_windows(): + settings = compute_recommended_workers( + cpu_count=16, + total_memory_bytes=64 * 1024 ** 3, + available_memory_bytes=64 * 1024 ** 3, + platform_name="win32", + max_workers=None, + tier="medium", + ) + assert settings.workers == 4 + + +def test_worker_count_unknown_platform_uses_most_permissive_known_cap(): + settings = compute_recommended_workers( + cpu_count=32, + total_memory_bytes=128 * 1024 ** 3, + available_memory_bytes=128 * 1024 ** 3, + platform_name="freebsd14", + max_workers=None, + tier="medium", + ) + assert settings.os_cap == 8 + + +def test_worker_count_honors_parallel_max_workers(): + settings = compute_recommended_workers( + cpu_count=16, + total_memory_bytes=64 * 1024 ** 3, + available_memory_bytes=64 * 1024 ** 3, + platform_name="linux", + max_workers=3, + tier="medium", + ) + assert settings.workers == 3 + + +def test_worker_count_never_below_one(): + settings = compute_recommended_workers( + cpu_count=1, + total_memory_bytes=256 * 1024 ** 2, + available_memory_bytes=128 * 1024 ** 2, + platform_name="linux", + max_workers=None, + tier="medium", + ) + assert settings.workers == 1 + + +def test_worker_count_uses_total_memory_when_available_unknown(): + settings = compute_recommended_workers( + cpu_count=8, + total_memory_bytes=3 * 1024 ** 3, + available_memory_bytes=None, + platform_name="linux", + max_workers=None, + tier="medium", + ) + assert settings.workers == 2 + + +def test_worker_count_treats_zero_available_memory_as_known_boundary(): + settings = compute_recommended_workers( + cpu_count=8, + total_memory_bytes=64 * 1024 ** 3, + available_memory_bytes=0, + platform_name="linux", + max_workers=None, + tier="medium", + ) + assert settings.workers == 1 + + +def test_low_tier_is_more_conservative_than_high_tier(): + low = compute_recommended_workers( + cpu_count=12, + total_memory_bytes=64 * 1024 ** 3, + available_memory_bytes=64 * 1024 ** 3, + platform_name="linux", + max_workers=None, + tier="low", + ) + high = compute_recommended_workers( + cpu_count=12, + total_memory_bytes=64 * 1024 ** 3, + available_memory_bytes=64 * 1024 ** 3, + platform_name="linux", + max_workers=None, + tier="high", + ) + assert low.workers < high.workers + + +def test_tier_changes_memory_per_worker_budget(): + low = compute_recommended_workers( + cpu_count=8, + total_memory_bytes=8 * 1024 ** 3, + available_memory_bytes=8 * 1024 ** 3, + platform_name="linux", + max_workers=None, + tier="low", + ) + medium = compute_recommended_workers( + cpu_count=8, + total_memory_bytes=8 * 1024 ** 3, + available_memory_bytes=8 * 1024 ** 3, + platform_name="linux", + max_workers=None, + tier="medium", + ) + high = compute_recommended_workers( + cpu_count=8, + total_memory_bytes=8 * 1024 ** 3, + available_memory_bytes=8 * 1024 ** 3, + platform_name="linux", + max_workers=None, + tier="high", + ) + assert low.memory_per_worker_gib > medium.memory_per_worker_gib > high.memory_per_worker_gib + + +def test_detect_effective_cpu_count_never_below_one(): + assert detect_effective_cpu_count() >= 1 + + +def test_detect_cgroup_cpu_quota_count_v2_parses_cpu_max(monkeypatch): + def fake_read_text(path): + values = { + "/sys/fs/cgroup/cpu.max": "200000 100000", + } + return values.get(path) + + monkeypatch.setattr(_parallel, "_read_text", fake_read_text) + assert _parallel._detect_cgroup_cpu_quota_count() == 2 + + +def test_detect_cgroup_cpu_quota_count_v1_parses_cfs_files(monkeypatch): + def fake_read_text(path): + values = { + "/sys/fs/cgroup/cpu.max": None, + "/sys/fs/cgroup/cpu/cpu.cfs_quota_us": "300000", + "/sys/fs/cgroup/cpu/cpu.cfs_period_us": "100000", + } + return values.get(path) + + monkeypatch.setattr(_parallel, "_read_text", fake_read_text) + assert _parallel._detect_cgroup_cpu_quota_count() == 3 + + +def test_detect_cgroup_cpu_quota_count_v1_parses_cpuacct_cpu_mount(monkeypatch): + def fake_read_text(path): + values = { + "/sys/fs/cgroup/cpu.max": None, + "/sys/fs/cgroup/cpu/cpu.cfs_quota_us": None, + "/sys/fs/cgroup/cpu/cpu.cfs_period_us": None, + "/sys/fs/cgroup/cpu,cpuacct/cpu.cfs_quota_us": None, + "/sys/fs/cgroup/cpu,cpuacct/cpu.cfs_period_us": None, + "/sys/fs/cgroup/cpuacct,cpu/cpu.cfs_quota_us": "250000", + "/sys/fs/cgroup/cpuacct,cpu/cpu.cfs_period_us": "100000", + } + return values.get(path) + + monkeypatch.setattr(_parallel, "_read_text", fake_read_text) + assert _parallel._detect_cgroup_cpu_quota_count() == 2 + + +def test_detect_cgroup_cpu_quota_count_v2_floors_fractional_quota(monkeypatch): + def fake_read_text(path): + values = { + "/sys/fs/cgroup/cpu.max": "110000 100000", + } + return values.get(path) + + monkeypatch.setattr(_parallel, "_read_text", fake_read_text) + assert _parallel._detect_cgroup_cpu_quota_count() == 1 + + +def test_detect_cgroup_cpu_quota_count_v1_floors_fractional_quota(monkeypatch): + def fake_read_text(path): + values = { + "/sys/fs/cgroup/cpu.max": None, + "/sys/fs/cgroup/cpu/cpu.cfs_quota_us": "110000", + "/sys/fs/cgroup/cpu/cpu.cfs_period_us": "100000", + } + return values.get(path) + + monkeypatch.setattr(_parallel, "_read_text", fake_read_text) + assert _parallel._detect_cgroup_cpu_quota_count() == 1 + + +def test_detect_total_memory_uses_sc_pagesize_fallback(monkeypatch): + def fake_sysconf(name): + if name == "SC_PAGE_SIZE": + raise OSError("unsupported") + if name == "SC_PAGESIZE": + return 4096 + if name == "SC_PHYS_PAGES": + return 10 + raise ValueError(name) + + monkeypatch.setattr(_parallel.os, "sysconf", fake_sysconf, raising=False) + monkeypatch.setattr(_parallel.sys, "platform", "linux") + assert _parallel.detect_total_memory_bytes() == 40960 + + +def test_parallel_report_header_formats_zero_memory_values(): + settings = _parallel.ParallelSettings( + tier="medium", + workers=1, + cpu_cap=1, + memory_cap=1, + os_cap=4, + effective_cpus=1, + total_memory_bytes=0, + available_memory_bytes=0, + memory_per_worker_gib=1.5, + ) + config = SimpleNamespace(_spec_kit_parallel_settings=settings) + header = pytest_report_header(config) + assert header is not None + assert "avail_mem=0.0GiB" in header + assert "total_mem=0.0GiB" in header + + +def test_parallel_report_header_uses_effective_workers_when_overridden(): + settings = _parallel.ParallelSettings( + tier="medium", + workers=6, + cpu_cap=6, + memory_cap=8, + os_cap=8, + effective_cpus=8, + total_memory_bytes=16 * 1024 ** 3, + available_memory_bytes=8 * 1024 ** 3, + memory_per_worker_gib=1.5, + ) + config = SimpleNamespace( + _spec_kit_parallel_settings=settings, + _spec_kit_parallel_effective_workers="auto", + ) + header = pytest_report_header(config) + assert header is not None + assert "workers=auto" in header + + +def test_is_xdist_disabled_detects_split_plugin_flag(): + assert _is_xdist_disabled(["--parallel", "-p", "no:xdist"]) + + +def test_is_xdist_disabled_detects_compact_plugin_flag(): + assert _is_xdist_disabled(["--parallel", "-pno:xdist"]) + + +def test_is_xdist_explicitly_enabled_detects_split_plugin_flag(): + assert _is_xdist_explicitly_enabled(["--parallel", "-p", "xdist"]) + + +def test_is_xdist_explicitly_enabled_detects_compact_plugin_flag(): + assert _is_xdist_explicitly_enabled(["--parallel", "-pxdist"]) + + +def test_is_xdist_explicitly_enabled_detects_qualified_plugin_name(): + assert _is_xdist_explicitly_enabled(["--parallel", "-p", "xdist.plugin"]) + + +def test_is_xdist_explicitly_enabled_ignores_non_xdist_plugin_names(): + assert not _is_xdist_explicitly_enabled(["--parallel", "-p", "myxdisthelper"]) + + +def test_numprocesses_and_dist_detection_ignore_args_after_double_dash(): + args = ["--parallel", "--", "-n", "4", "--dist", "load"] + assert not _has_numprocesses_arg(args) + assert not _has_dist_arg(args) + + +def test_extract_cli_option_ignores_args_after_double_dash(): + args = ["--parallel", "--", "--parallel-tier", "high"] + assert _extract_cli_option(args, "--parallel-tier", "medium") == "medium" + + +def test_args_before_double_dash_excludes_parallel_after_sentinel(): + args = ["-q", "--", "--parallel"] + assert "--parallel" not in _args_before_double_dash(args) + + +def test_load_initial_conftests_ignores_parallel_after_sentinel(): + args = ["-q", "--", "--parallel"] + original = list(args) + + pytest_load_initial_conftests(None, None, args) + + assert args == original + + +def test_load_initial_conftests_injects_before_sentinel(monkeypatch): + args = ["--parallel", "--", "tests/test_parallel_workers.py"] + + monkeypatch.setattr("tests.conftest._has_xdist_installed", lambda: True) + monkeypatch.setattr("tests.conftest._is_plugin_autoload_disabled", lambda: False) + monkeypatch.setattr("tests.conftest._is_xdist_disabled", lambda _args: False) + monkeypatch.setattr("tests.conftest._has_numprocesses_arg", lambda _args: False) + monkeypatch.setattr("tests.conftest._compute_parallel_settings_from_args", lambda _args: SimpleNamespace(workers=3)) + + pytest_load_initial_conftests(None, None, args) + + assert args == ["--parallel", "-n", "3", "--dist", "worksteal", "--", "tests/test_parallel_workers.py"] + + +def test_is_plugin_autoload_disabled_truthy(monkeypatch): + monkeypatch.setenv("PYTEST_DISABLE_PLUGIN_AUTOLOAD", "1") + assert _is_plugin_autoload_disabled() + + +def test_is_plugin_autoload_disabled_false_when_unset(monkeypatch): + monkeypatch.delenv("PYTEST_DISABLE_PLUGIN_AUTOLOAD", raising=False) + assert not _is_plugin_autoload_disabled() diff --git a/tests/test_path_utils.py b/tests/test_path_utils.py new file mode 100644 index 0000000000..3b441c95a6 --- /dev/null +++ b/tests/test_path_utils.py @@ -0,0 +1,37 @@ +"""Unit tests for shared path normalization helpers.""" + +import os + +from tests._path_utils import normalize_path_text, path_from_bash_output + + +def test_normalize_path_text_preserves_unc_prefix(): + value = r"\\server\share\\folder" + assert normalize_path_text(value) == "//server/share/folder" + + +def test_normalize_path_text_preserves_slash_normalized_unc_prefix(): + value = "//server/share//folder" + assert normalize_path_text(value) == "//server/share/folder" + + +def test_normalize_path_text_collapses_overprefixed_unc_leading_slashes(): + value = r"\\\\server\share\\folder" + assert normalize_path_text(value) == "//server/share/folder" + + +def test_normalize_path_text_collapses_redundant_non_unc_slashes(): + value = "foo///bar//baz" + assert normalize_path_text(value) == "foo/bar/baz" + + +def test_normalize_path_text_collapses_redundant_posix_leading_slashes(): + value = "///usr//local///bin" + assert normalize_path_text(value) == "/usr/local/bin" + + +def test_path_from_bash_output_trims_quotes_whitespace_and_crlf(): + raw = " '/tmp/my-feature/path'\r\n" + parsed = path_from_bash_output(raw) + expected_suffix = os.path.join("my-feature", "path") + assert str(parsed).endswith(expected_suffix) diff --git a/tests/test_self_upgrade_detection.py b/tests/test_self_upgrade_detection.py index ab575e7435..798dea90b5 100644 --- a/tests/test_self_upgrade_detection.py +++ b/tests/test_self_upgrade_detection.py @@ -733,16 +733,10 @@ def fake_run(argv, *args, **kwargs): class TestEditableInstallMetadata: - @pytest.mark.skipif( - not hasattr(importlib.metadata, "InvalidMetadataError"), - reason=( - "importlib.metadata.InvalidMetadataError does not exist on this " - "Python; _editable_direct_url_path only catches it when present, so " - "fabricating it would exercise a path that cannot fire in production" - ), - ) def test_editable_marker_false_when_metadata_is_invalid(self): - invalid_metadata_error = importlib.metadata.InvalidMetadataError + invalid_metadata_error = getattr(importlib.metadata, "InvalidMetadataError", None) + if invalid_metadata_error is None: + pytest.skip("importlib.metadata.InvalidMetadataError not available on this runtime") with patch( "importlib.metadata.distribution", diff --git a/tests/test_self_upgrade_execution.py b/tests/test_self_upgrade_execution.py index 6696b4fc79..8cd285910f 100644 --- a/tests/test_self_upgrade_execution.py +++ b/tests/test_self_upgrade_execution.py @@ -1,5 +1,6 @@ """Installer execution, verification, and error-path tests for `specify self upgrade`.""" +import os import errno import subprocess from unittest.mock import patch @@ -53,6 +54,8 @@ def test_absolute_installer_path_does_not_require_path_lookup( fake_uv.chmod(0o755) with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( "specify_cli._version.shutil.which", side_effect=lambda name: None + ), patch( + "specify_cli._version.os.access", return_value=True ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="0.7.5" ), patch( @@ -74,16 +77,17 @@ def test_absolute_installer_path_does_not_require_path_lookup( result = runner.invoke(app, ["self", "upgrade"]) assert result.exit_code == 0 - @requires_posix def test_relative_installer_path_does_not_require_path_lookup( - self, monkeypatch, uv_tool_argv0, clean_environ, tmp_path + self, uv_tool_argv0, clean_environ, tmp_path, monkeypatch ): - fake_uv = tmp_path / "uv" + fake_uv = tmp_path / "uv-installer" fake_uv.write_text("#!/bin/sh\n") fake_uv.chmod(0o755) monkeypatch.chdir(tmp_path) with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( "specify_cli._version.shutil.which", side_effect=lambda name: None + ), patch( + "specify_cli._version.os.access", return_value=True ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="0.7.5" ), patch( @@ -91,7 +95,7 @@ def test_relative_installer_path_does_not_require_path_lookup( ), patch( "specify_cli._version._assemble_installer_argv", return_value=[ - "./uv", + "./uv-installer", "tool", "install", "specify-cli", @@ -105,11 +109,10 @@ def test_relative_installer_path_does_not_require_path_lookup( result = runner.invoke(app, ["self", "upgrade"]) assert result.exit_code == 0 - assert mock_run.call_args.args[0][0] == "./uv" + assert mock_run.call_args.args[0][0] == "./uv-installer" - @requires_posix def test_relative_installer_path_missing_gets_path_specific_message( - self, monkeypatch, uv_tool_argv0, clean_environ, tmp_path + self, uv_tool_argv0, clean_environ, tmp_path, monkeypatch ): monkeypatch.chdir(tmp_path) with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( @@ -117,7 +120,7 @@ def test_relative_installer_path_missing_gets_path_specific_message( ), patch("specify_cli._version._get_installed_version", return_value="0.7.5"), patch( "specify_cli._version._assemble_installer_argv", return_value=[ - "./uv", + "./uv-installer", "tool", "install", "specify-cli", @@ -131,7 +134,7 @@ def test_relative_installer_path_missing_gets_path_specific_message( assert result.exit_code == 3 assert ( - "Installer path ./uv no longer exists; reinstall it and retry." + "Installer path ./uv-installer no longer exists; reinstall it and retry." in strip_ansi(result.output) ) assert "not found on PATH" not in strip_ansi(result.output) @@ -151,6 +154,8 @@ def fake_run(argv, *args, **kwargs): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( "specify_cli._version.shutil.which", side_effect=lambda name: str(fake_uv) if name == "uv" else None, + ), patch( + "specify_cli._version.os.access", return_value=True ), patch("specify_cli._version.subprocess.run", side_effect=fake_run), patch( "specify_cli._version._get_installed_version", return_value="0.7.5" ): @@ -169,7 +174,6 @@ def test_absolute_installer_path_not_executable_gets_specific_message( fake_uv = tmp_path / "installer-bin" / "uv" fake_uv.parent.mkdir() fake_uv.write_text("#!/bin/sh\n") - fake_uv.chmod(0o644) with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( "specify_cli._version.shutil.which", side_effect=lambda name: None ), patch("specify_cli._version.os.access", return_value=False), patch( @@ -194,13 +198,11 @@ def test_absolute_installer_path_not_executable_gets_specific_message( in strip_ansi(result.output) ) - @requires_posix def test_relative_installer_path_not_executable_gets_path_specific_message( - self, monkeypatch, uv_tool_argv0, clean_environ, tmp_path + self, uv_tool_argv0, clean_environ, tmp_path, monkeypatch ): - fake_uv = tmp_path / "uv" + fake_uv = tmp_path / "uv-installer" fake_uv.write_text("#!/bin/sh\n") - fake_uv.chmod(0o644) monkeypatch.chdir(tmp_path) with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( "specify_cli._version.shutil.which", side_effect=lambda name: None @@ -209,7 +211,7 @@ def test_relative_installer_path_not_executable_gets_path_specific_message( ), patch( "specify_cli._version._assemble_installer_argv", return_value=[ - "./uv", + "./uv-installer", "tool", "install", "specify-cli", @@ -224,10 +226,10 @@ def test_relative_installer_path_not_executable_gets_path_specific_message( out = strip_ansi(result.output) assert result.exit_code == 3 assert ( - "Installer path ./uv is not an executable file; fix the path or reinstall it and retry." + "Installer path ./uv-installer is not an executable file; fix the path or reinstall it and retry." in out ) - assert "Installer ./uv is not executable" not in out + assert "Installer ./uv-installer is not executable" not in out def test_real_installer_exit_126_is_not_treated_as_invalid_path( self, uv_tool_argv0, clean_environ diff --git a/tests/test_setup_plan_no_overwrite.py b/tests/test_setup_plan_no_overwrite.py index f29a629294..ebfc2dad78 100644 --- a/tests/test_setup_plan_no_overwrite.py +++ b/tests/test_setup_plan_no_overwrite.py @@ -9,6 +9,7 @@ import pytest from tests.conftest import requires_bash +from tests._path_utils import path_from_bash_output PROJECT_ROOT = Path(__file__).resolve().parent.parent COMMON_SH = PROJECT_ROOT / "scripts" / "bash" / "common.sh" @@ -94,7 +95,7 @@ def test_setup_plan_creates_plan_when_missing(plan_repo: Path) -> None: ) assert result.returncode == 0, result.stderr data = json.loads(result.stdout) - plan_path = Path(data["IMPL_PLAN"]) + plan_path = path_from_bash_output(data["IMPL_PLAN"]) assert plan_path.is_file() # Template content should be present content = plan_path.read_text(encoding="utf-8") diff --git a/tests/test_setup_tasks.py b/tests/test_setup_tasks.py index 961124d3a9..1a1c6a366e 100644 --- a/tests/test_setup_tasks.py +++ b/tests/test_setup_tasks.py @@ -4,11 +4,13 @@ import os import shutil import subprocess +import sys from pathlib import Path import pytest from tests.conftest import requires_bash +from tests._path_utils import assert_normalized_path_equal, path_from_bash_output PROJECT_ROOT = Path(__file__).resolve().parent.parent COMMON_SH = PROJECT_ROOT / "scripts" / "bash" / "common.sh" @@ -92,8 +94,51 @@ def _clean_env() -> dict[str, str]: if key.startswith("SPECIFY_"): env.pop(key) return env - - + + +def _is_shell_absolute(path_value: str) -> bool: + return Path(path_value).is_absolute() or path_value.startswith("/") + + +def _assert_tasks_template_matches(tasks_tmpl_raw: str, expected_path: Path) -> None: + assert _is_shell_absolute(tasks_tmpl_raw), "TASKS_TEMPLATE must be an absolute path" + expected = expected_path.resolve() + if os.name == "nt": + tasks_tmpl = path_from_bash_output(tasks_tmpl_raw) + assert tasks_tmpl.is_file(), "TASKS_TEMPLATE must point to an existing file" + assert_normalized_path_equal(tasks_tmpl_raw, expected) + return + tasks_tmpl = Path(tasks_tmpl_raw) + assert tasks_tmpl.is_file(), "TASKS_TEMPLATE must point to an existing file" + assert tasks_tmpl.resolve() == expected, f"Expected {expected} but got: {tasks_tmpl}" + + +def _run_bash_resolve_template(repo: Path, path_override: str | None = None) -> subprocess.CompletedProcess: + script = repo / ".specify" / "scripts" / "bash" / "common.sh" + cmd = 'source "$1"; ' + if path_override is not None: + cmd += 'export PATH="$2"; ' + cmd += 'resolve_template tasks-template "$PWD"' + argv = ["bash", "-c", cmd, "bash", str(script)] + if path_override is not None: + argv.append(path_override) + return subprocess.run( + argv, + cwd=repo, + capture_output=True, + text=True, + check=False, + env=_clean_env(), + ) + + +def _to_bash_path(path: Path) -> str: + value = str(path).replace("\\", "/") + if os.name == "nt" and len(value) >= 2 and value[1] == ":": + return f"/{value[0].lower()}{value[2:]}" + return value + + def _run_bash_format_command(repo: Path, command_name: str) -> subprocess.CompletedProcess: script = repo / ".specify" / "scripts" / "bash" / "common.sh" return subprocess.run( @@ -193,10 +238,10 @@ def test_setup_tasks_bash_core_template_resolved(tasks_repo: Path) -> None: assert result.returncode == 0, result.stderr + result.stdout data = json.loads(result.stdout) - tasks_tmpl = Path(data["TASKS_TEMPLATE"]) - assert tasks_tmpl.is_absolute(), "TASKS_TEMPLATE must be an absolute path" - assert tasks_tmpl.is_file(), "TASKS_TEMPLATE must point to an existing file" - assert tasks_tmpl.name == "tasks-template.md" + _assert_tasks_template_matches( + data["TASKS_TEMPLATE"], + tasks_repo / ".specify" / "templates" / "tasks-template.md", + ) @requires_bash @@ -227,13 +272,7 @@ def test_setup_tasks_bash_override_wins(tasks_repo: Path) -> None: assert result.returncode == 0, result.stderr + result.stdout data = json.loads(result.stdout) - tasks_tmpl = Path(data["TASKS_TEMPLATE"]) - assert tasks_tmpl.is_absolute(), "TASKS_TEMPLATE must be an absolute path" - assert tasks_tmpl.is_file(), "TASKS_TEMPLATE must point to an existing file" - # The resolved path must be inside the overrides directory - assert "overrides" in tasks_tmpl.parts, ( - f"Expected override path but got: {tasks_tmpl}" - ) + _assert_tasks_template_matches(data["TASKS_TEMPLATE"], override_file) @requires_bash @@ -266,12 +305,7 @@ def test_setup_tasks_bash_extension_wins_over_core(tasks_repo: Path) -> None: assert result.returncode == 0, result.stderr + result.stdout data = json.loads(result.stdout) - tasks_tmpl = Path(data["TASKS_TEMPLATE"]) - assert tasks_tmpl.is_absolute(), "TASKS_TEMPLATE must be an absolute path" - assert tasks_tmpl.is_file(), "TASKS_TEMPLATE must point to an existing file" - assert tasks_tmpl == extension_file.resolve(), ( - f"Expected extension path but got: {tasks_tmpl}" - ) + _assert_tasks_template_matches(data["TASKS_TEMPLATE"], extension_file) @requires_bash @@ -310,12 +344,7 @@ def test_setup_tasks_bash_preset_wins_over_extension(tasks_repo: Path) -> None: assert result.returncode == 0, result.stderr + result.stdout data = json.loads(result.stdout) - tasks_tmpl = Path(data["TASKS_TEMPLATE"]) - assert tasks_tmpl.is_absolute(), "TASKS_TEMPLATE must be an absolute path" - assert tasks_tmpl.is_file(), "TASKS_TEMPLATE must point to an existing file" - assert tasks_tmpl == preset_file.resolve(), ( - f"Expected preset path but got: {tasks_tmpl}" - ) + _assert_tasks_template_matches(data["TASKS_TEMPLATE"], preset_file) @requires_bash @@ -328,21 +357,23 @@ def test_setup_tasks_bash_preset_priority_order(tasks_repo: Path) -> None: # resolve_template reads .specify/presets/.registry as a JSON object with a # "presets" map where each entry has a numeric "priority" (lower = higher - # precedence). Create two presets; priority-1-preset wins over priority-2-preset. - high_priority_dir = ( - tasks_repo / ".specify" / "presets" / "priority-1-preset" / "templates" - ) - high_priority_dir.mkdir(parents=True, exist_ok=True) - high_priority_file = high_priority_dir / "tasks-template.md" - high_priority_file.write_text("# high priority preset tasks template\n", encoding="utf-8") + # precedence). Use explicit registry priorities instead of inferring from + # preset IDs so the contract is unambiguous on all platforms. low_priority_dir = ( tasks_repo / ".specify" / "presets" / "priority-2-preset" / "templates" ) - + low_priority_dir.mkdir(parents=True, exist_ok=True) low_priority_file = low_priority_dir / "tasks-template.md" low_priority_file.write_text("# low priority preset tasks template\n", encoding="utf-8") + high_priority_dir = ( + tasks_repo / ".specify" / "presets" / "priority-1-preset" / "templates" + ) + high_priority_dir.mkdir(parents=True, exist_ok=True) + high_priority_file = high_priority_dir / "tasks-template.md" + high_priority_file.write_text("# high priority preset tasks template\n", encoding="utf-8") + # Write .registry JSON using the correct schema: object with "presets" map, # each preset has a numeric "priority" (lower number = higher precedence). registry_json = tasks_repo / ".specify" / "presets" / ".registry" @@ -370,12 +401,7 @@ def test_setup_tasks_bash_preset_priority_order(tasks_repo: Path) -> None: assert result.returncode == 0, result.stderr + result.stdout data = json.loads(result.stdout) - tasks_tmpl = Path(data["TASKS_TEMPLATE"]) - assert tasks_tmpl.is_absolute(), "TASKS_TEMPLATE must be an absolute path" - assert tasks_tmpl.is_file(), "TASKS_TEMPLATE must point to an existing file" - assert tasks_tmpl == high_priority_file.resolve(), ( - f"Expected high-priority preset path but got: {tasks_tmpl}" - ) + _assert_tasks_template_matches(data["TASKS_TEMPLATE"], high_priority_file) @requires_bash @@ -495,13 +521,31 @@ def test_setup_tasks_bash_uses_invoke_separator_in_plan_hint(tasks_repo: Path) - script = tasks_repo / ".specify" / "scripts" / "bash" / "setup-tasks.sh" + env = _clean_env() + if os.name == "nt": + shim_dir = tasks_repo / ".specify" / "shim-bin" + shim_dir.mkdir(parents=True, exist_ok=True) + python3_shim = shim_dir / "python3" + python_exe = sys.executable.replace("\\", "/") + python3_shim.write_text( + f"#!/usr/bin/env bash\n\"{python_exe}\" \"$@\"\n", + encoding="utf-8", + newline="\n", + ) + python3_shim.chmod(0o755) + shim_dir_posix = str(shim_dir).replace("\\", "/") + # Keep inherited PATH bytes unchanged; rewriting Windows PATH delimiters + # can corrupt drive-letter entries under Git Bash. + inherited_path = env.get("PATH", "") + env["PATH"] = f"{shim_dir_posix}:{inherited_path}" if inherited_path else shim_dir_posix + result = subprocess.run( ["bash", str(script), "--json"], cwd=tasks_repo, capture_output=True, text=True, check=False, - env=_clean_env(), + env=env, ) assert result.returncode != 0 @@ -532,6 +576,72 @@ def test_check_prerequisites_bash_uses_invoke_separator_in_tasks_hint( assert "/speckit.tasks" not in result.stderr +@requires_bash +def test_resolve_template_uses_python_when_python3_missing(tasks_repo: Path) -> None: + presets_root = tasks_repo / ".specify" / "presets" + preset_dir = presets_root / "py-fallback" / "templates" + preset_dir.mkdir(parents=True, exist_ok=True) + (preset_dir / "tasks-template.md").write_text("# py fallback\n", encoding="utf-8") + (presets_root / ".registry").write_text(json.dumps({"presets": {"py-fallback": {"priority": 1}}}), encoding="utf-8") + + shim_dir = tasks_repo / ".specify" / "python-fallback-shim" + shim_dir.mkdir(parents=True, exist_ok=True) + python_shim = shim_dir / "python" + python_shim.write_text("#!/usr/bin/env bash\nprintf 'py-fallback\\n'\n", encoding="utf-8", newline="\n") + python_shim.chmod(0o755) + + result = _run_bash_resolve_template(tasks_repo, f"{_to_bash_path(shim_dir)}:/usr/bin:/bin") + + assert result.returncode == 0, result.stderr + _assert_tasks_template_matches(result.stdout.strip(), preset_dir / "tasks-template.md") + + +@requires_bash +def test_resolve_template_trims_crlf_preset_ids(tasks_repo: Path) -> None: + presets_root = tasks_repo / ".specify" / "presets" + preset_dir = presets_root / "crlf-preset" / "templates" + preset_dir.mkdir(parents=True, exist_ok=True) + (preset_dir / "tasks-template.md").write_text("# crlf\n", encoding="utf-8") + (presets_root / ".registry").write_text(json.dumps({"presets": {"crlf-preset": {"priority": 1}}}), encoding="utf-8") + + shim_dir = tasks_repo / ".specify" / "python-crlf-shim" + shim_dir.mkdir(parents=True, exist_ok=True) + python3_shim = shim_dir / "python3" + python3_shim.write_text("#!/usr/bin/env bash\nprintf 'crlf-preset\\r\\n'\n", encoding="utf-8", newline="\n") + python3_shim.chmod(0o755) + + result = _run_bash_resolve_template(tasks_repo, f"{_to_bash_path(shim_dir)}:/usr/bin:/bin") + + assert result.returncode == 0, result.stderr + _assert_tasks_template_matches(result.stdout.strip(), preset_dir / "tasks-template.md") + + +@requires_bash +def test_resolve_template_fallback_scan_is_deterministic_when_python_fails(tasks_repo: Path) -> None: + presets_root = tasks_repo / ".specify" / "presets" + a_dir = presets_root / "a-preset" / "templates" + b_dir = presets_root / "b-preset" / "templates" + a_dir.mkdir(parents=True, exist_ok=True) + b_dir.mkdir(parents=True, exist_ok=True) + (a_dir / "tasks-template.md").write_text("# a\n", encoding="utf-8") + (b_dir / "tasks-template.md").write_text("# b\n", encoding="utf-8") + (presets_root / ".registry").write_text("{invalid json", encoding="utf-8") + + shim_dir = tasks_repo / ".specify" / "python-fail-shim" + shim_dir.mkdir(parents=True, exist_ok=True) + fail_script = "#!/usr/bin/env bash\nexit 1\n" + (shim_dir / "python3").write_text(fail_script, encoding="utf-8", newline="\n") + (shim_dir / "python").write_text(fail_script, encoding="utf-8", newline="\n") + (shim_dir / "python3").chmod(0o755) + (shim_dir / "python").chmod(0o755) + + path_override = f"{_to_bash_path(shim_dir)}:/usr/bin:/bin" + result = _run_bash_resolve_template(tasks_repo, path_override) + + assert result.returncode == 0, result.stderr + _assert_tasks_template_matches(result.stdout.strip(), a_dir / "tasks-template.md") + + @requires_bash def test_setup_tasks_bash_passes_custom_branch_when_feature_json_valid( tasks_repo: Path, diff --git a/tests/test_timestamp_branches.py b/tests/test_timestamp_branches.py index 3f6d8bd2a8..a2d7121209 100644 --- a/tests/test_timestamp_branches.py +++ b/tests/test_timestamp_branches.py @@ -14,6 +14,7 @@ import pytest from tests.conftest import requires_bash +from tests._path_utils import assert_shell_path_matches PROJECT_ROOT = Path(__file__).resolve().parent.parent CREATE_FEATURE = PROJECT_ROOT / "scripts" / "bash" / "create-new-feature.sh" @@ -214,7 +215,11 @@ def test_long_name_truncation(self, git_repo: Path): """Test 5: Long branch name is truncated to <= 244 chars.""" long_name = "a-" * 150 + "end" result = run_script(git_repo, "--timestamp", "--short-name", long_name, "Long feature") - assert result.returncode == 0, result.stderr + if result.returncode != 0: + # On Windows, deep temp paths can still exceed fs limits even after truncation. + assert os.name == "nt" + assert re.search(r"too\s+long", result.stderr, flags=re.IGNORECASE) + pytest.xfail("Windows path-length limitation exceeded during long-name truncation test") branch = None for line in result.stdout.splitlines(): if line.startswith("BRANCH_NAME:"): @@ -409,7 +414,7 @@ def test_bash_specify_feature_prefixed_resolves_by_prefix(self, tmp_path: Path): text=True, ) assert result.returncode == 0, result.stderr - assert result.stdout.strip() == str(tmp_path / "specs" / "001-target-spec") + assert_shell_path_matches(result.stdout.strip(), tmp_path / "specs" / "001-target-spec") @pytest.mark.skipif(not _has_pwsh(), reason="pwsh not installed") @@ -1163,11 +1168,10 @@ def test_env_var_overrides_branch_lookup(self, git_repo: Path): env={**os.environ, "SPECIFY_FEATURE_DIRECTORY": str(custom_dir)}, ) assert result.returncode == 0, result.stderr - assert str(custom_dir) in result.stdout for line in result.stdout.splitlines(): if line.startswith("FEATURE_DIR="): val = line.split("=", 1)[1].strip("'\"") - assert val == str(custom_dir) + assert_shell_path_matches(val, custom_dir) break else: pytest.fail("FEATURE_DIR not found in output") @@ -1194,7 +1198,7 @@ def test_feature_json_overrides_branch_lookup(self, git_repo: Path): for line in result.stdout.splitlines(): if line.startswith("FEATURE_DIR="): val = line.split("=", 1)[1].strip("'\"") - assert val == str(custom_dir) + assert_shell_path_matches(val, custom_dir) break else: pytest.fail("FEATURE_DIR not found in output") @@ -1224,7 +1228,7 @@ def test_env_var_takes_priority_over_feature_json(self, git_repo: Path): for line in result.stdout.splitlines(): if line.startswith("FEATURE_DIR="): val = line.split("=", 1)[1].strip("'\"") - assert val == str(env_dir) + assert_shell_path_matches(val, env_dir) break else: pytest.fail("FEATURE_DIR not found in output") @@ -1246,7 +1250,7 @@ def test_fallback_to_branch_lookup(self, git_repo: Path): for line in result.stdout.splitlines(): if line.startswith("FEATURE_DIR="): val = line.split("=", 1)[1].strip("'\"") - assert val == str(spec_dir) + assert_shell_path_matches(val, spec_dir) break else: pytest.fail("FEATURE_DIR not found in output")