Skip to content

test(windows): auto-skip symlink tests when filesystem can't symlink#633

Merged
memtomem merged 2 commits intomainfrom
fix/windows-symlink-test-skip
May 1, 2026
Merged

test(windows): auto-skip symlink tests when filesystem can't symlink#633
memtomem merged 2 commits intomainfrom
fix/windows-symlink-test-skip

Conversation

@memtomem
Copy link
Copy Markdown
Owner

@memtomem memtomem commented May 1, 2026

Summary

Five tests (and one whole class via its fixture) call Path.symlink_to /
os.symlink directly. On Windows without Developer Mode or admin
privileges, that raises OSError: [WinError 1314] (client does not hold the required privilege) — and because most failures land in fixture
setup
, they take down otherwise-unrelated tests in the same class.
Surfaced when running uv run pytest on a fresh Windows shell. CI is
Linux-only (tracked separately in #634) so the regression is silent
there.

This PR adds a requires_symlinks pytest marker. conftest.py runs a
one-shot probe at import time (mirrors the existing ollama probe) that
attempts both a file-to-file and a directory symlink, and
pytest_collection_modifyitems auto-skips marked tests when either
branch fails. POSIX runs and Windows-with-Developer-Mode runs have the
probe return True, so the marker is a no-op there.

Both symlink kinds are probed because Windows historically treated them
as separate privilege classes, and TestFsList.fs_tree specifically
uses symlink_to(..., target_is_directory=True).

Tests marked:

File Site Why marked
test_runtime_paths.py test_falls_back_when_xdg_is_symlink calls os.symlink
test_runtime_paths.py test_refuses_existing_symlink calls os.symlink
test_context_settings.py test_symlink_target_is_treated_as_host_write Path.symlink_to
test_context_projects.py test_discover_symlink_dedup Path.symlink_to
test_context_install.py test_install_skips_symlinks_in_source Path.symlink_to
test_web_routes.py TestFsList class the fs_tree fixture creates 3 symlinks (incl. directory symlinks); all 14 tests in the class inherit the dependency

The marker is registered in root pyproject.toml so unknown-marker
warnings stay quiet.

Test plan

  • ruff check + ruff format --check clean
  • macOS (probe → True, both file + dir branches): all 19 marked
    tests PASS — no behavior change on POSIX or on Developer-Mode
    Windows
  • Probe forced to False (simulating locked-down Windows): all 19
    marked tests SKIPPED with reason "Filesystem cannot create symlinks (Windows without Developer Mode/admin)" — including the
    full 14-test TestFsList class
  • Real Windows runner is tracked in ci: add Windows runner to CI matrix #634 — until then this fix is
    verified by the forced-probe simulation

Notes

This is a test-runner ergonomics fix only — production code is
unchanged. The actual root fix (a Windows CI runner that would have
caught this on its own) is out of scope and tracked separately in #634.

Caveat: the probe runs in tempfile.gettempdir(), which is what
tmp_path defaults to. Users running pytest --basetemp=... pointed at
a filesystem with different symlink semantics (FAT32, certain network
mounts) may still see marked tests fail despite the probe passing — see
_can_create_symlink docstring.

🤖 Generated with Claude Code

…t symlink

On Windows, ``os.symlink`` requires Developer Mode or admin privileges;
without them every symlink-touching test raises
``OSError: [WinError 1314] (client does not hold the required privilege)``
during fixture setup, blowing up otherwise-unrelated tests in the same
class. CI is Linux-only so this regression is invisible there, but it
makes the suite unrunnable for contributors on a stock Windows shell.

Add a ``requires_symlinks`` pytest marker. ``conftest.py`` runs a one-shot
``Path.symlink_to`` probe at import time and ``pytest_collection_modifyitems``
auto-skips marked tests when the probe fails (mirrors the existing
``ollama`` skip pattern). Mark the five sites that create real symlinks:

- ``tests/test_runtime_paths.py``: ``test_falls_back_when_xdg_is_symlink``,
  ``test_refuses_existing_symlink``
- ``tests/test_context_settings.py``: ``test_symlink_target_is_treated_as_host_write``
- ``tests/test_context_projects.py``: ``test_discover_symlink_dedup``
- ``tests/test_context_install.py``: ``test_install_skips_symlinks_in_source``
- ``tests/test_web_routes.py``: entire ``TestFsList`` class (the
  ``fs_tree`` fixture creates symlinks, so all 14 tests in the class
  inherit the requirement)

Verified locally:
- macOS (probe=True): 19/19 marked tests PASS — no behavior change on
  POSIX or on Windows + Developer Mode.
- Probe forced to False: 19/19 SKIPPED with a precise reason string —
  matches the locked-down-Windows experience.

Co-Authored-By: Claude <[email protected]>
@memtomem memtomem mentioned this pull request May 1, 2026
4 tasks
Review feedback on PR #633: the probe in ``_can_create_symlink`` only
created a file-to-file symlink, but ``TestFsList.fs_tree`` uses
``symlink_to(..., target_is_directory=True)``. Historically Windows
treated directory symlinks as a separate privilege class from file
symlinks (``SeCreateSymbolicLinkPrivilege`` covers both today, but
hardened or older configurations could split them). On such a host the
file-only probe would pass and the 14-test ``TestFsList`` class would
still hard-fail in fixture setup — exactly the regression this PR is
meant to prevent.

Also documents the ``--basetemp`` caveat: the probe runs in
``tempfile.gettempdir()``, so users with ``--basetemp`` pointed at a
filesystem with different symlink semantics (FAT32, some network mounts)
may still see marked tests fail despite the probe passing.

Co-Authored-By: Claude <[email protected]>
@memtomem memtomem merged commit a4c276d into main May 1, 2026
7 checks passed
@memtomem memtomem deleted the fix/windows-symlink-test-skip branch May 1, 2026 02:55
@github-actions github-actions Bot locked and limited conversation to collaborators May 1, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants