Repository navigation
fix(tests): skip MAX_PATH-exceeding blocking-io anchors on Windows - #6361
Conversation
github-actions
Bot
added
risk:medium
willem-bd
left a comment
willem-bd
left a comment
There was a problem hiding this comment.
The approach (skip Windows on the MAX_PATH-bound anchors, keep POSIX running) matches the #6257/#5931 convention and the issue's proposed fix, but the import placement will fail backend CI lint (ruff check enforces isort via backend/ruff.toml) in all three files, and the module-level marker in the promotion file skips two anchors that are not MAX_PATH-bound.
| import json | ||
| from types import SimpleNamespace | ||
|
|
||
| import os |
There was a problem hiding this comment.
Lint: ruff check fails here with I001 (important). backend/ruff.toml selects the I (isort) rules and CI's lint-backend job runs make lint on every PR, so this import block fails CI: import os must sit with the other stdlib straight imports (import functools / import os / from types import SimpleNamespace), not after the from types import. Verified by running ruff 0.15.12 with the repo config against the head revision of this file. Fix: move the import up (or run make format).
| import functools | ||
| from types import SimpleNamespace | ||
|
|
||
| import os |
There was a problem hiding this comment.
Lint: ruff check fails here with I001 (important). Same issue as the other two files: import os belongs next to import functools in the stdlib section, and CI's lint-backend job (which runs make lint with the I rules from backend/ruff.toml) will fail on this. Fix: move the import up (or run make format).
| import functools | ||
| from types import SimpleNamespace | ||
|
|
||
| import os |
There was a problem hiding this comment.
Lint: ruff check fails here with I001 (important). Same issue as the other two files: import os must be sorted with the other stdlib straight imports (import functools / import os / from types import SimpleNamespace); the current placement fails CI lint. Fix: move the import up (or run make format).
| pytestmark = [ | ||
| pytest.mark.asyncio, | ||
| pytest.mark.allow_blocking_io, | ||
| pytest.mark.skipif( |
There was a problem hiding this comment.
Suggestion: this module-level skip also skips two anchors that are not MAX_PATH-bound. Issue #6353 lists exactly 2 of this file's 4 anchors as failing on Windows (test_attach_staging_copy_dispatches_off_the_loop, test_attach_staged_read_dispatches_off_the_loop). test_ingestion_open_and_link_commit_dispatch_off_the_loop and test_thread_files_listing_dispatches_off_the_loop exercise the shallow uploads/thread-files layout (not the deep users/<uid>/projects/<hash>/documents/... shelf) and reportedly pass on Windows, but a module-level pytestmark silently drops their Windows coverage too. Consider per-anchor @pytest.mark.skipif(os.name == "nt", ...) decorators on just the 12 MAX_PATH-bound anchors, matching the convention merged in #6257 (which skipped only the affected tests).
…ch anchors Per review: module-level skip also skipped ingestion/thread-files anchors that are not MAX_PATH-bound. Now only the two attach staging anchors skip on Windows.
|
Thanks for the detailed review! Addressed both points:
Commit: 96ffc8c. |
|
Hi @willem-bd — I've pushed the fix (commit |
Closes #6353.
Twelve blocking-io anchors under
backend/tests/blocking_io/fail deterministically on Windows hosts withOSError: [Errno 206] Filename or extension too longbefore the dispatch under test can run. The anchors exercise the real document layout on disk (users/<uid>/projects/<hash>/documents/<xx>/<sha256>/<id>/original), and under a pytesttmp_pathprefix that nesting pushes past the Windows MAX_PATH limit of 260 characters.This PR applies a shared module-level
pytest.mark.skipif(os.name == "nt", ...)marker to the three affected files, matching the host-bound anchor convention already merged in #6257 (and #5931). The anchors keep running on POSIX where they are valuable; on Windows they are skipped so the suite stays green and real regressions stand out.Verified:
python -m py_compile).No production code changes.