Skip to content

grass.pygrass: Mark test_raster_region resampling tests xfail on Windows - #7905

Merged
echoix merged 1 commit into
OSGeo:mainfrom
echoix:claude/raster-region-windows-followup
Sep 8, 2026
Merged

echoix merged 1 commit into
OSGeo:mainfrom
echoix:claude/raster-region-windows-followup

Conversation

@echoix

@echoix echoix commented Sep 6, 2026

Copy link
Copy Markdown
Member

Description

Follow-up to #7902 (grass.pygrass: Close RasterRow via context manager in region tests, already merged to main). Fixing that resource leak turned the fatal, whole-file-crashing FERROR: Input window changed while maps are open for read on Windows CI into a distinct, real bug: the small region that test_resampling_1, test_resampling_2, and test_resampling_to_numpy each set up isn't actually restricting what RasterRow/raster2numpy read back. Windows CI showed, for example, rast[0].tolist() coming back with roughly 100000 elements instead of the expected 10, and a numpy _ArrayMemoryError trying to allocate an 8,150,000-element array where an 8-element one was expected.

I built GRASS locally (CMake) to check whether this is a genuine Windows-only build difference or something reproducible here:

  • test_raster_region.py run in isolation: passes (3/3).
  • The entire 359-file gunittest suite run in sequence (matching how CI runs it, in case an earlier test file leaves region-related state behind that only this ordering exposes): still passes, and isn't among the files that failed in that full run.

Since it doesn't reproduce here even under matching full-suite conditions, this looks like a genuine platform difference in the compiled GRASS libraries (Windows CI here builds with MinGW via OSGeo4W) rather than a Python-level state leak, and isn't something I can responsibly guess a fix for without Windows access. Marked @xfail_windows on the three affected test methods instead, per the project's existing convention for exactly this situation (see python/grass/gunittest/utils.py), with a comment describing the symptom and noting it needs Windows access to debug further.

Motivation and context

#7902 already turned the fatal crash into three individually visible failures. Without xfail, those would show as ordinary CI red without acknowledging they're a known, unreproduced, Windows-only issue distinct from the leak #7902 fixed.

How has this been tested?

Built GRASS locally and ran the test both in isolation and as part of the full local gunittest suite (see above); confirmed it still passes on Linux with @xfail_windows added (a no-op there).

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • PR title provides summary of the changes and starts with one of the
    pre-defined prefixes
  • My code follows the code style
    of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.

I used Claude Code to help investigate and draft this fix, under my review throughout. #7902 already fixed the crash that was masking this bug; this PR doesn't fix the bug itself, only marks it xfail on Windows.

Follow-up to OSGeo#7902: fixing the resource leak there turned a fatal,
whole-file-crashing FERROR into three individually diagnosable
failures instead. The remaining issue is real but distinct from the
leak: the small region these tests set up isn't actually restricting
what RasterRow reads back (rast[0] comes back with roughly 100000
elements instead of 10).

Built GRASS locally and confirmed this is genuinely Windows-specific,
not a state leak reproducible here: the file passes reliably on Linux
both run in isolation and as part of the full 359-file gunittest suite
in sequence. Marking xfail on Windows per the existing convention
rather than guessing at a fix that can't be verified; needs
investigation with actual Windows access.
@github-actions github-actions Bot added Python Related code is in Python libraries tests Related to Test Suite labels Sep 6, 2026
@echoix
echoix requested a review from ninsbl September 6, 2026 15:18
@echoix
echoix enabled auto-merge (squash) September 6, 2026 15:19
@echoix

echoix commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

@ninsbl Can you take a quick look?

@echoix
echoix merged commit b585c80 into OSGeo:main Sep 8, 2026
26 checks passed
@github-actions github-actions Bot added this to the 8.6.0 milestone Sep 8, 2026
@echoix
echoix deleted the claude/raster-region-windows-followup branch September 12, 2026 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libraries Python Related code is in Python tests Related to Test Suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants