-
Notifications
You must be signed in to change notification settings - Fork 17
chore(ci): link Unity Cloud builds, test reports, timings, performance and automation from the CI status comment #9713
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
base: dev
Are you sure you want to change the base?
Changes from 11 commits
00ea5fd
68875b9
26cb13e
8ee55d7
3b856eb
94e21fc
34aad87
29b7e7b
1c93440
414b0f3
b5186df
4eb8b84
53dc7e3
23e9577
a94f73d
0532ad4
5d2384c
63a0cf4
a9f8e93
536e961
447ab7a
8f4123e
e480a3a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,17 +1,19 @@ | ||
| #!/usr/bin/env bash | ||
| # Create or update the single unified CI status comment on a PR, replacing only | ||
| # one section (build | lint | tests). All three CI comment workflows call this | ||
| # through the ci-status-comment composite action, so the three separate bot | ||
| # comments collapse into one. | ||
| # one section (build | lint | tests | performance | automation). All CI comment | ||
| # workflows call this through the ci-status-comment composite action, so the | ||
| # separate bot comments collapse into one. | ||
| # | ||
| # The comment is keyed by the hidden <!-- ci-status --> marker and holds three | ||
| # sections, each fenced by its own start/end markers: | ||
| # The comment is keyed by the hidden <!-- ci-status --> marker and holds one | ||
| # fenced block per section: | ||
| # | ||
| # <!-- ci-status --> | ||
| # ### 🚦 CI Status | ||
| # <!-- ci:build:start --> …build… <!-- ci:build:end --> | ||
| # <!-- ci:lint:start --> …lint… <!-- ci:lint:end --> | ||
| # <!-- ci:tests:start --> …tests… <!-- ci:tests:end --> | ||
| # <!-- ci:build:start --> …build… <!-- ci:build:end --> | ||
| # <!-- ci:lint:start --> …lint… <!-- ci:lint:end --> | ||
| # <!-- ci:tests:start --> …tests… <!-- ci:tests:end --> | ||
| # <!-- ci:performance:start --> …performance… <!-- ci:performance:end --> | ||
| # <!-- ci:automation:start --> …automation… <!-- ci:automation:end --> | ||
| # | ||
| # Build and Unity Test run as independent workflows whose comment writers can | ||
| # fire at the same time, so a plain read-modify-write would drop a section or | ||
|
|
@@ -20,6 +22,49 @@ | |
| # confirm the section landed and no duplicate slipped in — retrying otherwise. | ||
| set -euo pipefail | ||
|
|
||
| # Optional caller knobs (used by decentraland/performance-testing, which runs | ||
| # this script directly against unity-explorer's unified comment): | ||
| # SECTION_BODY_FILE — read the body from a file instead of $SECTION_BODY, | ||
| # for bodies too large to pass comfortably via env. | ||
| # NO_CREATE=1 — never create the unified comment; exit 3 when it does | ||
| # not exist so the caller can fall back to a standalone | ||
| # comment (a foreign-token creation would not be authored | ||
| # by github-actions[bot] and later writers would not | ||
| # find it, spawning duplicates). | ||
| if [ -n "${SECTION_BODY_FILE:-}" ]; then | ||
| SECTION_BODY="$(cat "$SECTION_BODY_FILE")" | ||
| fi | ||
|
|
||
| # GitHub caps an issue comment at 65536 chars across every section; keep one | ||
| # writer — whichever path its body arrived by — from consuming the whole budget | ||
| # and failing an unrelated section's PATCH with an opaque 422. Truncation is | ||
| # fine for a status section that already links out to the full report. | ||
| if [ "${#SECTION_BODY}" -gt 20000 ]; then | ||
| echo "::warning::Section body is ${#SECTION_BODY} chars; truncating to 20000." | ||
| SECTION_BODY="${SECTION_BODY:0:20000}" | ||
| # Close constructs the cut may have severed — an unterminated code fence or | ||
| # <details> makes GitHub render everything after it in this comment inside | ||
| # the open block, visually eating the neighbouring sections. | ||
| if [ $(( $(grep -c '^```' <<< "$SECTION_BODY") % 2 )) -ne 0 ]; then | ||
| SECTION_BODY="$SECTION_BODY"$'\n''```' | ||
| fi | ||
| opens=$(grep -oi '<details' <<< "$SECTION_BODY" | wc -l || true) | ||
| closes=$(grep -oi '</details' <<< "$SECTION_BODY" | wc -l || true) | ||
| while [ "${opens:-0}" -gt "${closes:-0}" ]; do | ||
| SECTION_BODY="$SECTION_BODY"$'\n</details>' | ||
| closes=$((closes + 1)) | ||
| done | ||
| SECTION_BODY="$SECTION_BODY"$'\n\n'"_…truncated; see the linked run for the full report._" | ||
| fi | ||
|
|
||
| # Fail fast on a section name outside the fence set — an unknown name would | ||
| # append a dead fence to the shared comment and then wedge the survive check | ||
| # for 5 attempts, burning ~15 API calls per write from then on. | ||
| case "${SECTION:-}" in | ||
| build|lint|tests|performance|automation) ;; | ||
| *) echo "::error::Unknown section '${SECTION:-}'."; exit 2 ;; | ||
| esac | ||
|
|
||
| MARKER="<!-- ci-status -->" | ||
| HEADER="### 🚦 CI Status" | ||
| BOT="github-actions[bot]" | ||
|
|
@@ -33,6 +78,8 @@ section_default() { | |
| build) printf '\n\n_Waiting for the build to start…_' ;; | ||
| lint) printf '\n\n_Waiting for lint to start…_' ;; | ||
| tests) printf '\n\n_Waiting for tests to start…_' ;; | ||
| automation) printf '\n\n_On demand — comment `/visual-tests` on this PR to run the visual regression suite against its build._' ;; | ||
| performance) printf '\n\n_Bare-metal benchmarks run automatically after each successful build; results arrive as a separate comment. Add the `perf_test` label to run the in-repo Unity performance suite instead (skips normal CI and blocks merge while set)._' ;; | ||
| esac | ||
| } | ||
|
|
||
|
|
@@ -41,11 +88,13 @@ wrap_section() { printf '<!-- ci:%s:start -->\n%s\n<!-- ci:%s:end -->' "$1" "$2" | |
|
|
||
| # A fresh comment with every section defaulted to "waiting". | ||
| skeleton() { | ||
| printf '%s\n%s\n\n%s\n\n%s\n\n%s\n' \ | ||
| printf '%s\n%s\n\n%s\n\n%s\n\n%s\n\n%s\n\n%s\n' \ | ||
| "$MARKER" "$HEADER" \ | ||
| "$(wrap_section build "$(section_default build)")" \ | ||
| "$(wrap_section lint "$(section_default lint)")" \ | ||
| "$(wrap_section tests "$(section_default tests)")" | ||
| "$(wrap_section tests "$(section_default tests)")" \ | ||
| "$(wrap_section performance "$(section_default performance)")" \ | ||
| "$(wrap_section automation "$(section_default automation)")" | ||
| } | ||
|
|
||
| # Emit the section body for this run to a file so awk can splice it verbatim, | ||
|
|
@@ -94,8 +143,24 @@ for attempt in 1 2 3 4 5; do | |
| while IFS= read -r line; do [ -n "$line" ] && IDS+=("$line"); done <<< "$(marker_ids "$COMMENTS")" | ||
| COMMENT_ID="${IDS[0]:-}" | ||
|
|
||
| # Collapse accidental duplicates from a create race: keep the oldest, drop the rest. | ||
| if [ "${#IDS[@]}" -gt 1 ]; then | ||
| if [ -z "$COMMENT_ID" ] && [ -n "${NO_CREATE:-}" ]; then | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The reasoning in the header comment is right, and it's the reasoning that makes me want one more line here.
I'm not claiming a new capability — a token that can PATCH the comment can generally delete it too, so a hostile caller isn't gated by this either way. The concern is blast radius under the accidental case: garbage-collecting unity-explorer's own bot comments is repo-maintenance work that a benchmark runner in another repo has no business doing, and if it ever misfires (a pagination hiccup, a marker-bearing comment that isn't actually a duplicate) the evidence is deleted and the # Collapse accidental duplicates from a create race: keep the oldest, drop the
# rest. Skipped for external callers — comment GC belongs to this repo's own
# workflows, which run often enough to clean up within minutes.
if [ "${#IDS[@]}" -gt 1 ] && [ -z "${NO_CREATE:-}" ]; thenTwo smaller notes on the block as written: No retry before giving up. This exits on attempt 1, so an external caller that arrives in the window before the build workflow seeds the comment falls back to a standalone comment even though the unified one appears seconds later. Since the loop is already here, letting it lose one round first would cut that noise — move the check to fire only on the last attempt, or
|
||
| # Lose one round before falling back: an external caller often lands here | ||
| # seconds before the build workflow seeds the comment, and the standalone | ||
| # fallback it would post instead is noise that never collapses. | ||
| if [ "$attempt" -ge 2 ]; then | ||
| echo "No unified CI status comment exists and NO_CREATE is set; leaving creation to the repo's own workflows." | ||
| exit 3 | ||
| fi | ||
| echo "No unified CI status comment yet (attempt $attempt); waiting for the repo's own workflows to seed it." | ||
| sleep $((attempt * 2)) | ||
| continue | ||
| fi | ||
|
|
||
| # Collapse accidental duplicates from a create race: keep the oldest, drop the | ||
| # rest. Skipped for external callers — comment GC belongs to this repo's own | ||
| # workflows, which run often enough to clean up within minutes, and a misfire | ||
| # under a foreign token would delete evidence with nothing logged. | ||
| if [ "${#IDS[@]}" -gt 1 ] && [ -z "${NO_CREATE:-}" ]; then | ||
| for extra in "${IDS[@]:1}"; do | ||
| echo "Deleting duplicate CI status comment $extra." | ||
| gh api -X DELETE "/repos/$REPO/issues/comments/$extra" >/dev/null || true | ||
|
|
@@ -108,10 +173,18 @@ for attempt in 1 2 3 4 5; do | |
| CURRENT_BODY="" | ||
| fi | ||
|
|
||
| # No unified comment yet, or one missing our section markers: start clean so | ||
| # all three sections are always present. | ||
| if [ -z "$CURRENT_BODY" ] || ! grep -qF "$START" <<< "$CURRENT_BODY"; then | ||
| # No unified comment yet: start from the full skeleton. A comment that exists | ||
| # but lacks our markers predates this section (e.g. it was written before the | ||
| # automation section existed) — append an empty fence for just our section | ||
| # instead of resetting the whole comment and wiping the other sections' state. | ||
| if [ -z "$CURRENT_BODY" ]; then | ||
| CURRENT_BODY="$(skeleton)" | ||
| # -x: whole-line, matching replace_section/extract_section's $0==s exactly. A | ||
| # substring hit on a marker embedded in a body line (which the strip filter | ||
| # deliberately lets through) would skip fence creation here while the awk | ||
| # matchers see nothing — leaving the section permanently unwritable. | ||
| elif ! grep -qxF "$START" <<< "$CURRENT_BODY"; then | ||
| CURRENT_BODY="$CURRENT_BODY"$'\n\n'"$(wrap_section "$SECTION" "$(section_default "$SECTION")")" | ||
| fi | ||
|
|
||
| NEW_BODY="$(replace_section "$CURRENT_BODY")" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,145 @@ | ||
| name: Fetch Unity Cloud Build Links | ||
| description: >- | ||
| Download the unity_build_info_* artifacts of a Unity Cloud Build run and emit | ||
| sanitized markdown linking each build id to its Unity Cloud dashboard page: | ||
| bare table rows for appending to an existing two-column table, and a standalone | ||
| table section for comment bodies that have no table of their own. | ||
|
|
||
| inputs: | ||
| run-id: | ||
| description: Workflow run id of the Unity Cloud Build run whose artifacts to read. | ||
| required: true | ||
| github-token: | ||
| description: Token used to download the run's artifacts. | ||
| required: true | ||
|
|
||
| outputs: | ||
| rows: | ||
| description: >- | ||
| "| Name | Link |"-shaped rows for an existing two-column table, one per | ||
| target, each pairing the GitHub job log, the Unity Cloud build page and | ||
| the Unity log artifact; empty when no valid build info was found. | ||
| value: ${{ steps.fetch.outputs.rows }} | ||
| section: | ||
| description: >- | ||
| Standalone table (header + rows); empty when no valid build info was found. | ||
| value: ${{ steps.fetch.outputs.section }} | ||
| windows-cell: | ||
| description: >- | ||
| The Windows row's link cell alone ("[GitHub job](…) · [Unity Cloud #N](…) · | ||
| [Unity log](…)"), for callers composing their own rows; empty when unknown. | ||
| value: ${{ steps.fetch.outputs.windows-cell }} | ||
| mac-cell: | ||
| description: Mac twin of windows-cell. | ||
| value: ${{ steps.fetch.outputs.mac-cell }} | ||
|
|
||
| runs: | ||
| using: composite | ||
| steps: | ||
| - name: Download and sanitize Unity Cloud build info | ||
| id: fetch | ||
| shell: bash | ||
| env: | ||
| GH_TOKEN: ${{ inputs.github-token }} | ||
| RUN_ID: ${{ inputs.run-id }} | ||
| REPO_FULL: ${{ github.repository }} | ||
| run: | | ||
| set -euo pipefail | ||
|
|
||
| # The info files come out of the PR-controlled build workflow, so treat them as | ||
| # untrusted input: accept only a numeric build id and a Unity dashboard URL with | ||
| # a conservative charset before letting them anywhere near a comment body. | ||
| # Mirrors the producer's '/builds/<id>' requirement (build.py) so the two | ||
| # validators agree, and pins the id to digits — a query-string-only path | ||
| # under a Unity host (open-redirect bait) no longer passes. | ||
| URL_RE='^https://(cloud\.unity\.com|developer\.cloud\.unity3d\.com|dashboard\.unity3d\.com)/[A-Za-z0-9./_%~?=&#-]*/builds/[0-9]+[A-Za-z0-9./_%~?=&#-]*$' | ||
| parse_info() { | ||
| local target="$1" | ||
| local dir="ucb_info_${target}" | ||
| REPLY_ID="" | ||
| REPLY_URL="" | ||
| if gh run download "$RUN_ID" \ | ||
| --repo "$REPO_FULL" \ | ||
| --name "unity_build_info_${target}_launcher" \ | ||
| --dir "$dir" 2>"${dir}.err"; then | ||
| REPLY_ID=$(grep -m1 '^BUILD_ID=' "$dir/unity_cloud_build_info.env" | cut -d= -f2- || true) | ||
| REPLY_URL=$(grep -m1 '^DASHBOARD_URL=' "$dir/unity_cloud_build_info.env" | cut -d= -f2- || true) | ||
| [[ "$REPLY_ID" =~ ^[0-9]+$ ]] || REPLY_ID="" | ||
| [[ "$REPLY_URL" =~ $URL_RE ]] || REPLY_URL="" | ||
| else | ||
| # Absence is normal for runs predating the info artifact; still surface the | ||
| # gh error so an auth/permission regression doesn't silently eat the rows. | ||
| echo "note: could not fetch unity_build_info_${target}_launcher: $(tr '\n' ' ' < "${dir}.err")" | ||
| fi | ||
| } | ||
|
|
||
| # Per-target GitHub job pages, from the trusted Actions API (jobs of the | ||
| # matrix job "Build (<target>)"), so each row pairs the Unity Cloud build | ||
| # page with the GitHub-side job log. | ||
| JOBS_JSON=$(gh api "/repos/$REPO_FULL/actions/runs/$RUN_ID/jobs?per_page=100" 2>/dev/null || echo '{"jobs":[]}') | ||
|
|
||
| # Suite id + artifact ids feed the per-target Unity log download links | ||
| # (artifact downloads hang off the check suite, not the run). | ||
| SUITE_ID=$(gh api "/repos/$REPO_FULL/actions/runs/$RUN_ID" --jq '.check_suite_id' 2>/dev/null || echo "") | ||
| [[ "$SUITE_ID" =~ ^[0-9]+$ ]] || SUITE_ID="" | ||
| ARTIFACTS_JSON=$(gh api "/repos/$REPO_FULL/actions/runs/$RUN_ID/artifacts?per_page=100" 2>/dev/null || echo '{"artifacts":[]}') | ||
|
|
||
| WINDOWS_CELL="" | ||
| MAC_CELL="" | ||
| ROWS="" | ||
| for entry in "windows64:Windows" "macos:Mac"; do | ||
| target="${entry%%:*}" | ||
| label="${entry#*:}" | ||
| parse_info "$target" | ||
| job_url=$(jq -r --arg n "Build ($target)" '.jobs[]? | select(.name==$n) | .html_url // empty' <<< "$JOBS_JSON" | head -1) | ||
| log_id=$(jq -r --arg n "${target}_launcher_unity_log" \ | ||
| '.artifacts[]? | select(.name==$n and .expired==false) | .id' <<< "$ARTIFACTS_JSON" | head -1) | ||
| [[ "$log_id" =~ ^[0-9]+$ ]] || log_id="" | ||
|
|
||
| parts=() | ||
| [ -n "$job_url" ] && parts+=("[GitHub job](${job_url})") | ||
| # A URL without a valid id only occurs on a tampered artifact — drop the link | ||
| # rather than render an empty "[#](...)" label. | ||
| if [ -n "$REPLY_ID" ] && [ -n "$REPLY_URL" ]; then | ||
| parts+=("[Unity Cloud #${REPLY_ID}](${REPLY_URL})") | ||
| elif [ -n "$REPLY_ID" ]; then | ||
| parts+=("Unity Cloud #${REPLY_ID}") | ||
| fi | ||
| if [ -n "$log_id" ] && [ -n "$SUITE_ID" ]; then | ||
| parts+=("[Unity log](${GITHUB_SERVER_URL:-https://github.com}/${REPO_FULL}/suites/${SUITE_ID}/artifacts/${log_id})") | ||
| fi | ||
|
|
||
| cell="" | ||
| if [ "${#parts[@]}" -gt 0 ]; then | ||
| cell=$(printf '%s · ' "${parts[@]}") | ||
| cell="${cell% · }" | ||
| ROWS+="| ${label} | ${cell} |"$'\n' | ||
| fi | ||
| case "$target" in | ||
| windows64) WINDOWS_CELL="$cell" ;; | ||
| macos) MAC_CELL="$cell" ;; | ||
| esac | ||
| done | ||
|
|
||
| SECTION="" | ||
| if [ -n "$ROWS" ]; then | ||
| SECTION="| Name | Link |"$'\n'"| -------- | ----------------------- |"$'\n'"$ROWS" | ||
| fi | ||
|
|
||
| # The payload derives from artifact bytes, so the heredoc delimiter must not be | ||
| # guessable content even though the validation above already forbids newlines. | ||
| DELIM="UCB_EOF_${RANDOM}${RANDOM}_$$" | ||
| { | ||
| echo "rows<<${DELIM}" | ||
| printf '%s' "$ROWS" | ||
| echo "${DELIM}" | ||
| echo "section<<${DELIM}" | ||
| printf '%s' "$SECTION" | ||
| echo "${DELIM}" | ||
| echo "windows-cell<<${DELIM}" | ||
| printf '%s' "$WINDOWS_CELL" | ||
| echo "${DELIM}" | ||
| echo "mac-cell<<${DELIM}" | ||
| printf '%s' "$MAC_CELL" | ||
| echo "${DELIM}" | ||
| } >> "$GITHUB_OUTPUT" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -597,6 +597,10 @@ jobs: | |
| QUEUE_TIMEOUT: 14400 | ||
| BUILD_TIMEOUT: 10800 | ||
| TARGET: t_${{ matrix.target }} | ||
| # For the live PR status-comment update the moment the Unity-side | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line changes what the
In the other direction, the job runs PR-authored code (
build:
name: Build
runs-on: ubuntu-latest
needs: prebuild
permissions:
contents: read
pull-requests: writeWorth confirming against Unrelated and pre-existing, but adjacent enough to mention while you're in this file: L800's |
||
| # build id is known; empty PR number (push/dispatch) disables it. | ||
| GH_TOKEN: ${{ github.token }} | ||
| PR_NUMBER: ${{ github.event.pull_request.number || '' }} | ||
| BRANCH_NAME: ${{ github.head_ref || github.ref_name }} | ||
| COMMIT_SHA: ${{ needs.prebuild.outputs.commit_sha }} | ||
| BUILD_OPTIONS: ${{ needs.prebuild.outputs.options }} | ||
|
|
@@ -976,6 +980,19 @@ jobs: | |
| path: unity_cloud_log.log | ||
| if-no-files-found: error | ||
|
|
||
| # Written by build.py as soon as the Unity-side build id is known, so it exists for | ||
| # failed builds too. The PR status comment uses it to deep-link the Unity Cloud | ||
| # build page instead of asking humans to search cloud.unity.com by hand. | ||
| - name: Upload Unity Cloud build info | ||
| if: ${{ always() && hashFiles('unity_cloud_build_info.env') != '' }} | ||
| uses: actions/upload-artifact@v6 | ||
| with: | ||
| name: unity_build_info_${{ matrix.target }}_${{ needs.prebuild.outputs.install_source }} | ||
| path: unity_cloud_build_info.env | ||
| if-no-files-found: error | ||
| # Only consumed by the immediately-following PR status comment run. | ||
| retention-days: 7 | ||
|
|
||
| - name: Print cloud logs | ||
| if: ${{ always() && hashFiles('unity_cloud_log.log') != '' }} | ||
| run: cat unity_cloud_log.log | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The stated motivation for this knob — "for bodies too large to pass comfortably via env" — is also what makes it worth a guard, because the next ceiling up is a hard one.
The file content correctly rejoins the existing path: it lands in
$SECTION_BODYand goes through the same marker-strip at L76-77, so a foreign body can't open or close a section fence. That's the part that mattered most and it's right.What the knob newly invites is a body that pushes
NEW_BODYpast GitHub's 65,536-character issue-comment limit. When that happens the PATCH at L153 returns 422,gh apiexits non-zero, andset -euo pipefailaborts the script mid-loop — no truncation, no fallback, no::warning::. The external caller sees a failed step whose log saysHTTP 422and nothing about the shared comment being full. Worse, the ceiling is on the combined body, so the failure surfaces on whichever writer happens to run when the total crosses over — which may well be the build or tests section, not the perf section that grew.A cap on the section body keeps the failure local and legible, and truncation is the right behaviour for a status section that already links out to the full report:
Separately, while this is now a documented cross-repo entry point:
$SECTIONis still unvalidated. It only ever reachesSTART/END, so it isn't an injection concern — but a typo'd or drifting value (perfinstead ofperformance, or anything carrying a run id) takes the L141 append branch, permanently adds a dead fence to that PR's comment, then fails the survive check for 5 attempts × a full paginated re-read. Two lines make the new contract fail fast instead of wedging the shared comment: