Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trigger Safari & Safari Technology Preview runs after epochs #48087

Merged
merged 1 commit into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions .github/workflows/check-workflow-run.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
name: Check workflow_run

on:
workflow_call:
inputs:
check-refs:
description: "Refs to check whether they've been updated"
required: true
type: string
outputs:
updated-refs:
description: "Refs which have been updated"
value: ${{ jobs.check-workflow-run.outputs.output }}

jobs:
check-workflow-run:
name: "Check for appropriate epochs"
if: ${{ github.event_name == 'workflow_run' }}
runs-on:
- ubuntu-22.04
permissions:
actions: read
outputs:
output: ${{ steps.check.outputs.test }}
steps:
- uses: actions/download-artifact@v4
with:
name: git-push-output
path: ${{ runner.temp }}/git-push-output.txt
run-id: ${{ github.event.workflow_run.id }}
- id: check
run: |-
python3 tools/ci/check_for_updated_refs.py >> "$GITHUB_OUTPUT"
env:
GIT_PUSH_OUTPUT: ${{ runner.temp }}/git-push-output.txt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd have a preference for passing this via stdin if we can, rather than as an environment variable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

REFS: ${{ inputs.check-refs }}
7 changes: 7 additions & 0 deletions .github/workflows/epochs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,10 @@ jobs:
run: ./tools/ci/epochs_update.sh
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- name: Upload git-push output
uses: actions/upload-artifact@v4
with:
name: git-push-output
path: ${{ runner.temp }}/git-push-output.txt
if-no-files-found: error
compression-level: 1
21 changes: 21 additions & 0 deletions .github/workflows/safari_stable.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ name: "All Tests: Safari (stable)"
permissions: {}

on:
workflow_dispatch:
workflow_run:
workflows: [epochs]
types:
- completed
push:
branches:
- epochs/daily
Expand All @@ -17,8 +22,24 @@ env:
SAFARIDRIVER_DIAGNOSE: false

jobs:
check-workflow-run:
name: "Check for appropriate epochs"
uses: ./.github/workflows/check-workflow-run.yml
with:
check-refs: '["refs/heads/epochs/daily"]'
permissions:
actions: read

safari-stable-results:
name: "All Tests: Safari (stable)"
needs: check-workflow-run
if: |
# We need always() here to then check for success/skipped from the dependency, as otherwise
# the skip cascades. See
# https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/using-jobs-in-a-workflow#defining-prerequisite-jobs.
always() &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we have a comment here? I don't know why always() is needed. The first condition after that makes sense, and I guess the second is "run if we were manually triggered, or if there's an updated ref", but it would be nice to be sure.

Copy link
Member Author

@gsnedders gsnedders Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not just manually triggered, also a push to epochs/three_hourly or triggers/safari_stable that hasn't come from a GitHub Actions GitHub token (though obviously there's unlikely to be any of the former).

But added a comment about the always()

(needs.check-workflow-run.result == 'success' || needs.check-workflow-run.result == 'skipped') &&
(github.event_name != 'workflow_run' || fromJSON(needs.check-workflow-run.outputs.updated-refs)[0] != null)
runs-on:
- self-hosted
- webkit-ews
Expand Down
17 changes: 17 additions & 0 deletions .github/workflows/safari_technology_preview.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ name: "All Tests: Safari Technology Preview"
permissions: {}

on:
workflow_dispatch:
push:
branches:
- epochs/three_hourly
Expand All @@ -17,8 +18,24 @@ env:
SAFARIDRIVER_DIAGNOSE: false

jobs:
check-workflow-run:
name: "Check for appropriate epochs"
uses: ./.github/workflows/check-workflow-run.yml
with:
check-refs: '["refs/heads/epochs/daily"]'
permissions:
actions: read

safari-technology-preview-results:
name: "All Tests: Safari Technology Preview"
needs: check-workflow-run
if: |
# We need always() here to then check for success/skipped from the dependency, as otherwise
# the skip cascades. See
# https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/using-jobs-in-a-workflow#defining-prerequisite-jobs.
always() &&
(needs.check-workflow-run.result == 'success' || needs.check-workflow-run.result == 'skipped') &&
(github.event_name != 'workflow_run' || fromJSON(needs.check-workflow-run.outputs.updated-refs)[0] != null)
runs-on:
- self-hosted
- webkit-ews
Expand Down
47 changes: 47 additions & 0 deletions tools/ci/check_for_updated_refs.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import json
import os
import re
import sys
from typing import IO, Container, Dict, Iterable, List, Optional

GIT_PUSH = re.compile(
r"^(?P<flag>.)\t(?P<from>[^\t:]*):(?P<to>[^\t:]*)\t(?P<summary>.*[^\)])(?: \((?P<reason>[^\)]*)\))?\n$"
)


def parse_push(fd: IO[str]) -> Iterable[Dict[str, Optional[str]]]:
for line in fd:
m = GIT_PUSH.match(line)
if m is not None:
yield m.groupdict()


def process_push(fd: IO[str], refs: Container[str]) -> List[str]:
updated_refs = []

for ref_status in parse_push(fd):
flag = ref_status["flag"]
if flag not in (" ", "+", "-", "*"):
continue

to = ref_status["to"]
assert to is not None
if to in refs:
updated_refs.append(to)

return updated_refs


def main() -> None:
git_push_output = os.environ["GIT_PUSH_OUTPUT"]
refs = json.loads(os.environ["REFS"])

with open(git_push_output, "r") as fd:
updated_refs = process_push(fd, refs)

json.dump(updated_refs, sys.stdout, indent=2)
sys.stdout.write("\n")


if __name__ == "__main__":
main()
2 changes: 1 addition & 1 deletion tools/ci/epochs_update.sh
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ main () {
done
# This is safe because `git push` will by default fail for a non-fast-forward
# push, for example if the remote branch is ahead of the local branch.
git push --tags ${REMOTE} ${ALL_BRANCHES_NAMES}
git push --porcelain --tags ${REMOTE} ${ALL_BRANCHES_NAMES} | tee "${RUNNER_TEMP}/git-push-output.txt"
}

cd $WPT_ROOT
Expand Down
106 changes: 106 additions & 0 deletions tools/ci/tests/test_check_for_updated_refs.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
# mypy: allow-untyped-defs

import io

from tools.ci import check_for_updated_refs


def test_parse_push():
s = io.StringIO(
"""
To github.com:gsnedders/web-platform-tests.git
= refs/heads/a:refs/heads/a [up to date]
- :refs/heads/b [deleted]
+ refs/heads/c:refs/heads/c a6eb923e19...9b6507e295 (forced update)
* refs/heads/d:refs/heads/d [new branch]
\x20 refs/heads/e:refs/heads/e 0acd8f62f1..6188942729
! refs/heads/f:refs/heads/f [rejected] (atomic push failed)
Done
"""
)

actual = list(check_for_updated_refs.parse_push(s))
print(repr(actual))
expected = [
{
"flag": "=",
"from": "refs/heads/a",
"to": "refs/heads/a",
"summary": "[up to date]",
"reason": None,
},
{
"flag": "-",
"from": "",
"to": "refs/heads/b",
"summary": "[deleted]",
"reason": None,
},
{
"flag": "+",
"from": "refs/heads/c",
"to": "refs/heads/c",
"summary": "a6eb923e19...9b6507e295",
"reason": "forced update",
},
{
"flag": "*",
"from": "refs/heads/d",
"to": "refs/heads/d",
"summary": "[new branch]",
"reason": None,
},
{
"flag": " ",
"from": "refs/heads/e",
"to": "refs/heads/e",
"summary": "0acd8f62f1..6188942729",
"reason": None,
},
{
"flag": "!",
"from": "refs/heads/f",
"to": "refs/heads/f",
"summary": "[rejected]",
"reason": "atomic push failed",
},
]

assert expected == actual


def test_process_push():
s = io.StringIO(
"""
To github.com:gsnedders/web-platform-tests.git
= refs/heads/a:refs/heads/a [up to date]
- :refs/heads/b [deleted]
+ refs/heads/c:refs/heads/c a6eb923e19...9b6507e295 (forced update)
* refs/heads/d:refs/heads/d [new branch]
\x20 refs/heads/e:refs/heads/e 0acd8f62f1..6188942729
! refs/heads/f:refs/heads/f [rejected] (atomic push failed)
Done
"""
)

actual = list(
check_for_updated_refs.process_push(
s,
[
"refs/heads/e",
"refs/heads/b",
"refs/heads/c",
"refs/heads/d",
"refs/heads/e",
"refs/heads/x",
],
)
)
expected = [
"refs/heads/b",
"refs/heads/c",
"refs/heads/d",
"refs/heads/e",
]

assert expected == actual