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 thereFreeSurferColorLUT.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.
Problem
mne/tests/test_line_endings.pyskips itself unconditionally on Windows:The CI matrix currently runs 8 combinations, all
ubuntu-*ormacos-*— there is noWindows 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 Git for Windows' default
core.autocrlf=true, a checkout turns every text file intoCRLF, so
crcountwould be non-zero for the entire repository and the test would fail forreasons 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=lffor every extension ingood_exts)would make the working tree LF and let the test run — but it would also rewrite the files that
are deliberately CRLF.
skip_fileslists several of them:test_old_layout_latin1_software_filter.vhdr/.vmrk/_longname.vhdr— the comment inthe 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.datForcing those to LF would change the very bytes those compatibility tests depend on. So
.gitattributesis 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(thei/field is the index/worktree-independent line ending), or byreading blobs through
git cat-file. That is unaffected bycore.autocrlf, so the checkwould give the same answer on every platform and the
win32skip 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.