Skip to content

windows: Fix two Windows-only gunittest failures - #7895

Open
echoix wants to merge 4 commits into
OSGeo:mainfrom
echoix:claude/windows-test-failures-vh9fvw
Open

windows: Fix two Windows-only gunittest failures#7895
echoix wants to merge 4 commits into
OSGeo:mainfrom
echoix:claude/windows-test-failures-vh9fvw

Conversation

@echoix

@echoix echoix commented Sep 5, 2026

Copy link
Copy Markdown
Member

I'm preparing to enable running Windows tests in random order, and have launched 30+15 runs with that. But, some test fixes were made along the way. This PR is one of those such fixes, two small and simple fixes.

Description

Fixes two of the tests failing in the Windows OSGeo4W CI run (found by
downloading and inspecting the testreport-windows-2022 artifact from a
recent main run):

  • scripts/db.univar/testsuite/test_db_univar.py::test_calculate_extended
    db.univar.py's sortfile() in-memory fallback (used when the sort
    command is unavailable, e.g. on Windows) converted every line to float
    without skipping empty ones, crashing on a trailing blank line with
    ValueError: could not convert string to float: ''. The main statistics
    loop a few lines above already skips empty lines; sortfile() now does
    the same.
  • lib/init/testsuite/test_grass_tmp_mapset.py (all 4 tests, failing in
    setUp) — GISBASE itself (where the grass.bat launcher lives in this
    CI's from-source build) is never added to PATH on Windows, only
    GISBASE\bin is, so the bare "grass.bat" name raised
    FileNotFoundError. There was already a TODO in the code acknowledging
    this. The executable is now resolved through GISBASE when it is set.

Motivation and context

These are 2 of 8 gunittest files failing in the Windows CI run; the other 6
are environment-specific issues (missing OpenGL on the headless runner, a
numpy repr version difference in a doctest, an access-violation crash, a
timing issue, an md5sum mismatch in a shell test, and an external addon
server returning 404) that aren't simple, isolated code fixes.

How has this been tested?

  • The db.univar.py fix was verified directly: reproduced the exact crash
    (ValueError: could not convert string to float: '') against a file with
    a trailing blank CRLF line using the old code, then confirmed the fixed
    version sorts the same input correctly.
  • The test_grass_tmp_mapset.py fix was verified end-to-end on a scratch
    CMake build of GRASS on Linux, with os.name patched to "nt" so the
    Windows branch actually executes: reproduced the original
    FileNotFoundError with the pre-fix code inside a live GRASS session,
    then confirmed the fixed code resolves the launcher through GISBASE and
    all 4 tests pass.
  • Additionally ran a one-time manual Windows CI dispatch that installs the
    grass-dev OSGeo4W nightly package instead of compiling from source (with
    db.univar.py patched post-install, since the nightly package predates
    this fix), to see how these fixes hold up against a real packaged
    install rather than this CI's from-source build. Confirmed the
    db.univar fix passes there too, but test_grass_tmp_mapset.py fails
    differently: a real package puts the launcher at OSGEO4W_ROOT\bin, not
    inside GISBASE, so the GISBASE-based resolution added here doesn't
    generalize to that layout. This matches how this CI job actually builds
    GRASS (from source, not from a package), so the fix as written is correct
    for it, but it's worth flagging as a known limitation if a package-based
    install is ever used instead.

Screenshots (if appropriate)

N/A

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing
    functionality to not work as before)

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.

Large use of AI (Claude Code) was used to investigate the CI failures,
diagnose root causes, and write and verify these fixes.

sortfile()'s in-memory fallback (used when the 'sort' command is
unavailable, e.g. on Windows) converted every line to float without
skipping empty ones, crashing on a trailing blank line with
ValueError: could not convert string to float: ''. The main
statistics loop already skips empty lines; apply the same handling
here.

Found and fixed with the help of Claude Code, using a Windows CI
test report to diagnose the failure.
GISBASE itself (where the grass.bat launcher lives) is never added to
PATH on Windows, only GISBASE\bin is, so the bare "grass.bat" name
failed with FileNotFoundError when this test ran inside an existing
GRASS session (as it does under grass.gunittest.main in CI). Resolve
the launcher through GISBASE when it is set, keeping the bare name as
a fallback otherwise.

Root-caused with the help of Claude Code, by tracing the Windows
OSGeo4W CI build scripts to see where the launcher batch file ends up
and what gets added to PATH, and verified by reproducing and fixing
the failure in a scratch Linux build with os.name patched to "nt".
@echoix
echoix requested a review from nilason September 5, 2026 20:17
Restores the known-limitation note dropped when the old TODO comments
were replaced: the GISBASE-based executable resolution assumes a
from-source build's grass.bat, but an OSGeo4W package install names
the launcher with a version suffix (e.g. grass86.bat), so it isn't
covered here.
@echoix
echoix force-pushed the claude/windows-test-failures-vh9fvw branch from 8f81fe8 to e8d02d3 Compare September 5, 2026 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant