[awesomekling]

Report the command, return code and captured output when a JSON-producing test runner fails to produce valid JSON. The updater currently discards the child status and only reports a JSONDecodeError, which hides the information needed to diagnose the failure.

Capture stdout and stderr, report the exit status and output lengths before parsing, and include both streams in the error if JSON decoding fails. Valid JSON keeps its existing meaning, including when the runner returns 1 for failing tests.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 47 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: c04a64be-51dd-41c1-9625-a066b4ff37ee

Commits

Reviewing files that changed from the base of the PR and between 1298a23 and 3e3bf09.

Files selected for processing: 1

• run_all_and_update_results.py

Walkthrough

The change adds run_json_command to capture command output, report stderr and output lengths, and parse stdout as JSON. Parser-test and Test262 execution now use this helper. New unit tests cover valid JSON, nonzero child exit status, empty or malformed stdout, and diagnostic details in parsing errors.

Priority: Pending latest changes

Merge Risk: Low · previous review 1298a

Undecodable runner output can hide diagnostics or reject valid results. The change is otherwise mergeable with this narrow issue tracked.

Pre-merge checks: 3

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		The description directly explains the changes to capture command output, report exit status, improve JSON parsing diagnostics, and preserve valid JSON results from failing test runners.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

---

Review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 253e4f14-9675-45b1-b0db-280cdc844162

Commits

Reviewing files that changed from the base of the PR and between 0090e44 and 1298a23.

Files selected for processing: 2

• run_all_and_update_results.py
• test_run_all_and_update_results.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

[coderabbitai]

Minor · Stability & Availability · Quick win

Supported by static analysis

Script executed:

sed -n '1,75p' run_all_and_update_results.py
sed -n '1,90p' test_run_all_and_update_results.py

Repository: LadybirdBrowser/libjs-test262

Length of output: 5136

---

Decode captured output without changing stdout.

subprocess.run(..., text=True) decodes stderr strictly. If the child writes bytes that the locale encoding cannot decode, run raises UnicodeDecodeError before run_json_command logs the exit status or output. The except clause catches only json.JSONDecodeError, so the error escapes. Valid stdout JSON is then rejected. Before this change, run_command did not capture stderr, so this failure is new.

Setting errors="replace" on both streams also changes stdout before json.loads. Replacement characters inside JSON strings parse without error, so corrupted test data can pass silently. Capture both streams as bytes instead. Decode stderr with replacement. Decode stdout strictly, and report a decode failure with the same diagnostics. Decode each stream once, and use the decoded values in the existing messages so their text and lengths stay unchanged.

Suggested fix

 import json
+import locale
 import shlex
 import subprocess
 import sys
@@
+def decode_output(data: bytes, encoding: str, errors: str = "strict") -> str:
+    return data.decode(encoding, errors).replace("\r\n", "\n").replace("\r", "\n")
+
+
 def run_json_command(command: str, **kwargs):
-    process = subprocess.run(shlex.split(command), capture_output=True, check=False, text=True, **kwargs)
-    if process.stderr:
-        sys.stderr.write(process.stderr)
+    encoding = locale.getpreferredencoding(False)
+    process = subprocess.run(shlex.split(command), capture_output=True, check=False, **kwargs)
+    stderr = decode_output(process.stderr, encoding, errors="replace")
+    if stderr:
+        sys.stderr.write(stderr)
+    try:
+        stdout = decode_output(process.stdout, encoding)
+    except UnicodeDecodeError as error:
+        raise RuntimeError(
+            f"Command did not produce decodable stdout: {command!r}\n"
+            f"Return code: {process.returncode}\n"
+            f"stdout ({len(process.stdout)} bytes):\n{process.stdout!r}\n"
+            f"stderr ({len(stderr)} characters):\n{stderr}"
+        ) from error
     print(
         f"Command {command!r} exited with {process.returncode}; "
-        f"stdout: {len(process.stdout)} characters; stderr: {len(process.stderr)} characters",
+        f"stdout: {len(stdout)} characters; stderr: {len(stderr)} characters",
         file=sys.stderr,
         flush=True,
     )
     try:
-        return json.loads(process.stdout.strip())
+        return json.loads(stdout.strip())
     except json.JSONDecodeError as error:
         raise RuntimeError(
             f"Command did not produce valid JSON: {command!r}\n"
             f"Return code: {process.returncode}\n"
-            f"stdout ({len(process.stdout)} characters):\n{process.stdout}\n"
-            f"stderr ({len(process.stderr)} characters):\n{process.stderr}"
+            f"stdout ({len(stdout)} characters):\n{stdout}\n"
+            f"stderr ({len(stderr)} characters):\n{stderr}"
         ) from error

Tools

ast-grep (0.50.0)

[error] 24-24: Use of unsanitized data to create processes
Context: subprocess.run(shlex.split(command), capture_output=True, check=False, text=True, **kwargs)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(os-system-unsanitized-data)

---

[error] 24-24: Command coming from incoming request
Context: subprocess.run(shlex.split(command), capture_output=True, check=False, text=True, **kwargs)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(subprocess-from-request)