Skip to content

test: decode Node output as UTF-8 in the portability check - #624

Merged
lywinged merged 1 commit into
agentrust-io:mainfrom
dinakarjs:fix/node-output-utf8
Oct 27, 2026
Merged

lywinged merged 1 commit into
agentrust-io:mainfrom
dinakarjs:fix/node-output-utf8

Conversation

@dinakarjs

Copy link
Copy Markdown
Contributor

The non-UTF-8 portability job fails in test_the_javascript_side_agrees when Node emits UTF-8 reporter symbols and subprocess.run(text=True) uses the locale's ASCII codec. For example, #577 CI job 120516119668 completed the ordinary tests successfully, then failed with UnicodeDecodeError in 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.

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>
@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

🟡 Contributor Check: MEDIUM

Check Result
Profile MEDIUM
Credential LOW
Overall MEDIUM

Automated check by AgenTrust Contributor Check.

@github-actions github-actions Bot added the needs-review:MEDIUM Contributor check flagged MEDIUM risk label Oct 23, 2026

Copy link
Copy Markdown
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.

@lywinged lywinged left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 --test uses the spec reporter by default from Node 23: kDefaultReporter = 'spec' in lib/internal/test_runner/utils.js at v23.0.0 and at v24.0.0, where v22.22.0 has process.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 says ubuntu-latest migrates 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=spec through NODE_OPTIONS here to stand in for the newer default. On main the root suite has one failure, this test, UnicodeDecodeError on byte 0xe2: 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, with LC_ALL=C.UTF-8 and PYTHONUTF8=1 in its environment, does not help, because the decoding is the parent's. The node --version call above it needs nothing: it prints an ASCII version string such as v24.21.0.
  • This is the only node --test the suite runs. Three other node calls decode with the locale's codec the same way, in test_independent_verifier.py, test_schema_regex_semantics.py and test_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=2 on tests/test_review_bar.py 52 passed; on 3.12, the conformance suite under the same locale 958 passed, 2 skipped and 5 xpassed.

Not run: Node 24 itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-review:MEDIUM Contributor check flagged MEDIUM risk

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants