mne-tools / mne-tools/mne-python

MAINT: line ending test cannot run on Windows (check git blobs instead of the working tree?)

Open
#14,299 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
3.5k
Forks
1.6k
Avg merge
1d 6h
Merged PRs (30d)
100

Description

Problem

mne/tests/test_line_endings.py skips itself unconditionally on Windows:

def _assert_line_endings(dir_):
    """Check line endings for a directory."""
    if sys.platform == "win32":
        pytest.skip("Skipping line endings check on Windows")

The CI matrix currently runs 8 combinations, all ubuntu-* or macos-* — there is no
Windows runner — so between the two, this check never executes on Windows at all.

It has to skip, because the check reads the working-tree bytes:

with open(filename, "rb") as fid:
    text = fid.read().decode("utf-8")
...
crcount = text.count("\r")

With Git for Windows' default core.autocrlf=true, a checkout turns every text file into
CRLF, so crcount would be non-zero for the entire repository and the test would fail for
reasons that have nothing to do with what is committed.

The practical consequence is that a Windows contributor cannot run this check locally; a CRLF
file only gets caught once Linux CI sees it.

A fix that won't work (writing it down so nobody tries it)

Adding a broad .gitattributes (e.g. *.py text eol=lf for every extension in good_exts)
would make the working tree LF and let the test run — but it would also rewrite the files that
are deliberately CRLF. skip_files lists several of them:

  • test_old_layout_latin1_software_filter.vhdr / .vmrk / _longname.vhdr — the comment in
    the test says part of testing compatibility with older BrainVision formats is testing the
    line endings and coding schemes used there
  • FreeSurferColorLUT.txt, test_edf_stim_channel.txt, FieldTrip.py, license.txt,
    searchindex.dat

Forcing those to LF would change the very bytes those compatibility tests depend on. So
.gitattributes is not a safe way to make this test runnable on Windows.

Suggestion

Check the committed bytes rather than the checked-out ones, e.g. via
git ls-files --eol (the i/ field is the index/worktree-independent line ending), or by
reading blobs through git cat-file. That is unaffected by core.autocrlf, so the check
would give the same answer on every platform and the win32 skip could go away.

This is a change to how the test works rather than a bug fix, so I'd rather check the
direction first. Happy to implement it if you think it's worth doing; also completely fine if
you'd rather keep the current behaviour and leave the check to Linux CI.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in mne/tests/test_line_endings.py, especially _assert_line_endings and its skip_files list, and review how the current test reads working-tree bytes. Check the repository's existing CI and Git behavior, then verify the test can inspect committed bytes consistently on Windows and other platforms without changing deliberately CRLF files. Done means the Windows skip is no longer needed and the line-ending check still covers the intended files.

Written by the indexing model from the issue text.

Assessment

Tech stack
git, python
Domain
testing-qa
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.