Skip to content

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

Closed
echoix wants to merge 1 commit into
mainfrom
claude/raster-region-windows-followup
Closed

echoix wants to merge 1 commit into
mainfrom
claude/raster-region-windows-followup

Conversation

@echoix

@echoix echoix commented Sep 5, 2026 •

Copy link
Copy Markdown
Owner

Description

Follow-up to OSGeo#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

OSGeo#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 OSGeo#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. OSGeo#7902 already fixed the crash that was masking this bug; this PR doesn't fix the bug itself, only marks it xfail on Windows using the project's own existing mechanism for an unreproduced platform-specific failure, rather than claiming a fix I can't verify.

@echoix
echoix force-pushed the claude/raster2numpy-region-timing-fix branch from 80fc75c to c4916ec Compare September 5, 2026 02:59
@echoix
echoix force-pushed the claude/raster-region-windows-followup branch from de713f3 to 800a40f Compare September 5, 2026 02:59
@echoix echoix changed the title pygrass: Mark test_raster_region resampling tests xfail on Windows grass.pygrass: Mark test_raster_region resampling tests xfail on Windows Sep 5, 2026
@echoix
echoix marked this pull request as ready for review September 5, 2026 05:04
@echoix
echoix force-pushed the claude/raster2numpy-region-timing-fix branch from c4916ec to 4f13a2d Compare September 5, 2026 21:07
@echoix
echoix force-pushed the claude/raster-region-windows-followup branch from 800a40f to 6c37419 Compare September 5, 2026 21:07
@echoix
echoix force-pushed the claude/raster2numpy-region-timing-fix branch 2 times, most recently from fee78ab to 0a385ac Compare September 5, 2026 21:09
@echoix
echoix force-pushed the claude/raster-region-windows-followup branch 2 times, most recently from 12843ef to 0c30f10 Compare September 5, 2026 21:12
@echoix
echoix force-pushed the claude/raster2numpy-region-timing-fix branch from 0a385ac to 4a7dc34 Compare September 6, 2026 11:04
@echoix
echoix force-pushed the claude/raster-region-windows-followup branch from 0c30f10 to a8b514d Compare September 6, 2026 11:04
@echoix
echoix deleted the branch main September 6, 2026 11:17
@echoix echoix closed this Sep 6, 2026
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.
@echoix echoix reopened this Sep 6, 2026
@echoix
echoix changed the base branch from claude/raster2numpy-region-timing-fix to main September 6, 2026 11:22
@echoix
echoix force-pushed the claude/raster-region-windows-followup branch from a8b514d to add9cd3 Compare September 6, 2026 11:22
@echoix echoix closed this Sep 12, 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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant