Skip to content

fix(tests): write CRLF CSV fixtures byte-exact on Windows - #175

Open
dieterolson wants to merge 1 commit into
jewbetcha:mainfrom
dieterolson:fix/tests-trackman-newline
Open

fix(tests): write CRLF CSV fixtures byte-exact on Windows#175
dieterolson wants to merge 1 commit into
jewbetcha:mainfrom
dieterolson:fix/tests-trackman-newline

Conversation

@dieterolson

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds newline="" to the two CRLF CSV fixture writes in tests/test_compare_trackman.py so the fixture bytes reach disk exactly as written on every platform.

Why was this required?

Path.write_text applies default newline translation ("\n"os.linesep), which turns the fixtures' embedded \r\n into \r\r\n on Windows. The BOM/sep= preamble test (test_handles_excel_sep_preamble_and_bom) then parses 0 shots instead of 2 and fails on every Windows checkout. These fixtures exist to reproduce what TrackMan actually exports, so they must be byte-exact; newline="" disables translation and is a no-op on POSIX.

Automated tests

Test-only change — the module is the coverage: tests/test_compare_trackman.py goes from 34 passed / 1 failed to 35/35 on Windows; unchanged on Linux/macOS.

Manual (human) testing

  • Reproduced the failure on Windows 11 before the change (assert 0 == 2 — zero shots parsed from the CRLF fixture).
  • After: full module passes on Windows; confirmed via the sibling _write_trackman_csv helper (which already used open(..., newline="")) that this matches the file's existing convention.

Checklist

  • Single feature/fix — this PR is scoped to one thing with a clear story above
  • Automated tests included — test-only change; the module is the coverage
  • Manual testing described — I documented what I verified by hand above
  • Python tests pass (uv run pytest tests/ -v)
  • Pylint passes (uv run pylint src/openflight/ --fail-under=9)
  • Ruff passes (uv run ruff check src/openflight/)
  • UI builds (cd ui && npm run build) — not applicable, no UI changes
  • UI lint passes (cd ui && npm run lint) — not applicable, no UI changes
  • Updated docs or CHANGELOG if needed — test-only, no changelog entry
  • No unrelated changes mixed in

Path.write_text default newline translation turns the embedded \r\n
in the TrackMan CSV fixtures into \r\r\n on Windows, so the BOM/sep
preamble test parsed 0 shots. newline="" preserves the fixture
content on every platform.
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