Skip symlink tests when the host cannot create symlinks by Zuhef · Pull Request #13286 · aio-libs/aiohttp

Zuhef · 2026-07-30T05:12:05Z

What do these changes do?

Six tests create symlinks unconditionally. On Windows os.symlink() requires
SeCreateSymbolicLinkPrivilege, which is only held by an elevated process or when
Developer Mode is enabled, so on an unprivileged local checkout these tests fail with
OSError: [WinError 1314] A required privilege is not held by the client rather than
skipping.

This adds a symlinks_supported fixture to tests/conftest.py that probes the
capability once in a throwaway temporary directory and skips only when the privilege is
genuinely missing, then requests it from the six affected tests. It follows the existing
unix_sockname pattern, which already skips with pytest.skip("requires UNIX sockets").

The probe deliberately tests the capability instead of the platform. A plain
skipif(sys.platform == "win32") would also disable these tests on CI, where the
privilege is held — that would silently drop coverage, which is worse than the
problem being fixed.

Are there changes in behavior for the user?

No. Test-only change; no library code is touched.

Is it a substantial burden for the maintainers to support this?

No — it is one small fixture plus six signature changes, and it removes a
platform-specific failure mode rather than adding a configuration surface. The fixture
has no dependencies beyond tempfile/pathlib, both already imported in conftest.py.
The one judgement call worth reviewing is capability-probing vs. platform-gating; I chose
probing specifically so CI coverage is preserved.

Related issue number

None — found while running the suite on Windows. I did not open a separate issue to avoid
adding tracker noise for a test-only fix; happy to file one if you would prefer that.

Checklist

I think the code is well written
Unit tests for the changes exist
- N/A in the strict sense — this is a test-infrastructure change. Both branches of
  the new fixture were verified explicitly (see the log block below).
Documentation reflects the changes
- N/A — no user-facing behaviour changed.
If you provide code modification, please add yourself to CONTRIBUTORS.txt
Add a new news fragment into the CHANGES/ folder

Verification log (Windows 11, Python 3.13, AIOHTTP_NO_EXTENSIONS=1)

Before, full suite — six symlink tests error out:

FAILED tests/test_urldispatch.py::test_add_static_append_version_follow_symlink - OSError: [WinError 1314] ...
FAILED tests/test_urldispatch.py::test_add_static_append_version_not_follow_symlink - OSError: [WinError 1314] ...
FAILED tests/test_web_urldispatcher.py::test_access_symlink_loop - OSError: [WinError 1314] ...
FAILED tests/test_web_urldispatcher.py::test_access_compressed_file_as_symlink - OSError: [WinError 1314] ...
FAILED tests/test_web_urldispatcher.py::test_follow_symlink - OSError: [WinError 1314] ...
FAILED tests/test_web_urldispatcher.py::test_follow_symlink_directory_traversal_after_normalization - OSError: [WinError 1314] ...
7 failed, 4470 passed, 80 skipped, 14 xfailed in 110.66s

After — they skip with a reason, and the suite is otherwise unchanged:

SKIPPED [1] tests/test_web_urldispatcher.py:201: requires privilege to create symlinks: [WinError 1314] ...
SKIPPED [1] tests/test_web_urldispatcher.py:261: requires privilege to create symlinks: [WinError 1314] ...
SKIPPED [1] tests/test_web_urldispatcher.py:524: requires privilege to create symlinks: [WinError 1314] ...
SKIPPED [1] tests/test_web_urldispatcher.py:544: requires privilege to create symlinks: [WinError 1314] ...
SKIPPED [1] tests/test_urldispatch.py:466: requires privilege to create symlinks: [WinError 1314] ...
SKIPPED [1] tests/test_urldispatch.py:490: requires privilege to create symlinks: [WinError 1314] ...
1 failed, 4470 passed, 86 skipped, 14 xfailed in 106.17s

test_follow_symlink_directory_traversal still runs — it does not create a symlink, so
the fixture was deliberately not added to it.

Both fixture branches were checked, since this host cannot exercise the privileged path:

[branch: real host    ] SKIPPED -> Skipped
[branch: capable host ] did NOT skip -> tests RUN (CI coverage preserved)
PASS: skips only when the privilege is genuinely absent

The capable-host branch was exercised by making the probe succeed, which is what happens
on POSIX and on your Windows CI runners.

Lint and types:

pre-commit run --files tests/conftest.py tests/test_urldispatch.py \
                       tests/test_web_urldispatcher.py CONTRIBUTORS.txt
  isort / black / pyupgrade / flake8 / codespell / file contents sorter ... Passed
mypy tests/conftest.py tests/test_urldispatch.py tests/test_web_urldispatcher.py
  Found 21 errors in 7 files   # identical to baseline on an unmodified tree

Two notes, both outside the scope of this change and left alone:

tests/test_circular_imports.py::test_no_warnings[aiohttp._websocket.reader_c] is the
remaining failure above. It fails under AIOHTTP_NO_EXTENSIONS=1 because the Cython
module is absent. Say the word and I will open a separate issue.
The 21 mypy errors are pre-existing and Windows-specific (socket.AF_UNIX missing, an
unused type: ignore on the proactor branch).

Drafted with Kiro CLI (claude-opus-5); reviewed by @Zuhef.

Six tests create symlinks unconditionally. On Windows os.symlink() needs SeCreateSymbolicLinkPrivilege, which is only held by an elevated process or when Developer Mode is enabled, so an unprivileged local checkout fails them with `OSError: [WinError 1314] A required privilege is not held by the client` instead of skipping. Add a `symlinks_supported` fixture that probes the capability in a throwaway temporary directory and skips only when the privilege is genuinely missing. CI holds the privilege, so these tests continue to run there and no coverage is lost.

codecov · 2026-07-30T05:15:25Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.98%. Comparing base (c3f07fc) to head (2a75392).
⚠️ Report is 3 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #13286   +/-   ##
=======================================
  Coverage   98.98%   98.98%           
=======================================
  Files         132      132           
  Lines       49023    49030    +7     
  Branches     2551     2551           
=======================================
+ Hits        48526    48533    +7     
  Misses        373      373           
  Partials      124      124

Flag	Coverage Δ
Autobahn	`22.12% <28.57%> (+<0.01%)`	⬆️
CI-GHA	`98.90% <100.00%> (+<0.01%)`	⬆️
OS-Linux	`98.67% <100.00%> (-0.01%)`	⬇️
OS-Windows	`97.03% <100.00%> (+<0.01%)`	⬆️
OS-macOS	`97.93% <100.00%> (-0.01%)`	⬇️
Py-3.10	`98.11% <100.00%> (+<0.01%)`	⬆️
Py-3.11	`98.37% <100.00%> (+<0.01%)`	⬆️
Py-3.12	`98.46% <100.00%> (+<0.01%)`	⬆️
Py-3.13	`98.44% <100.00%> (+<0.01%)`	⬆️
Py-3.14	`98.45% <100.00%> (-0.01%)`	⬇️
Py-3.14t	`97.56% <100.00%> (+<0.01%)`	⬆️
Py-pypy-3.11	`97.39% <100.00%> (-0.01%)`	⬇️
VM-macos	`97.93% <100.00%> (-0.01%)`	⬇️
VM-ubuntu	`98.67% <100.00%> (-0.01%)`	⬇️
VM-windows	`97.03% <100.00%> (+<0.01%)`	⬆️
cython-coverage	`37.97% <28.57%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

codspeed-hq · 2026-07-30T05:24:24Z

Merging this PR will not alter performance

✅ 84 untouched benchmarks
⏩ 83 skipped benchmarks¹

_{Comparing Zuhef:fix/beta-skip-symlink-tests-without-privilege (2a75392) with master (c3f07fc)²}

83 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
No successful run was found on master (9bb501f) during the generation of this report, so c3f07fc was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

The except branch only runs on a host without SeCreateSymbolicLinkPrivilege, so it is never executed on CI and showed up as a patch coverage miss. pragma: no cover is already used for platform-conditional code elsewhere in tests/.

Zuhef added 2 commits July 30, 2026 10:41

Add change note for the symlink test skip

61fc97e

psf-chronographer Bot added the bot:chronographer:provided There is a change note present in this PR label Jul 30, 2026

Zuhef marked this pull request as ready for review July 30, 2026 14:56

Zuhef requested review from asvetlov and webknjaz as code owners July 30, 2026 14:56

Navigation Menu

Uh oh!

Uh oh!

Skip symlink tests when the host cannot create symlinks - #13286

Skip symlink tests when the host cannot create symlinks#13286
Zuhef wants to merge 3 commits into
aio-libs:masterfrom
Zuhef:fix/beta-skip-symlink-tests-without-privilege

Zuhef commented Jul 30, 2026

Uh oh!

codecov Bot commented Jul 30, 2026 •

edited

Loading

Uh oh!

codspeed-hq Bot commented Jul 30, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

Zuhef commented Jul 30, 2026

What do these changes do?

Are there changes in behavior for the user?

Is it a substantial burden for the maintainers to support this?

Related issue number

Checklist

Uh oh!

codecov Bot commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

codspeed-hq Bot commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

codecov Bot commented Jul 30, 2026 •

edited

Loading

codspeed-hq Bot commented Jul 30, 2026 •

edited

Loading