Refactor test and release CI#4290

Merged

npaun merged 3 commits intomainfrom

npaun/test-action-refac

Jun 12, 2025

Member

npaun commented Jun 9, 2025 •

edited

Loading

This PR is my attempt at organizing workerd's CI. Let me know what you guys think!

What we get:

Re-use the output of the linux x64 build as much as possible (test-linux)
- Dan already uses the binary for workers-sdk-tests
- Save Bazel's XML test logs as an artifact. We can make use of this later.
- Use test logs to produce a WPT report for each build
- Check snapshot effectively depends on the entire workerd build, so I'm now uploading generated-snapshot from test-linux to avoid invoking Bazel again.
Combine code between _bazel.yml and release.yml. They do some slightly different things but I tried my best to union them.
- I was able to verify that release.yml works fine, through some annoying GH actions tricks.

Unfortunately, we did lose something:

If a test is cached by Bazel, we won't get a test.xml and our uploaded artifact will be junk. So unfortunately I had to turn off test caching for test-linux which will slow down builds.

npaun force-pushed the npaun/test-action-refac branch from 882f98a to 3e454f9 Compare

June 9, 2025 20:14

github-advanced-security bot found potential problems

View reviewed changes

.github/workflows/test.yml Fixed Show fixed Hide fixed

npaun force-pushed the npaun/test-action-refac branch 3 times, most recently from 11c4e4f to 57be0a9 Compare

June 9, 2025 20:40

github-advanced-security bot found potential problems

View reviewed changes

.github/workflows/_wpt.yml Fixed Show fixed Hide fixed

.github/workflows/test.yml Fixed Show fixed Hide fixed

npaun force-pushed the npaun/test-action-refac branch 3 times, most recently from f70da00 to 5004192 Compare

June 9, 2025 20:50

github-advanced-security bot found potential problems

View reviewed changes

.github/workflows/test.yml Fixed Show fixed Hide fixed

npaun force-pushed the npaun/test-action-refac branch 3 times, most recently from 21353e7 to cb23506 Compare

June 9, 2025 21:12

github-advanced-security bot found potential problems

View reviewed changes

.github/workflows/_wpt.yml Fixed Show fixed Hide fixed

github-advanced-security bot found potential problems

View reviewed changes

.github/workflows/test.yml Fixed Show fixed Hide fixed

npaun force-pushed the npaun/test-action-refac branch 5 times, most recently from bdd8e86 to f61e80d Compare

June 9, 2025 22:13

npaun marked this pull request as ready for review

June 9, 2025 23:08

npaun requested review from a team as code owners

June 9, 2025 23:08

npaun requested review from danlapid, fhanau and mikea

June 9, 2025 23:08

This comment was marked as outdated.

Sign in to view

npaun force-pushed the npaun/test-action-refac branch from 8bc983f to bae7c90 Compare

June 9, 2025 23:13

anonrig approved these changes

View reviewed changes

cloudflare deleted a comment from graphite-app bot

cloudflare deleted a comment from claude bot

cloudflare deleted a comment from graphite-app bot

cloudflare deleted a comment from graphite-app bot

cloudflare deleted a comment from graphite-app bot

cloudflare deleted a comment from windsurf-bot bot

cloudflare deleted a comment from windsurf-bot bot

cloudflare deleted a comment from windsurf-bot bot

fhanau approved these changes

View reviewed changes

Contributor

fhanau left a comment

Great work – I didn't see any major issues despite the size of the change. Did not look closely at the ts/python changes for WPT, hopefully the other reviewers will be able to do so.

os_name is upper case for release yet lower case for test (e.g. linux vs. Linux), but as far as I can see that's benign
I'm not sure if the build:linux --action_env=CC=/usr/lib/llvm-19/bin/clang block is needed, but it also seems unlikely to be harmful.
For binary names we're using matrix.target-arch instead of runner.arch now – if that happens to follow a different scheme that could be an issue, I assume you've checked that these are the same as before.

npaun force-pushed the npaun/test-action-refac branch from 7a0a023 to 89e74c8 Compare

June 12, 2025 21:35

This comment was marked as duplicate.

Sign in to view

Member Author

npaun commented Jun 12, 2025

Did not look closely at the ts/python changes for WPT, hopefully the other reviewers will be able to do so.

No worries, Yagiz looked at those.

os_name is upper case for release yet lower case for test (e.g. linux vs. Linux), but as far as I can see that's benign
For binary names we're using matrix.target-arch instead of runner.arch now – if that happens to follow a different scheme that could be an issue, I assume you've checked that these are the same as before.

These all derive from existing inconsistencies between release and test. I decided not to unify them in this PR just to have less worries about breaking stuff.

	os_name	architecture
Test	Custom list; values don't match Github	`runner.arch`
Release	`runner.os`	Custom list; values match `runner.arch`

npaun force-pushed the npaun/test-action-refac branch from 89e74c8 to 07179a2 Compare

June 12, 2025 21:49

This comment was marked as duplicate.

Sign in to view

npaun force-pushed the npaun/test-action-refac branch from 07179a2 to f747d00 Compare

June 12, 2025 21:57

This comment was marked as duplicate.

Sign in to view

npaun force-pushed the npaun/test-action-refac branch from f747d00 to add7094 Compare

June 12, 2025 22:22

This comment was marked as duplicate.

Sign in to view

This comment was marked as duplicate.

Sign in to view

graphite-app bot reviewed

View reviewed changes

tools/cross/wpt_logs.py Show resolved Hide resolved

npaun added 3 commits

June 12, 2025 15:37


          _bazel.yml can upload test logs, extract out WPT stuff

7fc00b0


          Attempt to unify release.yml and test.yml


          Add an experimental-workflow.yml file to help people test CI changes …

4653cd0

…faster

npaun force-pushed the npaun/test-action-refac branch from 3ae24f8 to 4653cd0 Compare

June 12, 2025 22:39

This comment was marked as duplicate.

Sign in to view

graphite-app bot reviewed

View reviewed changes

.github/workflows/test.yml Show resolved Hide resolved

Member Author

npaun commented Jun 12, 2025 •

edited

Loading

I dropped the check-snapshot changes from this PR as they can be applied independently in another PR. They stopped working for some reason and I couldn't figure it out from a quick glance.

npaun enabled auto-merge (squash)

June 12, 2025 22:50

npaun merged commit 7d91b30 into main

23 checks passed

npaun deleted the npaun/test-action-refac branch

June 12, 2025 22:55

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

Reviewers

anonrig anonrig approved these changes

graphite-app[bot] graphite-app[bot] left review comments

fhanau fhanau approved these changes

mikea Awaiting requested review from mikea

danlapid Awaiting requested review from danlapid

+1 more reviewer

windsurf-bot[bot] windsurf-bot[bot] left review comments

Labels

None yet