Run JSC stress tests on PRs#243
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR adds a new ChangesJSC Tests Workflow Job
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/jsc-tests.yml:
- Line 146: The summary string written in the workflow currently says "### JSC
tests (quick)\n\n" but the tests run in full mode; update the out.write call
that writes the summary (the line containing out.write(f"### JSC tests
(quick)\n\n")) to remove "(quick)" so it reads "### JSC tests\n\n" to accurately
reflect the test mode in the workflow.
- Around line 134-152: Wrap the json.load(...) call in a try/except that catches
json.JSONDecodeError (and optionally Exception) to handle malformed
jsc_results.json: on exception, print an error to stderr including the filename
and error message, set r = {} (or stress = []), and proceed to write a summary
message to "${{ runner.temp }}/summary.md" indicating that parsing failed so the
workflow doesn't emit a Python traceback; update references to r and stress in
the block so they are safe when parsing fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 178a0c47-ddb3-4794-8f6e-301e12acbfd6
📒 Files selected for processing (1)
.github/workflows/jsc-tests.yml
| python3 - <<'EOF' | ||
| import json, sys | ||
| with open("jsc_results.json") as f: | ||
| r = json.load(f) | ||
| stress = r.get("stressTestFailures", []) | ||
| print(f"## JSC test summary", file=sys.stderr) | ||
| print(f"stress failures: {len(stress)}") | ||
| for t in stress[:50]: | ||
| print(f" FAIL {t}") | ||
| if len(stress) > 50: | ||
| print(f" ... and {len(stress)-50} more") | ||
| with open("${{ runner.temp }}/summary.md", "w") as out: | ||
| out.write(f"### JSC tests (quick)\n\n") | ||
| out.write(f"- stress failures: **{len(stress)}**\n") | ||
| if stress: | ||
| out.write("\n<details><summary>failures</summary>\n\n```\n") | ||
| out.write("\n".join(stress[:200])) | ||
| out.write("\n```\n</details>\n") | ||
| EOF |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add error handling for JSON parsing failures.
The Python script does not handle the case where jsc_results.json exists but contains malformed JSON. Although this is an edge case, wrapping the JSON load in a try/except block would make the summary step more robust and prevent the workflow from showing a Python traceback on parse errors.
🛡️ Proposed fix to add JSON error handling
python3 - <<'EOF'
import json, sys
-with open("jsc_results.json") as f:
- r = json.load(f)
-stress = r.get("stressTestFailures", [])
+try:
+ with open("jsc_results.json") as f:
+ r = json.load(f)
+ stress = r.get("stressTestFailures", [])
+except (json.JSONDecodeError, KeyError, IOError) as e:
+ print(f"## JSC test summary", file=sys.stderr)
+ print(f"Failed to parse results: {e}", file=sys.stderr)
+ sys.exit(0)
print(f"## JSC test summary", file=sys.stderr)
print(f"stress failures: {len(stress)}")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/jsc-tests.yml around lines 134 - 152, Wrap the
json.load(...) call in a try/except that catches json.JSONDecodeError (and
optionally Exception) to handle malformed jsc_results.json: on exception, print
an error to stderr including the filename and error message, set r = {} (or
stress = []), and proceed to write a summary message to "${{ runner.temp
}}/summary.md" indicating that parsing failed so the workflow doesn't emit a
Python traceback; update references to r and stress in the block so they are
safe when parsing fails.
| if len(stress) > 50: | ||
| print(f" ... and {len(stress)-50} more") | ||
| with open("${{ runner.temp }}/summary.md", "w") as out: | ||
| out.write(f"### JSC tests (quick)\n\n") |
There was a problem hiding this comment.
Remove "(quick)" from summary title—tests run in full mode.
Line 146 outputs "### JSC tests (quick)\n" but the test command (line 113) does not include the --quick flag, consistent with the file header comment (line 7: "full mode set"). The summary title should reflect that the full test suite is running.
📝 Proposed fix to align summary with actual test mode
- out.write(f"### JSC tests (quick)\n\n")
+ out.write(f"### JSC tests\n\n")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| out.write(f"### JSC tests (quick)\n\n") | |
| out.write(f"### JSC tests\n\n") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/jsc-tests.yml at line 146, The summary string written in
the workflow currently says "### JSC tests (quick)\n\n" but the tests run in
full mode; update the out.write call that writes the summary (the line
containing out.write(f"### JSC tests (quick)\n\n")) to remove "(quick)" so it
reads "### JSC tests\n\n" to accurately reflect the test mode in the workflow.
1 participant
Adds a
JSC Testsworkflow that runs the upstreamrun-javascriptcore-testsdriver against a releasejscshell on each PR. The existing Preview Build only verifies the tree compiles; this is the first correctness gate.What it runs
PORT=JSCOnlyreleasejsconlinux-x64-gh, clang-21, system ICU,-march=haswell(same flags as the Docker artifact build minus the custom ICU repack)Tools/Scripts/run-javascriptcore-tests --quick --memory-limited, stress + mozilla + ChakraCore + jsc-layout-tests; native test binaries (testmasm/testair/testb3/testdfg/testapi/testwasmdebugger) skipped soDEVELOPER_MODE=ONisn't neededjsc_results.jsonartifact + a step summary listing failuresWhy
--quickUpstream EWS budgets ~1h for the full release run.
--quickkeepsdefault+no-cjit-validatemodes only for the stress corpus. We can drop--quickonce we know the runner timing.Verified locally
The Perl→Ruby driver chain runs end-to-end against a locally-built
jsc(--root= cmake build dir;executableProductDir()appends/binon non-Apple). JSON output schema matches what the summary step consumes.Open questions
USE_BUN_JSC_ADDITIONS. First green run will establish the expectations file.