ci: add Windows unit-test job to catch OS-specific path bugs - #1522
ci: add Windows unit-test job to catch OS-specific path bugs#1522dimitri-yatsenko wants to merge 1 commit into
Conversation
The CI matrix ran Linux-only, so platform-specific defects — e.g. #1520, where file-protocol paths rendered with native backslashes broke garbage collection on Windows — could not be caught. Add a windows-latest leg running the container-free unit suite across both ends of the supported Python range (3.10, 3.14). pixi targets linux/osx only ([tool.pixi] platforms), so this leg installs via pip (.[test]) rather than pixi. Unit tests need no containers, so no Docker/DB is required on Windows.
MilagrosMarin
left a comment
There was a problem hiding this comment.
CI proves this out — unit-tests-windows (py3.10) and (py3.14) both pass on windows-latest (~2 min each). Verified the supporting claims: [tool.pixi.workspace] platforms genuinely has no win-64 so the pip fallback is unavoidable; fetch-depth: 0 is correctly present for hatch-vcs on the editable install; the Linux unit-tests job is untouched; and the 3.10/3.14 matrix matches requires-python.
One thing worth addressing: the Windows leg resolves a different test dependency set than the Linux leg. pyproject.toml defines test twice — [dependency-groups].test (PEP 735, what pixi uses) includes graphviz; [project.optional-dependencies].test (what pip install -e ".[test]" resolves) instead includes s3fs and psycopg2-binary and omits graphviz.
Nothing in tests/unit/ or src/datajoint/ imports graphviz today, so there's no live breakage — but it's a latent trap: a future unit test touching graphviz would pass on Linux and error on Windows, which reads as an OS bug when it's really a dep-set mismatch. A comment in the new job noting that the pip leg resolves [project.optional-dependencies] (not the pixi group) would save someone that debugging session.
Also worth confirming the merge order: the body cites test_storage_adapter.py::test_file_protocol_full_path_uses_forward_slashes as the guard, but that test lands in #1521, not here. Merged alone, this PR gives a Windows runner with no #1520-specific coverage — so #1521 should go first (or they merge together) for the pairing to actually catch the motivating bug.
Why
The CI matrix ran Linux-only, so OS-specific defects couldn't be caught. #1520 is the motivating case:
StorageBackend._full_path()rendered file-protocol paths with native backslashes on Windows, which broke garbage collection (every live file looked orphaned;collect()could delete referenced files). #1521 fixes it, but nothing in CI exercises Windows, so the regression — and its whole class — would recur silently.What
Adds a
unit-tests-windowsjob onwindows-latestrunning the container-free unit suite (tests/unit) across both ends of the supported Python range (3.10, 3.14).testjob needs Docker/Linux containers (testcontainers), which don't fit Windows runners. The unit suite needs no containers and is where the path-handling logic (incl. Bug: GarbageCollector deletes live files on Windows due to backslash/forward-slash path mismatch in file-protocol stores #1520) actually lives, so it's the right and sufficient surface for a Windows guard.[tool.pixi] platformshas nowin-64, and the lock isn't solved for it), so this leg usesactions/setup-python+pip install -e ".[test]". Thetestextra's deps all ship Windows wheels. The existing Linuxunit-tests(pixi) job is unchanged.fetch-depth: 0sohatch-vcscan derive the version on the editable install.Notes
test_storage_adapter.py::test_file_protocol_full_path_uses_forward_slashesfails. (As noted in the Use POSIX separators in file-protocol storage URLs and paths #1521 review, an end-to-end GC integration test on Windows would strengthen this further — this PR provides the runner; the deeper test can follow.)win-64is added to the pixi platforms and the lock re-solved.