Skip to content

Nifty CI table - #5104

Open
ptomato wants to merge 8 commits into
tc39:mainfrom
ptomato:4118-nifty-ci-table
Open

Nifty CI table#5104
ptomato wants to merge 8 commits into
tc39:mainfrom
ptomato:4118-nifty-ci-table

Conversation

@ptomato

@ptomato ptomato commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

This removes the old CircleCI config (which is often broken) and replaces it with a GitHub Action that runs new and changed tests on each available engine, and posts a comment with a nifty Markdown table showing which tests passed on which engine, and which failed and why.

Example of what the result looks like (on a scratch repo, so this link will disappear at some point after this PR is merged): ptomato/test262temp#1 (comment)

LLM disclosure: I used a bot to make a first draft of this, and refine that draft several times. I then made further modifications without using a bot until I was satisfied with the result and that I understood it. I also removed long-winded LLM comments and replaced them with my own words if I felt a comment was necessary. Do note that I'm not super familiar with GH Actions config so if I missed anything important it's likely to be in engine-tests.yml or engine-tests-report.yml.

Closes: #4118

Replaces the CircleCI definitions with GitHub Actions definitions, and
uses the output of the jobs to create a persistent PR comment showing
what the results were of any new or changed tests in each engine.

A few cleanups while we're here:

- Removed the special-case LANG=C environment variable from JSC runs.
  It looks like this was added years ago to address a temporary failure.

- Combine the separate V8 and V8-harmony jobs. Just pass --harmony in
  the regular V8 job, as we do with --features=all in the Engine262 job.
  Use the equivalent CLI option for JSC. (Currently, such an option does
  not exist for SpiderMonkey, otherwise I'd pass it there as well.)

LLM disclosure: Drafted and refined using a bot, reviewed by me.
- Hermes is skipped because it currently fails to install with esvu.
- Boa is skipped because test262-harness needs to publish a new version
  with updated eshost.
- Kiesel, LibJS, etc. are skipped because esvu cannot install them.
@ptomato
ptomato requested a review from a team as a code owner August 10, 2026 21:37
Comment thread tools/misc/build-results-comment.mjs Outdated
Comment thread tools/misc/build-results-comment.mjs Outdated
Comment thread tools/misc/build-results-comment.mjs
Comment thread .github/workflows/engine-tests.yml
@nicolo-ribaudo

Copy link
Copy Markdown
Member

If the table is long, could it be inside a <details> element? I'm thinking of PR that change 100 tests, for which the table would get a bit in the way.

Instead of logging a warning in the CI log, print a list of encountered
errors in the Markdown comment.
Check up front if all the keys we need are there, and add an error to
the error list if not. If the keys are there, assume they are the right
type.

We actually don't need the `file` key. There is no reason to use it as a
backup for the `relative` key; we should consider the entry broken if
there is no `relative` key.
This would return an object with an `engines` key if returning early,
and an `orderedEngines` key if returning at the end of the function. Fix
this so the key is always named `engines`.
Avoid long comment that you have to scroll past.
Pipes will break the table, despite being delimited by both () and "".
Left angle brackets don't need to be escaped in link titles, but they
could present a security risk in case someone manages to break the
formatting in some other way and cause them to render as an HTML
element.
@ptomato

ptomato commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

If the table is long, could it be inside a <details> element? I'm thinking of PR that change 100 tests, for which the table would get a bit in the way.

I've made a cutoff of 15 tests, more than that and the table will be collapsed inside <details>.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Automatically add a results table for CI runs on pull requests

3 participants