wpt icon indicating copy to clipboard operation
wpt copied to clipboard

Safari / Safari Technology Preview runs aren't actually running on the epochs

Open gsnedders opened this issue 1 year ago • 4 comments

This e.g. led to https://github.com/web-platform-tests/wpt/actions/runs/11357446457 and https://github.com/web-platform-tests/wpt/actions/runs/11358412726 to both run against https://github.com/web-platform-tests/wpt/commit/3585305e8365779fac42dd87e4f52c89c3e884a6 (which was current master when both ran), instead of 100f9048d8 and 3585305e83 respectively (see the respective epoch jobs: https://github.com/web-platform-tests/wpt/actions/runs/11357430600 and https://github.com/web-platform-tests/wpt/actions/runs/11358394569).

I think this should be as simple as telling actions/checkout what to checkout at:

https://github.com/web-platform-tests/wpt/blob/00e1df7e329f3d11b91d7b2e11a2db63bbd98ef9/.github/workflows/safari_technology_preview.yml#L47-L50

That said, merely passing fromJSON(needs.check-workflow-run.outputs.updated-refs)[0] will result in the runs being non-rerunnable, as they'll always run against whatever refs/heads/epochs/three_hourly then is. This is probably still a net-win, even if it's only a partial fix.

We probably ultimately want .github/workflows/check-workflow-run.yml to return an object like {"refs/heads/epochs/three_hourly": "100f9048d8"}. Or just return the shas.

gsnedders avatar Oct 17 '24 00:10 gsnedders

This shows potentially one major downside of using workflow_run to trigger the safari_technology_preview workflow on completion of the epochs workflow; if we instead used workflow_dispatch from epochs then that could trigger the workflow with the correct ref — but then the epochs workflow needs to know about safari_technology_preview.

For more background, see https://github.com/web-platform-tests/wpt/issues/48031.

gsnedders avatar Oct 17 '24 00:10 gsnedders

To summarize the status quo, given @jgraham asked why we can't use the workflow_run event:

The workflow_run event payload data doesn't help us much, as it mostly just lets us know that we were triggered by the epochs workflow completing, and that the epochs workflow ran against the master branch.

That doesn't tell us what epochs (if any!) got updated, so we can't get out of having to pass the ref we want to run ourselves.

Currently, we store the output of git-push --porcelain as an artifact on the epochs workflow run, which we then download (and parse) in the workflow where we actually invoke wptrunner to check that the ref we care about got updated.

Further, to make the runs re-runnable, we need to know not just what refs got updated, but what they got updated to.

And finally we then need to tell wpt.fyi, when uploading results, both what ref we ran and what commit we ran against. and we need to be able to trust that — which we admittedly probably can for runs on the default branch on the upstream repo. But it becomes an oddity where we can't trust the workflow run metadata to tell us what was being run — as that will always be the latest commit on master, because workflow_run always runs against the default branch.


The more I consider our possibilities here, I really do think going back on the previous decision to use workflow_run is the right one, as if we move to workflow_dispatch we can just pass the relevant ref and then re-runs work as expected, the run metadata is what would be expected, etc.

gsnedders avatar Oct 18 '24 04:10 gsnedders

FWIW this seems to have led to consistent "near misses" of getting any aligned runs (for stable browser channels) for the past 9 days -- right now https://wpt.fyi/runs consistently shows Safari-stable having picked a commit that was slightly later than the one that Chrome/Firefox-stable picked, at the end of each day (with Edge stable typically showing results for both commits).

The most recent day with an aligned stable run was on March 23, where everyone chose the same commit -- perhaps just because there weren't many commits to choose from on that day (only 4).

This is kinda troublesome since the interop front page only updates when we get an aligned run, which means it's stuck showing those March 23 test results indefinitely until we get lucky again and happen to have all browsers choose the same commit again as we did on March 23.

dholbert avatar Apr 01 '25 22:04 dholbert

Based on us not wanting any secrets in this repo, given the number of people with access, the other obvious approach here that makes everything work potentially more as expected is to just move https://github.com/web-platform-tests/wpt/blob/master/tools/ci/epochs_update.sh to another repo (web-platform-tests/epochs-update, say).

gsnedders avatar Jun 10 '25 05:06 gsnedders

Ah, this still isn't working, because wpt.fyi just uses the workflow SHA as revision: https://github.com/web-platform-tests/wpt.fyi/blob/0c7863e52709886815df7528b089eeeb8497949e/api/ghactions/notify.go#L156

For PRs, we rely on having the revision be the same for both the PR head and the PR base commit, so we either need to take one of the other approaches (e.g., pushing with some actual token, not just the automatic one), or we need to modify the wpt.fyi ingest pipeline to ignore the workflow SHA except for pull requests.

gsnedders avatar Sep 09 '25 17:09 gsnedders