fix(tests): pin UTF-8 encoding in test_auxiliary_config_bridge — salvage of #21473#22460
Merged
Conversation
Three tests in tests/agent/test_auxiliary_config_bridge.py read
in-tree source files (gateway/run.py and cli.py) via
Path.read_text() with no encoding argument. The default falls
back to the system locale, which on Western Windows installs is
cp1252, and the read fails as soon as the source contains any
byte that isn't valid cp1252 (e.g. an em-dash in a comment):
UnicodeDecodeError: 'charmap' codec can't decode byte 0x8f
in position 41190: character maps to <undefined>
Linux CI doesn't catch this because the default Linux locale is
UTF-8. Windows contributors hit it on every run of the test suite.
Pin encoding="utf-8" on the three call sites that read repo
source files. This matches the existing precedent in
hermes_cli/doctor.py:363, where the same pattern (with an
explanatory comment) was applied to fix the .env read on
non-UTF-8 Windows locales.
Affected tests now pass on Windows + Python 3.12:
- TestGatewayBridgeCodeParity.test_gateway_has_auxiliary_bridge
- TestGatewayBridgeCodeParity.test_gateway_no_compression_env_bridge
- TestCLIDefaultsHaveAuxiliaryKeys.test_cli_defaults_can_merge_auxiliary
Contributor
🔎 Lint report:
|
3 tasks
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
1 participant
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.
Summary
Salvage of #21473 by @KvnGz — pins
encoding="utf-8"on 3Path.read_text()calls intests/agent/test_auxiliary_config_bridge.pythat read in-tree source files (gateway/run.py,cli.py). On Western Windows installs the default locale is cp1252, so any non-ASCII byte in those source files (em-dash in a comment, etc.) crashes the read withUnicodeDecodeError: 'charmap' codec can't decode byte .... Linux CI doesn't catch this; Windows contributors hit it on every test run.Matches the existing precedent in
hermes_cli/doctor.py:363.Why a salvage
#21473 was 161 commits behind main. Although the diff applies cleanly with no stale-branch reverts (the PR is small and surgical), I ran the salvage path so the commit lands against fresh main rather than 161 commits stale, and so I could rerun the suite end-to-end. @KvnGz's commit is preserved with original authorship via plain
git cherry-pick(no--authorrewrite needed).Verification
scripts/run_tests.sh tests/agent/test_auxiliary_config_bridge.py— 19/19 pass on the salvage branchgit diffshows pure additions ofencoding="utf-8"and explanatory comments, no other editsgit diff origin/main..pr-21473 | git apply --checkreturned 0)Sibling-area note
I also audited every other
.read_text()intests/while reviewing — there are ~30 more withoutencoding=, but they all read test-controlled JSON / state files that won't contain non-ASCII bytes. The 3 calls in this PR are the only ones reading repo source files (which DO contain em-dashes). Author's scope is correctly minimal.Credit
Original work by @KvnGz in #21473. Their commit is preserved here with original authorship (
obafemiferanmi1999@gmail.com, mapped to KvnGz via #22458).Notes for reviewer
Depends on #22458 (
chore(release): add KvnGz to AUTHOR_MAP) being merged first.Merge with
--rebaseto preserve @KvnGz's authorship in the commit history.Closes #21473