Repository navigation
test: decode Node output as UTF-8 in the portability check - #624
Merged
Merged
Conversation
Keep the portability job from decoding Node's UTF-8 reporter output using the locale's ASCII codec. Signed-off-by: Srinivasa Dinakar Jainti <dinakarjs@gmail.com>
dinakarjs
requested review from
a team,
lywinged and
rajnisht7
as code owners
October 23, 2026 16:19
Contributor
|
🟡 Contributor Check: MEDIUM
Automated check by AgenTrust Contributor Check. |
Contributor
Author
|
@rajnisht7 requesting independent review of a04343d. Its CI, Conformance CI, Review bar and CodeQL pass. The same inherited non-UTF-8 locale failure currently blocks both #577 and the refreshed docs-only #596 (CI run 40199855363). This remains the isolated repair; neither proposal incorporates an unrelated test workaround or relaxes portability. Review and maintainer integration are the remaining dependencies. |
2 of 4 tasks
rajnisht7
approved these changes
Oct 25, 2026
1 of 3 tasks
lywinged
approved these changes
Oct 27, 2026
lywinged
left a comment
Collaborator
There was a problem hiding this comment.
Approved. Reviewed at a04343d, merged here onto today's main da69b31 without conflict; tests/test_integer_by_value.py is unchanged on main since your base c5d49a5.
What I checked rather than read:
- The cause, from the sources rather than from the failure.
node --testuses thespecreporter by default from Node 23:kDefaultReporter = 'spec'inlib/internal/test_runner/utils.jsat v23.0.0 and at v24.0.0, where v22.22.0 hasprocess.stdout.isTTY ? 'spec' : 'tap'. In actions/runner-images the Ubuntu 26.04 image carries Node 24 and the 24.04 image Node 22; GitHub's notice on these runs saysubuntu-latestmigrates to Ubuntu 26 beginning 19 October, and the runner-images README calls that migration gradual, so a run can land on either. Piped, the spec reporter writes this file's results with nine✔and eightℹ; the TAP reporter writes none. - The failure and the fix under the portability step's own environment,
LC_ALL=C PYTHONUTF8=0, with Node 22 given--test-reporter=specthroughNODE_OPTIONShere to stand in for the newer default. Onmainthe root suite has one failure, this test,UnicodeDecodeErroron byte0xe2: 1 failed, 4784 passed and 49 skipped. With this change merged it passes, on 3.11 and on 3.12: 4785 passed and 49 skipped on both. Without the override both pass, which is what a run on the 24.04 image sees. - The line is the one that matters. With
encoding="utf-8"removed the error comes back; telling the child to use UTF-8 instead, withLC_ALL=C.UTF-8andPYTHONUTF8=1in its environment, does not help, because the decoding is the parent's. Thenode --versioncall above it needs nothing: it prints an ASCII version string such asv24.21.0. - This is the only
node --testthe suite runs. Three other node calls decode with the locale's codec the same way, intest_independent_verifier.py,test_schema_regex_semantics.pyandtest_verifier_token_crypto_interop.py; none uses the test runner, so the reporter change does not reach them, and they pass under the same conditions. - CI's other steps with the pinned install: ruff, the dash check and mypy clean on 3.11 and 3.12; the suite 4786 passed and 48 skipped on both; the experiments lane 116 passed and 4 skipped;
PYTHONOPTIMIZE=2ontests/test_review_bar.py52 passed; on 3.12, the conformance suite under the same locale 958 passed, 2 skipped and 5 xpassed.
Not run: Node 24 itself.
2 of 9 tasks
This was referenced Oct 28, 2026
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.
The non-UTF-8 portability job fails in
test_the_javascript_side_agreeswhen Node emits UTF-8 reporter symbols andsubprocess.run(text=True)uses the locale's ASCII codec. For example, #577 CI job 120516119668 completed the ordinary tests successfully, then failed withUnicodeDecodeErrorin this existing test. The same invocation remains on main at c5d49a5.Set
encoding="utf-8"on the Node test invocation so the portability check can consume Node's output while preserving the test's return-code assertion.Validation on Python 3.12 / Node 24.19.0: under
LC_ALL=C PYTHONUTF8=0, executing the exact function extracted from the pinned test file against all seven committed number-spelling vectors reproduces UnicodeDecodeError before this change and passes afterward. The function was isolated to avoid substituting an installed SDK for checkout imports; this is a focused invocation check, not a full pytest run. Ruff passes with the repository configuration. Hosted full-suite and portability checks remain required.One-line test-only change; no production API or schema change. DCO sign-off included. Prepared with OpenAI Codex.