fix(tests): read and write test files as UTF-8 so the suite runs on Windows - #80895
Closed
Adolanium wants to merge 1 commit into
Closed
fix(tests): read and write test files as UTF-8 so the suite runs on Windows#80895Adolanium wants to merge 1 commit into
Adolanium wants to merge 1 commit into
Conversation
…indows
`tests/hermes_cli/test_plugins_cmd.py::TestNoAutoActivation::test_compressor_default_ignores_plugin`
fails on every Windows machine:
UnicodeDecodeError: 'charmap' codec can't decode byte 0x8f in
position 47744: character maps to <undefined>
The test reads `run_agent.py` back as text to assert a removed comment is
gone, but called `open()` with no `encoding=`. Python then falls back to
the locale preferred encoding, which is cp1252 on a default Windows
install rather than UTF-8. `run_agent.py` contains nine bytes cp1252
leaves undefined, so the read raises before the assertion is reached. On
Linux and macOS the preferred encoding is UTF-8 and the same line is
fine, which is why CI never caught it.
That one line is the only active failure. The rest of this change closes
the same gap in the files it touches, which `scripts/check-windows-footguns.py`
flags and which the NousResearch#71014 read_text campaign has been working through
elsewhere in the tree:
- `tests/hermes_cli/test_plugins_cmd.py`: nine bare `write_text`/`read_text`
calls writing YAML manifests, config and plugin sources
- `tests/tools/test_web_tools_truncate.py`: reads stored extracted web text,
which is arbitrary content from the internet
- `tests/stress/test_atypical_scenarios.py`: writes and reads worker task
ids and a barrier file
All three files are now clean under `check-windows-footguns.py`.
Reads go through `Path.read_text(encoding="utf-8")` rather than
`open(...).read()`, which also closes the handle instead of leaving it to
the garbage collector. On Windows a live handle blocks tmpdir cleanup, so
that part is not cosmetic either.
No new test. The repaired test is the regression coverage: it fails
before this change and passes after, on Windows.
1 task
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
tests/hermes_cli/test_plugins_cmd.py::TestNoAutoActivation::test_compressor_default_ignores_pluginfails on every native Windows machine:The test reads
run_agent.pyback as text to assert a removed comment is gone, but callsopen()with noencoding=. Python falls back tolocale.getpreferredencoding(False), which is UTF-8 on Linux and macOS but cp1252 on a default US Windows install.run_agent.pyis UTF-8 and contains nine bytes cp1252 leaves undefined, so the decode raises before the assertion is ever reached. It passes everywhere else, which is why CI stayed green.That one line is the only active failure, and fixing it takes
test_plugins_cmd.pyfrom 38 passed with 1 failure to 39 passing on Windows.Why the rest of the diff.
scripts/check-windows-footguns.pyflags eleven more bare encoding sites across the same three files, and the #71014 read_text campaign has been closing this class elsewhere in the tree. Two of them matter beyond tidiness:tests/tools/test_web_tools_truncate.py:52reads stored extracted web text, which is arbitrary content from the internet and therefore one non-ASCII page away from the same crash, andtests/stress/test_atypical_scenarios.py:708is the write that pairs with the read at 739, so fixing only the read would leave an asymmetric round trip. All three files are clean undercheck-windows-footguns.pyafter this change.Reads go through
Path.read_text(encoding="utf-8")rather thanopen(...).read(), which also closes the handle instead of leaving it to the garbage collector. A live handle blocks tmpdir cleanup on Windows, so that part is not cosmetic either.Prior art: #37423 covered the
gateway/run.pyinstances of this and is closed, #71014 carried the campaign forward, and #79490 is the same fix applied totests/scripts/test_contributor_map.py. This is thetests/hermes_cli,tests/toolsandtests/stresscorner of it.Related Issue
Fixes #80894
Type of Change
Changes Made
tests/hermes_cli/test_plugins_cmd.py: the failing read now usesPath.read_text(encoding="utf-8"). Nine barewrite_textcalls writing YAML manifests, config and plugin sources get an explicitencoding="utf-8".tests/tools/test_web_tools_truncate.py: read the stored full text with an explicit encoding.tests/stress/test_atypical_scenarios.py: explicit encoding on the two barrier and result writes and the matching read.How to Test
On Windows, before this change:
After:
Full set touched by this PR:
The footgun check reports no findings on all three files after the change.
To see the underlying cause on any platform:
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passOn the tests checkbox, to be straight about it: no new test was added, because the thing being fixed is a test. It fails on Windows before this change and passes after, so it is its own regression coverage.
scripts/check-windows-footguns.pyis the mechanical guard against the pattern coming back, and it is clean on these files now.Documentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AScreenshots / Logs
Before: