fix: read wRTC dashboard fixtures as UTF-8#4876
Conversation
|
Welcome to RustChain! Thanks for your first pull request. Before we review, please make sure:
Bounty tiers: Micro (1-10 RTC) | Standard (20-50) | Major (75-100) | Critical (100-150) A maintainer will review your PR soon. Thanks for contributing! |
idan57570-art
left a comment
There was a problem hiding this comment.
Code looks clean. Approved for merge.
saim256
left a comment
There was a problem hiding this comment.
Reviewed current head b1af11a. The change is narrowly scoped to the dashboard test fixture reads: all three reads of UTF-8 dashboard files now pass encoding="utf-8", and the touched test file has the required SPDX header. This addresses the Windows default-codepage failure without changing dashboard runtime behavior.
Validation run locally on Windows:
python -m pytest tools\wrtc-bridge-dashboard\test_bridge_dashboard.py -q-> 27 passedpython -m py_compile tools\wrtc-bridge-dashboard\test_bridge_dashboard.py-> passedgit diff --check origin/main...pr-4876-> passedpython tools\bcos_spdx_check.py --base-ref origin/main-> OK
508704820
left a comment
There was a problem hiding this comment.
Code Review: Read wRTC Dashboard Fixtures as UTF-8
Summary
Adds explicit encoding="utf-8" to open() calls in the wRTC bridge dashboard tests. Fixes potential UnicodeDecodeError on Windows systems that default to cp1252.
What Works Well
- Explicit encoding: Prevents platform-dependent encoding issues
- SPDX header: Added MIT license identifier
- Low risk: Test-only change
Verdict: Approve
Good encoding practice. Python 3 best practice for cross-platform compatibility.
loganoe
left a comment
There was a problem hiding this comment.
Approved. The fixture reads now explicitly use UTF-8, which matches the dashboard files and avoids platform-default encoding failures without changing test behavior.
Validation performed:
git diff --check origin/main...HEADpython3 -m py_compile tools/wrtc-bridge-dashboard/test_bridge_dashboard.pyPYTEST_DISABLE_PLUGIN_AUTOLOAD=1 pytest -q tools/wrtc-bridge-dashboard/test_bridge_dashboard.py-> 27 passed
jaxint
left a comment
There was a problem hiding this comment.
Great contribution! LGTM.
jaxint
left a comment
There was a problem hiding this comment.
Great contribution! LGTM.
Summary
Root cause
test_bridge_dashboard.pyopened UTF-8 files without an encoding. On Windows, Python can default to cp1252, so the dashboard's Unicode title/arrows raiseUnicodeDecodeErrorbefore the assertions run.Validation
python -m pytest tools\wrtc-bridge-dashboard\test_bridge_dashboard.py -q-> 27 passedpython -m py_compile tools\wrtc-bridge-dashboard\test_bridge_dashboard.pygit diff --check -- tools\wrtc-bridge-dashboard\test_bridge_dashboard.pypython tools\bcos_spdx_check.py --base-ref origin/main