fix(encoding): explicitly use UTF-8 for all file I/O (#630)#798
Conversation
All open(), read_text(), write_text(), and aiofiles.open() calls in production code now pass encoding="utf-8", removing reliance on the system locale (which can be cp1252 on Windows or ASCII in minimal containers). Also adds encoding="utf-8" to the subprocess.run(text=True) call in the e2e test harness, which caused UnicodeDecodeError when capturing nac-test's emoji-containing stdout under a non-UTF-8 locale. E2e fixtures now contain multi-byte UTF-8 characters (in comments and docstrings only, not data values) to act as regression guards for the affected file I/O paths.
…CLI entry (#630) Add encoding='utf-8' to all NamedTemporaryFile and open() calls that were missed in the initial fix: subprocess_auth.py, subprocess_client.py, device_executor.py, subprocess_runner.py, orchestrator.py. Also fix generated Python scripts inside f-strings (subprocess_auth.py, subprocess_client.py) — these are written to temp files and executed via os.system(), so their open() calls need encoding too. Set os.environ.setdefault('PYTHONUTF8', '1') at CLI entry point (nac_test/cli/main.py) to propagate UTF-8 mode to all child processes. Add targeted Unicode regression test for combined report generation.
…os (#630) Revert decorative UTF-8 comments from 9 fixture scenarios (24 files) that only had UTF-8 in YAML comments and Python comments — content that never flows through the report pipeline. Keep and enhance success + mixed fixtures with meaningful UTF-8: - data.yaml: UTF-8 site names (SITE_München_100, SITE_日本_100) that flow through YAML → Jinja2 → test names → HTML report - pyATS TITLE/DESCRIPTION constants with German and Japanese text that render directly in the combined HTML report - Robot Documentation line with non-ASCII characters
oboehmer
force-pushed
the
fix/630-read-write-utf8
branch
from
c9ec5c2 to
872bb23
Compare
oboehmer
force-pushed
the
fix/630-read-write-utf8
branch
from
872bb23 to
b9bbf8f
Compare
|
@aitestino , do you have an opinion if this is needed? technically it is not, current nac-test handles this fine on Linux (where utf-8 is default encoding) as well as Windows. Certain windows terminals will require utf-8 encoding env to display the emojis (nothing we can do here).. please review, but we can also close it.. |
aitestino
left a comment
aitestino
left a comment
There was a problem hiding this comment.
The defensive coding is reasonable for Docker LANG=C edge cases and follows PEP 597 best practices. The only thing I'd actually want fixed before merge is html_helpers.py:292 — if you're fixing encoding everywhere, the test harness that reads those files should match.
P.S. — This comment was drafted using voice-to-text via Claude Code. If the tone comes across as overly direct or terse, please know that's just how it tends to phrase things. No offense or criticism is intended — this is purely an objective technical review of the PR. Thanks for understanding! 🙂
|
thanks for the feedback.. addressed the missing piece and now merge it.. |
2 participants
Description
Fix
UnicodeEncodeErroron Windows when generating HTML reports containing Unicode characters (↕, ✓, →).Python uses the system locale for
open()by default —cp1252on Windows,ASCIIin minimal containers — which fails on the Unicode sort indicators and navigation arrows in the HTML report templates.Closes
Related Issue(s)
PYTHONIOENCODINGCI workaround is now removed as redundant)Type of Change
Test Framework Affected
Network as Code (NaC) Architecture Affected
Platform Tested
Key Changes
encoding="utf-8"to allopen(),read_text(),write_text(), andNamedTemporaryFile(mode="w")calls across 16 production filessubprocess_auth.py,subprocess_client.py) written to temp files and executed viaos.system()os.environ.setdefault("PYTHONUTF8", "1")at CLI entry point to propagate UTF-8 mode to child processesPYTHONIOENCODINGworkaround from CI workflow (UnicodeEncodeError on Windows when stdout uses cp1252 encoding #723)success+mixedscenarios with meaningful UTF-8 in data fields that flow through the full YAML → Jinja2 → report pipelineTesting Done
pytest/pre-commit run -a)Test Commands Used
uv run pytest tests/unit/ tests/e2e/ -x -q -n auto --dist loadscope # 1768 passed, 377 skippedChecklist
pre-commit run -apasses)Additional Notes
PYTHONUTF8=1a transitional measureencoding="utf-8"on every I/O call is the primary fix;PYTHONUTF8=1is belt-and-suspenders for child processes spawned viaos.system()andsubprocess