Skip to content

test: skip POSIX-only fixtures on Windows instead of failing - #2920

Open
sashankh wants to merge 1 commit into
Graphify-Labs:v8from
sashankh:test/posix-only-guards
Open

test: skip POSIX-only fixtures on Windows instead of failing#2920
sashankh wants to merge 1 commit into
Graphify-Labs:v8from
sashankh:test/posix-only-guards

Conversation

@sashankh

Copy link
Copy Markdown

Fixes #2919.

The problem

Five tests construct their fixture out of POSIX-only OS behaviour with no platform guard, so
on Windows they fail before the code under test is reached:

tests/test_non_regular_files.py::test_fifo_is_rejected
    AttributeError: module 'os' has no attribute 'mkfifo'
tests/test_non_regular_files.py::test_unix_socket_is_rejected
    AttributeError: module 'socket' has no attribute 'AF_UNIX'
tests/test_non_regular_files.py::test_symlink_pointing_at_a_fifo_is_rejected
    AttributeError: module 'os' has no attribute 'mkfifo'
tests/test_watch.py::test_rebuild_code_deleted_cwd_without_repo_root_returns_false
    PermissionError: [WinError 32] ... '...\gone'
tests/test_watch.py::test_rebuild_code_deleted_cwd_uses_graphify_repo_root
    PermissionError: [WinError 32] ... '...\gone'

The last two come from

os.chdir(gone)
gone.rmdir()      # POSIX unlinks a live CWD; Windows holds an open handle on it

which is the exact state the test wants (_rebuild_code surviving a detached hook's deleted
CWD) — and which Windows cannot be made to enter at all.

This is the argument #2642 already made for symlinks:

A plain non-elevated Windows shell raises OSError: [WinError 1314], which pytest reports as
a FAILURE — 15 of them, drowning out real defects — when what it means is "unsupported here".

tests/test_watch.py already skips ten tests with fcntl-only (POSIX), so the practice is
settled; these five were simply missed.

The change

Extends the _can_symlink / requires_symlinks pair in tests/conftest.py with siblings for
the three capabilities involved:

fixture guards
requires_fifo os.mkfifo
requires_unix_socket binding an AF_UNIX socket
requires_deletable_cwd removing a directory that is the process CWD

Each is probed, not inferred from sys.platform#2642's reasoning applies unchanged:
Windows 10+ does support AF_UNIX and CPython exposes it on some builds, and a POSIX host can
lack FIFOs when its temp dir is on a filesystem that has none. Hosts that can do these things
keep the coverage.

They live in conftest.py beside _can_symlink rather than in the two test files, so the
"probe the capability, don't guess from the platform name" doctrine stays in one place and the
next test needing a FIFO finds it.

Also adds the existing requires_symlinks fixture to the two symlink tests in
test_non_regular_files.py that never took it — test_symlink_to_a_regular_file_is_accepted
and test_broken_symlink_is_rejected_without_raising. They pass on a Windows box with
Developer Mode enabled and raise WinError 1314 on one without, which is precisely the case
that fixture exists for.

Verification

Windows 11 / Python 3.12.10, v8 @ b2cd362.

Targeted:

$ pytest tests/test_non_regular_files.py tests/test_watch.py -q -rs
120 passed, 17 skipped
  SKIPPED tests/test_non_regular_files.py:37: named pipes (os.mkfifo) unavailable on this platform
  SKIPPED tests/test_non_regular_files.py:45: AF_UNIX sockets unavailable on this platform
  SKIPPED tests/test_non_regular_files.py:68: named pipes (os.mkfifo) unavailable on this platform
  SKIPPED tests/test_watch.py:654: cannot remove a directory that is the process CWD on this
                                   platform (Windows holds an open handle: WinError 32)
  SKIPPED tests/test_watch.py:680: (same)

Full suite, before and after:

failures passed skipped
origin/v8 50 4511 216
this branch 44 4512 221

Set-diff of the FAILED ids: no new failures. The six that stopped failing are the five
guarded here plus test_incremental_mtime_collision.py::test_same_size_rewrite_in_one_tick_is_requeued,
which is an unrelated timing-sensitive test that happened to pass on this run — not something
this PR fixes.

Tests only — no change under graphify/. On Linux every probe succeeds, nothing skips, and CI
behaviour is unchanged.

…y-Labs#2919)

Five tests build their fixture out of POSIX-only OS behaviour with no platform
guard, so on Windows they fail before the code under test is ever reached:

  os.mkfifo(fifo)                     AttributeError: no attribute 'mkfifo'
  socket.socket(socket.AF_UNIX, ...)  AttributeError: no attribute 'AF_UNIX'
  os.chdir(gone); gone.rmdir()        PermissionError: [WinError 32]

That is the case Graphify-Labs#2642 already made for symlinks - a failure that means
'unsupported here' drowns out real defects - and test_watch.py already skips
ten tests with 'fcntl-only (POSIX)'. These five were missed.

Extends the _can_symlink/requires_symlinks pair in conftest.py with siblings
for the three capabilities involved. Probed rather than inferred from
sys.platform, for Graphify-Labs#2642's reason: Windows 10+ does support AF_UNIX and CPython
exposes it on some builds, and a POSIX host can lack FIFOs when its temp dir
sits on a filesystem without them, so hosts that can do these things keep the
coverage.

Also adds the existing requires_symlinks fixture to the two symlink tests in
test_non_regular_files.py that never took it. They pass on a Windows box with
Developer Mode enabled and raise WinError 1314 on one without - exactly what
that fixture exists for.

Tests only. On Linux every probe succeeds and nothing skips, so CI is
unaffected.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 2 advisory finding(s) below merit a look before merge.


Graphify review — findings

Adds probed requires_fifo, requires_unix_socket, and requires_deletable_cwd skip fixtures to tests/conftest.py, each testing the capability at runtime rather than by platform name. Applies them (plus the existing requires_symlinks) to the FIFO, unix-socket, symlink, and deleted-CWD tests in test_non_regular_files.py and test_watch.py so they skip cleanly on Windows and unsupported filesystems.

Worth a look

  • AF_UNIX probe does not handle socket creation failurestests/conftest.py:80 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
  • AF_UNIX probe leaks socket-construction OSError instead of reporting unsupportedtests/conftest.py:96 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 310 functions depend on the 310 functions this change touches.

Health — grade A; no new coupling hotspots.

Verification — 310 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 310 function(s) in the blast radius were not formally verified this run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tests: five POSIX-only fixtures (mkfifo, AF_UNIX, rmdir of the CWD) fail instead of skipping on Windows

1 participant