Skip to content

geotiff: fix test_writer_returns_are_not_none on Windows (#1938)#1960

Merged
brendancol merged 1 commit into
mainfrom
fix-ci-vrt-writer-tempdir-windows-1938
May 15, 2026
Merged

geotiff: fix test_writer_returns_are_not_none on Windows (#1938)#1960
brendancol merged 1 commit into
mainfrom
fix-ci-vrt-writer-tempdir-windows-1938

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Summary

CI on main has been failing on every Windows job since #1942 merged. The post-merge test test_writer_returns_are_not_none in xrspatial/geotiff/tests/test_writer_return_path_1938.py used tempfile.TemporaryDirectory() for its scratch space. write_vrt reads each source through the module-level _MmapCache in _reader.py, which keeps the file handle and mmap of src.tif open after _FileSource.close() so repeated reads of the same file stay cheap. On Windows that cached handle blocks os.unlink (WinError 32), and the synchronous tempdir teardown raises before the test can return:

PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: '...\\tmpz1dsba1w\\src.tif'

Failing CI run for reference: https://github.com/xarray-contrib/xarray-spatial/actions/runs/25926750990

The fix swaps tempfile.TemporaryDirectory() for the tmp_path pytest fixture used by the other seven tests in the same file. tmp_path defers cleanup to pytest's session-end sweep, which tolerates the still-open handle. Library behaviour is unchanged; the mmap cache holding the handle past close() is by design (performance).

Test plan

  • pytest xrspatial/geotiff/tests/test_writer_return_path_1938.py passes locally (Linux)
  • CI passes on the windows-latest jobs that previously failed

The post-merge test added in #1938 used ``tempfile.TemporaryDirectory``
for its setup. ``write_vrt`` reads each source through the module-level
``_MmapCache`` in ``_reader.py`` (kept around so repeated reads of the
same file are cheap), so the file handle and mmap of ``src.tif`` stay
open past ``_FileSource.close()``. On Windows that cached handle blocks
``os.unlink`` with ``WinError 32`` and the synchronous tempdir teardown
fails before the test can return, breaking pytest on every Windows job.

Switch the test to the ``tmp_path`` fixture used by the other seven
tests in this file. ``tmp_path`` defers cleanup to pytest's session-end
sweep, which tolerates the still-open handle. The library behaviour is
intentional and unchanged.
Copilot AI review requested due to automatic review settings May 15, 2026 16:52
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 15, 2026
@brendancol brendancol merged commit 9ce0e60 into main May 15, 2026
12 of 13 checks passed
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

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

Labels

performance PR touches performance-sensitive code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants