diff --git a/.github/actions/ci-status-comment/action.yml b/.github/actions/ci-status-comment/action.yml index 22f41506f1e..4d0bd9e58b0 100644 --- a/.github/actions/ci-status-comment/action.yml +++ b/.github/actions/ci-status-comment/action.yml @@ -1,8 +1,10 @@ name: Upsert CI Status Comment description: >- Create or update the single unified CI status comment on a PR, replacing only - the given section (build | lint | tests). Seeds a skeleton with all three - sections the first time it runs, and re-reads/retries so concurrent writers + the given section (build | lint | tests | performance | automation | + inworld). Seeds a skeleton with every always-present section the first time + it runs, appends a missing section fence to older comments (and for the + on-demand inworld section), and re-reads/retries so concurrent writers (build vs. Unity Test) never clobber each other's section. inputs: @@ -10,10 +12,16 @@ inputs: description: Pull request number to comment on. required: true section: - description: Which section to replace — one of build, lint, tests. + description: Which section to replace — one of build, lint, tests, performance, automation, inworld. required: true body: - description: Markdown for this section (inline badge + message). Rendered as-is between the section markers. + description: >- + Markdown for this section (inline badge + message). Rendered between the + section markers after dropping marker-shaped lines; bodies over 10000 + chars are truncated with fences/
re-closed and a truncation note. + Callers must keep it under ~120KB: it travels as one env string, and + Linux rejects any single env entry over 128KiB (E2BIG) before the + truncation here can run. required: true github-token: description: Token with pull-requests:write used to read and upsert the comment. diff --git a/.github/actions/ci-status-comment/test-upsert-ci-status.sh b/.github/actions/ci-status-comment/test-upsert-ci-status.sh new file mode 100644 index 00000000000..ee3cba29bd4 --- /dev/null +++ b/.github/actions/ci-status-comment/test-upsert-ci-status.sh @@ -0,0 +1,270 @@ +#!/usr/bin/env bash +# Functional tests for upsert-ci-status.sh against a stubbed gh whose comment +# store is a JSON file — no network, no repo. Run from anywhere: +# bash .github/actions/ci-status-comment/test-upsert-ci-status.sh +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +UPSERT="$SCRIPT_DIR/upsert-ci-status.sh" +WORK="$(mktemp -d)" +trap 'rm -rf "$WORK"' EXIT + +# --- gh stub ---------------------------------------------------------------- +# Supports exactly the calls the script makes; comments live in $STORE as a +# JSON array of {id, user:{login}, body}. +mkdir -p "$WORK/bin" +cat > "$WORK/bin/gh" <<'STUB' +#!/usr/bin/env bash +set -euo pipefail +exec python3 "$GH_STUB_PY" "$@" +STUB +chmod +x "$WORK/bin/gh" +cat > "$WORK/gh-stub.py" <<'PY' +import json, os, sys + +store = os.environ['STORE'] + +def load(): + with open(store) as f: + return json.load(f) + +def save(comments): + with open(store, 'w') as f: + json.dump(comments, f) + +args = sys.argv[1:] +if args[0] != 'api': + sys.exit(f'gh stub: unsupported subcommand {args[0]}') +args = args[1:] + +method = 'GET' +if '-X' in args: + i = args.index('-X') + method = args[i + 1] + del args[i:i + 2] +read_stdin = '--input' in args +path = next(a for a in args if a.startswith('/')) + +comments = load() +if method == 'GET': + # Optional eventual-consistency simulation: while the STALE_READS_FILE + # counter is positive and a pre-PATCH snapshot exists, serve the snapshot + # instead of the live store and decrement the counter. + stale_file = os.environ.get('STALE_READS_FILE') + snapshot = store + '.prev' + if stale_file and os.path.exists(stale_file) and os.path.exists(snapshot): + remaining = int(open(stale_file).read().strip() or 0) + if remaining > 0: + with open(snapshot) as f: + comments = json.load(f) + with open(stale_file, 'w') as f: + f.write(str(remaining - 1)) + # --paginate --slurp shape: array of pages. + print(json.dumps([comments])) +elif method == 'POST': + body = json.load(sys.stdin)['body'] + new_id = max([c['id'] for c in comments], default=0) + 1 + comments.append({'id': new_id, 'user': {'login': 'github-actions[bot]'}, 'body': body}) + save(comments) + print(json.dumps({'id': new_id})) +elif method == 'PATCH': + cid = int(path.rsplit('/', 1)[1]) + body = json.load(sys.stdin)['body'] + if os.environ.get('STALE_READS_FILE'): + # Snapshot the pre-PATCH store so stale GETs can serve it. + with open(store + '.prev', 'w') as f: + json.dump(comments, f) + for c in comments: + if c['id'] == cid: + c['body'] = body + save(comments) + print(json.dumps({'id': cid})) +elif method == 'DELETE': + cid = int(path.rsplit('/', 1)[1]) + save([c for c in comments if c['id'] != cid]) +PY + +export PATH="$WORK/bin:$PATH" +export GH_STUB_PY="$WORK/gh-stub.py" +export STORE="$WORK/comments.json" +export REPO="example/repo" PR_NUMBER="1" GITHUB_TOKEN="stub" + +FAILED=0 +fail() { echo "FAIL: $1"; FAILED=1; } +pass() { echo "ok: $1"; } + +reset_store() { echo "${1:-[]}" > "$STORE"; } + +body_of() { python3 -c 'import json,sys; print(json.load(open(sys.argv[1]))[int(sys.argv[2])]["body"])' "$STORE" "${1:-0}"; } +count() { python3 -c 'import json,sys; print(len(json.load(open(sys.argv[1]))))' "$STORE"; } + +run_upsert() { (cd "$WORK" && SECTION="$1" SECTION_BODY="$2" bash "$UPSERT"); } + +# --- 1. create from skeleton ------------------------------------------------- +reset_store +run_upsert build "BUILD-CONTENT" >/dev/null +[ "$(count)" = 1 ] || fail "create: expected 1 comment" +BODY="$(body_of)" +grep -q 'BUILD-CONTENT' <<< "$BODY" || fail "create: build content missing" +grep -q '' <<< "$BODY" || fail "create: performance fence missing" +grep -q '' <<< "$BODY" && fail "create: on-demand inworld fence in skeleton" +grep -q '🚦 CI Status' <<< "$BODY" || fail "create: emoji header missing" +grep -q 'decentraland_256x256' <<< "$BODY" && fail "create: retired logo header present" +pass "create seeds skeleton with all always-present sections" + +# --- 2. section update preserves the others --------------------------------- +run_upsert tests "TESTS-CONTENT" >/dev/null +BODY="$(body_of)" +grep -q 'BUILD-CONTENT' <<< "$BODY" || fail "update: build content lost" +grep -q 'TESTS-CONTENT' <<< "$BODY" || fail "update: tests content missing" +pass "section update preserves other sections" + +# --- 3. missing fence appended, others intact -------------------------------- +reset_store "$(python3 - <<'PY' +import json +body = ('\n### Decentraland CI Status\n' + "\nOLD-BUILD\n") +print(json.dumps([{"id": 5, "user": {"login": "github-actions[bot]"}, "body": body}])) +PY +)" +run_upsert automation "AUTO-CONTENT" >/dev/null +BODY="$(body_of)" +grep -q 'OLD-BUILD' <<< "$BODY" || fail "append: existing section wiped" +grep -q 'AUTO-CONTENT' <<< "$BODY" || fail "append: new section missing" +grep -q '🚦 CI Status' <<< "$BODY" || fail "append: logo header not migrated" +grep -q 'decentraland_256x256' <<< "$BODY" && fail "append: retired logo header still present" +pass "missing fence appended + header migrated" + +# --- 3b. on-demand section: appended to an existing comment, and on create ---- +# inworld is not in the skeleton; a write must append its fence to a comment +# seeded without it — and a write that has to create the comment must append +# the fence to the fresh skeleton too, not wedge the survive check. +reset_store +run_upsert build "BUILD-FIRST" >/dev/null +run_upsert inworld "INWORLD-CONTENT" >/dev/null +[ "$(count)" = 1 ] || fail "inworld append: expected 1 comment" +BODY="$(body_of)" +grep -q 'BUILD-FIRST' <<< "$BODY" || fail "inworld append: build content lost" +grep -q 'INWORLD-CONTENT' <<< "$BODY" || fail "inworld append: content missing" +[ "$(grep -cF '' <<< "$BODY")" = 1 ] || fail "inworld append: fence count wrong" +reset_store +OUT="$(run_upsert inworld "INWORLD-SEEDS")" +grep -q 'updated (attempt 1)' <<< "$OUT" || fail "inworld create: did not settle on attempt 1" +BODY="$(body_of)" +grep -q 'INWORLD-SEEDS' <<< "$BODY" || fail "inworld create: content missing" +grep -q '' <<< "$BODY" || fail "inworld create: skeleton sections missing" +pass "on-demand inworld section appended on update and on create" + +# --- 4. marker-shaped body lines are stripped -------------------------------- +reset_store +run_upsert build "$(printf 'SAFE\n\nALSO-SAFE')" >/dev/null +BODY="$(body_of)" +[ "$(grep -c '' <<< "$BODY")" = 1 ] || fail "strip: injected fence survived" +grep -q 'ALSO-SAFE' <<< "$BODY" || fail "strip: legitimate line lost" +pass "marker-shaped body lines stripped" + +# --- 5. duplicate GC keeps the oldest ---------------------------------------- +reset_store "$(python3 - <<'PY' +import json +mk = lambda i: {"id": i, "user": {"login": "github-actions[bot]"}, + "body": "\nhdr\n\nB%d\n" % i} +print(json.dumps([mk(3), mk(9)])) +PY +)" +run_upsert build "DEDUPED" >/dev/null +[ "$(count)" = 1 ] || fail "gc: duplicate not deleted" +grep -q 'DEDUPED' <<< "$(body_of)" || fail "gc: content missing on survivor" +pass "duplicate collapse keeps one comment with the write" + +# --- 6. NO_CREATE exits 3 without creating ----------------------------------- +reset_store +set +e +(cd "$WORK" && SECTION=performance SECTION_BODY=X NO_CREATE=1 bash "$UPSERT") >/dev/null 2>&1 +RC=$? +set -e +[ "$RC" = 3 ] || fail "no-create: expected exit 3, got $RC" +[ "$(count)" = 0 ] || fail "no-create: comment was created" +pass "NO_CREATE exits 3, creates nothing" + +# --- 7. unknown section exits 2 ---------------------------------------------- +set +e +(cd "$WORK" && SECTION=bogus SECTION_BODY=X bash "$UPSERT") >/dev/null 2>&1 +RC=$? +set -e +[ "$RC" = 2 ] || fail "allowlist: expected exit 2, got $RC" +pass "unknown section exits 2" + +# --- 8. oversized body truncates and re-closes constructs -------------------- +reset_store +BIG="$WORK/big-body.md" +{ + echo '
big' + echo '```' + for i in $(seq 1 3000); do echo "line $i of filler to overflow the cap"; done +} > "$BIG" +(cd "$WORK" && SECTION=tests SECTION_BODY= SECTION_BODY_FILE="$BIG" bash "$UPSERT") >/dev/null +BODY="$(body_of)" +grep -q 'truncated' <<< "$BODY" || fail "truncate: no truncation note" +[ "$(( $(grep -c '^```' <<< "$BODY") % 2 ))" = 0 ] || fail "truncate: unbalanced code fence" +SECTION_CONTENT="$(awk '/^$/{grab=1;next} /^$/{grab=0} grab' <<< "$BODY")" +[ "${#SECTION_CONTENT}" -le 10000 ] || fail "truncate: section is ${#SECTION_CONTENT} chars, closers re-inflated past the cap" +pass "oversized body truncated with constructs closed" + +# --- 9. embedded own-section markers must not scramble the comment ------------ +# The wedge shape: the body smuggles in this section's own end marker (which +# would truncate the fence) and the top-level comment marker. The script strips +# such lines, so the write must settle on the first attempt with the structure +# intact — one marker, one end fence — and later writers must still land. +reset_store +OUT="$(run_upsert build "$(printf 'BEFORE\n\n \nAFTER')")" +grep -q 'updated (attempt 1)' <<< "$OUT" || fail "wedge: write did not settle on attempt 1" +[ "$(count)" = 1 ] || fail "wedge: expected 1 comment" +BODY="$(body_of)" +grep -q 'BEFORE' <<< "$BODY" || fail "wedge: content before marker lost" +grep -q 'AFTER' <<< "$BODY" || fail "wedge: content after marker lost" +[ "$(grep -c '' <<< "$BODY")" = 1 ] || fail "wedge: embedded end marker survived" +[ "$(grep -c '' <<< "$BODY")" = 1 ] || fail "wedge: embedded comment marker survived" +run_upsert lint "LINT-AFTER-WEDGE" >/dev/null +BODY="$(body_of)" +grep -q 'LINT-AFTER-WEDGE' <<< "$BODY" || fail "wedge: later section write lost" +grep -q 'BEFORE' <<< "$BODY" || fail "wedge: later write wiped earlier section" +pass "embedded section markers stripped, comment structure intact" + +# --- 10. stale re-read after PATCH retries and converges ---------------------- +# Create-race shape: the PATCH lands, but the confirming re-read returns a +# stale body whose section content differs from WANT. The script must treat +# that as unsettled, retry, and converge once reads are fresh again. +reset_store "$(python3 - <<'PY' +import json +body = ("\n### 🚦 CI Status\n" + "\nPRE-RACE\n") +print(json.dumps([{"id": 7, "user": {"login": "github-actions[bot]"}, "body": body}])) +PY +)" +echo 1 > "$WORK/stale-reads" +OUT="$( (cd "$WORK" && SECTION=build SECTION_BODY="RACE-CONVERGED" STALE_READS_FILE="$WORK/stale-reads" bash "$UPSERT") )" +grep -q "not settled (attempt 1)" <<< "$OUT" || fail "race: stale re-read did not trigger a retry" +grep -q 'updated (attempt 2)' <<< "$OUT" || fail "race: did not converge on attempt 2" +grep -q 'RACE-CONVERGED' <<< "$(body_of)" || fail "race: final body missing converged content" +[ "$(cat "$WORK/stale-reads")" = 0 ] || fail "race: stale read was not consumed" +pass "stale re-read retried until convergence" + +# --- 11. CRLF body normalized in place, fences not duplicated ----------------- +reset_store "$(python3 - <<'PY' +import json +body = ("\r\n### 🚦 CI Status\r\n" + "\r\nOLD-BUILD\r\n") +print(json.dumps([{"id": 9, "user": {"login": "github-actions[bot]"}, "body": body}])) +PY +)" +run_upsert build "CRLF-BUILD" >/dev/null +[ "$(count)" = 1 ] || fail "crlf: expected 1 comment" +BODY="$(body_of)" +[ "$(grep -cF '' <<< "$BODY")" = 1 ] || fail "crlf: build fence duplicated" +grep -q 'CRLF-BUILD' <<< "$BODY" || fail "crlf: new content missing" +grep -q 'OLD-BUILD' <<< "$BODY" && fail "crlf: stale content still rendered" +grep -q $'\r' <<< "$BODY" && fail "crlf: body still carries CR" +pass "CRLF body replaced in place, no duplicate fences" + +[ "$FAILED" = 0 ] && echo "ALL PASS" || { echo "FAILURES PRESENT"; exit 1; } diff --git a/.github/actions/ci-status-comment/upsert-ci-status.sh b/.github/actions/ci-status-comment/upsert-ci-status.sh index 933e1b36209..f75330cfc92 100755 --- a/.github/actions/ci-status-comment/upsert-ci-status.sh +++ b/.github/actions/ci-status-comment/upsert-ci-status.sh @@ -1,17 +1,22 @@ #!/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 | inworld). CI +# comment workflows call this through the ci-status-comment composite action; +# build.py (live build rows), decentraland/performance-testing (benchmark +# report) and decentraland/explorer-automation (InWorld suite) run it directly. +# Either way the separate bot comments collapse into one. # -# The comment is keyed by the hidden marker and holds three -# sections, each fenced by its own start/end markers: +# The comment is keyed by the hidden marker and holds the +# $HEADER heading plus one fenced block per section: # # # ### 🚦 CI Status -# …build… -# …lint… -# …tests… +# …build… +# …lint… +# …tests… +# …performance… +# …automation… +# …inworld… # # 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,8 +25,83 @@ # confirm the section landed and no duplicate slipped in — retrying otherwise. set -euo pipefail +# Optional caller knobs for direct invocations (build.py, and +# decentraland/performance-testing writing against unity-explorer's 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 + +# Everything below matches markers as whole lines, which CRLF endings defeat — +# and GitHub's web editor resubmits an edited comment with \r\n. Normalize the +# body here and every API read below, so one manual edit cannot make each +# writer append a duplicate fence beneath a stale, still-rendering one. +SECTION_BODY="${SECTION_BODY//$'\r'/}" + +# 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. +# +# The cap is per-section and blind to the others, so it only bounds the total +# if (section count × CAP) stays under 65536 with headroom for the header and +# the fences. There are 6 allowlisted sections (see below): 6 × 10000 = 60000, +# leaving ~5.5k. Re-derive this if a section is added, or the ${#NEW_BODY} +# guard below is the only thing left standing between a big comment and a 422. +CAP=10000 +if [ "${#SECTION_BODY}" -gt "$CAP" ]; then + echo "::warning::Section body is ${#SECTION_BODY} chars; truncating to $CAP." + NOTE=$'\n\n'"_…truncated; see the linked run for the full report._" + # Cut with headroom for the note, then close constructs the cut may have + # severed — an unterminated code fence or
makes GitHub render + # everything after it in this comment inside the open block, visually eating + # the neighbouring sections. The appended closers count against the cap too, + # so re-cut and re-balance until the finished body fits inside it. + CUT=$((CAP - ${#NOTE})) + while :; do + TRUNCATED="${SECTION_BODY:0:CUT}" + if [ $(( $(grep -c '^```' <<< "$TRUNCATED") % 2 )) -ne 0 ]; then + TRUNCATED="$TRUNCATED"$'\n''```' + fi + opens=$(grep -oi '' + closes=$((closes + 1)) + done + TRUNCATED="$TRUNCATED$NOTE" + if [ "${#TRUNCATED}" -le "$CAP" ]; then + break + fi + CUT=$((CUT - (${#TRUNCATED} - CAP))) + if [ "$CUT" -lt 0 ]; then + CUT=0 + fi + done + SECTION_BODY="$TRUNCATED" +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|inworld) ;; + *) echo "::error::Unknown section '${SECTION:-}'."; exit 2 ;; +esac + MARKER="" -HEADER="### 🚦 CI Status" +HEADER='### 🚦 CI Status' +# Retired header spellings, migrated to $HEADER whenever a section write runs. +OLD_HEADERS=( + '### Decentraland CI Status' + '### CI Status' +) BOT="github-actions[bot]" START="" END="" @@ -33,19 +113,27 @@ section_default() { build) printf '![Build](https://img.shields.io/badge/Build-Waiting-lightgrey?logo=unity&logoColor=white&style=for-the-badge)\n\n_Waiting for the build to start…_' ;; lint) printf '![Lint](https://img.shields.io/badge/Lint-Waiting-lightgrey?logo=jetbrains&logoColor=white&style=for-the-badge)\n\n_Waiting for lint to start…_' ;; tests) printf '![Tests](https://img.shields.io/badge/Tests-Waiting-lightgrey?logo=codecov&logoColor=white&style=for-the-badge)\n\n_Waiting for tests to start…_' ;; + automation) printf '![Automation](https://img.shields.io/badge/Automation-On%%20demand-lightgrey?logo=github&logoColor=white&style=for-the-badge)\n\n_On demand — comment `/visual-tests` on this PR to run the visual regression suite against its build._' ;; + performance) printf '![Performance](https://img.shields.io/badge/Performance-Waiting-lightgrey?logo=speedtest&logoColor=white&style=for-the-badge)\n\n_Bare-metal benchmarks run automatically after each successful build; results land in this section. Add the `perf_test` label to run the in-repo Unity performance suite instead (skips normal CI and blocks merge while set)._' ;; + inworld) printf '![InWorld](https://img.shields.io/badge/InWorld-Waiting-lightgrey?logo=unity&logoColor=white&style=for-the-badge)\n\n_Waiting for the InWorld suite…_' ;; esac } # One section, fenced by its start/end markers. wrap_section() { printf '\n%s\n' "$1" "$2" "$1"; } -# A fresh comment with every section defaulted to "waiting". +# A fresh comment with every always-present section defaulted to "waiting". +# inworld is deliberately absent: the suite only runs on release/hotfix PRs +# into main, and a permanent "waiting" row on every other PR would be noise — +# the append-missing-fence path below adds it the first time it reports. 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 +182,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 + # 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 @@ -104,25 +208,60 @@ for attempt in 1 2 3 4 5; do if [ -n "$COMMENT_ID" ]; then CURRENT_BODY=$(jq -r --arg id "$COMMENT_ID" '.[] | select(.id==($id|tonumber)) | .body' <<< "$(flatten_pages "$COMMENTS")") + CURRENT_BODY="${CURRENT_BODY//$'\r'/}" else 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 lacks + # our markers predates this section (e.g. it was written before the automation + # section existed) or the section is not part of the skeleton (inworld) — + # append an empty fence for just our section instead of resetting the whole + # comment and wiping the other sections' state. Two independent checks, not + # if/elif: a fresh skeleton needs the fence appended too when the section is + # a non-skeleton one, or replace_section would find no fence and the survive + # check would burn all 5 attempts. + if [ -z "$CURRENT_BODY" ]; then CURRENT_BODY="$(skeleton)" fi + # -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. + if ! grep -qxF "$START" <<< "$CURRENT_BODY"; then + CURRENT_BODY="$CURRENT_BODY"$'\n\n'"$(wrap_section "$SECTION" "$(section_default "$SECTION")")" + fi + + # Migrate comments created under a retired header spelling. + for OLD_HEADER in "${OLD_HEADERS[@]}"; do + CURRENT_BODY="${CURRENT_BODY/"$OLD_HEADER"/$HEADER}" + done NEW_BODY="$(replace_section "$CURRENT_BODY")" + # Near GitHub's 65536-char comment cap the write starts 422ing; the per- + # section CAP cannot see the other sections, so at least say why. + if [ "${#NEW_BODY}" -gt 65000 ]; then + echo "::warning::Unified comment is ${#NEW_BODY} chars — at/over GitHub's 65536 cap; a section needs a tighter cap." + fi + + # A failed write must land in the retry loop, not kill the script under + # set -e — that would fail this section's job and strand the section stale. if [ -z "$COMMENT_ID" ]; then - RESULT=$(jq -n --arg b "$NEW_BODY" '{body:$b}' \ - | gh api -X POST "/repos/$REPO/issues/$PR_NUMBER/comments" --input -) + if ! RESULT=$(jq -n --arg b "$NEW_BODY" '{body:$b}' \ + | gh api -X POST "/repos/$REPO/issues/$PR_NUMBER/comments" --input -); then + echo "Create failed (attempt $attempt); retrying." + sleep $((attempt * 2)) + continue + fi COMMENT_ID=$(jq -r '.id' <<< "$RESULT") else - jq -n --arg b "$NEW_BODY" '{body:$b}' \ - | gh api -X PATCH "/repos/$REPO/issues/comments/$COMMENT_ID" --input - >/dev/null + if ! jq -n --arg b "$NEW_BODY" '{body:$b}' \ + | gh api -X PATCH "/repos/$REPO/issues/comments/$COMMENT_ID" --input - >/dev/null; then + echo "Update failed (attempt $attempt); retrying." + sleep $((attempt * 2)) + continue + fi fi # Re-read and confirm our section landed on the surviving comment, and that no @@ -132,6 +271,16 @@ for attempt in 1 2 3 4 5; do RIDS=() while IFS= read -r line; do [ -n "$line" ] && RIDS+=("$line"); done <<< "$(marker_ids "$RECHECK")" LIVE_BODY=$(jq -r --arg id "$COMMENT_ID" '.[] | select(.id==($id|tonumber)) | .body' <<< "$(flatten_pages "$RECHECK")") + LIVE_BODY="${LIVE_BODY//$'\r'/}" + + # A write that landed on a younger duplicate is doomed: GC keeps the oldest, + # so this section's content would vanish with the duplicate. Retry on the + # survivor instead of declaring success on a comment about to be deleted. + if [ -n "${RIDS[0]:-}" ] && [ "$COMMENT_ID" != "${RIDS[0]}" ]; then + echo "Comment $COMMENT_ID lost the create race to ${RIDS[0]}; retrying on the survivor." + sleep $attempt + continue + fi # Success means our section landed on the comment we wrote — nothing more. # Duplicate collapsing is best-effort cleanup (the DELETE above may lack diff --git a/.github/actions/ucb-build-links/action.yml b/.github/actions/ucb-build-links/action.yml new file mode 100644 index 00000000000..79f178514ed --- /dev/null +++ b/.github/actions/ucb-build-links/action.yml @@ -0,0 +1,178 @@ +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 + install-source: + description: >- + Install source half of the info-artifact name + (unity_build_info__), matching the build + workflow's install_source matrix value. + required: false + default: launcher + +outputs: + rows: + description: >- + Two-column table rows, one per target, each pairing the GitHub job log, + the Unity Cloud build page, the Unity log artifact and a "⏱" duration + cell; a row omits the parts whose data was absent or invalid, and rows + built purely from the Actions API (job/log links) still render when the + info artifact itself is missing. + value: ${{ steps.fetch.outputs.rows }} + section: + description: >- + Standalone table (header + rows); empty only when no row could be built. + value: ${{ steps.fetch.outputs.section }} + windows-cell: + description: >- + The Windows row's 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 }} + INSTALL_SOURCE: ${{ inputs.install-source }} + 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/' 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="" + REPLY_QUEUE="" + REPLY_BUILD="" + if gh run download "$RUN_ID" \ + --repo "$REPO_FULL" \ + --name "unity_build_info_${target}_${INSTALL_SOURCE}" \ + --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_QUEUE=$(grep -m1 '^QUEUE_SECS=' "$dir/unity_cloud_build_info.env" | cut -d= -f2- || true) + REPLY_BUILD=$(grep -m1 '^BUILD_SECS=' "$dir/unity_cloud_build_info.env" | cut -d= -f2- || true) + [[ "$REPLY_ID" =~ ^[0-9]+$ ]] || REPLY_ID="" + [[ "$REPLY_URL" =~ $URL_RE ]] || REPLY_URL="" + [[ "$REPLY_QUEUE" =~ ^[0-9]+$ ]] || REPLY_QUEUE="" + [[ "$REPLY_BUILD" =~ ^[0-9]+$ ]] || REPLY_BUILD="" + 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}_${INSTALL_SOURCE}: $(tr '\n' ' ' < "${dir}.err")" + fi + } + + # Per-target GitHub job pages, from the trusted Actions API (jobs of the + # matrix job "Build ()"), 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":[]}') + + fmt_dur() { + # Digits-only input may still carry leading zeros; force base-10 so + # $(( )) can't parse a value like 0900 as (invalid) octal. + local s=$((10#$1)) + if [ "$s" -ge 3600 ]; then printf '%dh %dm' $((s/3600)) $((s%3600/60)) + elif [ "$s" -ge 60 ]; then printf '%dm %ds' $((s/60)) $((s%60)) + else printf '%ds' "$s"; fi + } + + 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}_${INSTALL_SOURCE}_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 + # Unlinked fallback must not say "#N": GitHub autolinks bare #N in + # comments to issue N. + parts+=("Unity Cloud build ${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 + if [ -n "$REPLY_BUILD" ]; then + t="⏱ $(fmt_dur "$REPLY_BUILD") build" + [ -n "$REPLY_QUEUE" ] && [ "$REPLY_QUEUE" -gt 0 ] && t+=" + $(fmt_dur "$REPLY_QUEUE") queue" + parts+=("$t") + 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="| Platform | Links & timing |"$'\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. + # Every value is emitted with exactly one trailing newline — a value glued to + # the delimiter line would make the runner miss the terminator entirely. + DELIM="UCB_EOF_${RANDOM}${RANDOM}_$$" + emit() { + local name="$1" val="$2" + echo "${name}<<${DELIM}" + if [ -n "$val" ]; then printf '%s\n' "${val%$'\n'}"; fi + echo "${DELIM}" + } + { + emit rows "$ROWS" + emit section "$SECTION" + emit windows-cell "$WINDOWS_CELL" + emit mac-cell "$MAC_CELL" + } >> "$GITHUB_OUTPUT" diff --git a/.github/workflows/build-profile-nightly.yml b/.github/workflows/build-profile-nightly.yml index e918f08ee91..494014b8972 100644 --- a/.github/workflows/build-profile-nightly.yml +++ b/.github/workflows/build-profile-nightly.yml @@ -71,6 +71,13 @@ jobs: name: Build Unity Cloud needs: [check-commits, get-info] if: needs.check-commits.outputs.should_build == 'true' + # Union of the called workflow's job-level grants — a caller caps its + # callee, so this keeps the call working if the repo default tightens. + permissions: + contents: read + statuses: write + actions: read + pull-requests: write uses: ./.github/workflows/build-unitycloud.yml with: profile: profile diff --git a/.github/workflows/build-release-main.yml b/.github/workflows/build-release-main.yml index ffece0ca056..a289ff313eb 100644 --- a/.github/workflows/build-release-main.yml +++ b/.github/workflows/build-release-main.yml @@ -38,6 +38,13 @@ jobs: build: name: Build Unity Cloud needs: get-info + # Union of the called workflow's job-level grants — a caller caps its + # callee, so this keeps the call working if the repo default tightens. + permissions: + contents: read + statuses: write + actions: read + pull-requests: write strategy: matrix: install_source: ['launcher', 'epic'] diff --git a/.github/workflows/build-unitycloud.yml b/.github/workflows/build-unitycloud.yml index 511cd8433f0..dca61904c14 100644 --- a/.github/workflows/build-unitycloud.yml +++ b/.github/workflows/build-unitycloud.yml @@ -216,6 +216,14 @@ jobs: prebuild: name: Prebuild runs-on: ubuntu-latest + # contents: read — checkout + the version composite's `git fetch origin main:main --tags` + # over the checkout's persisted credentials. + # statuses: write — "Skip build and test checks" posts 4 commit statuses via github-script. + # pull-requests: read — step-security/changed-files' REST fallback (GET /pulls/{n}/files). + permissions: + contents: read + statuses: write + pull-requests: read timeout-minutes: 20 # Skip when PR has 'perf_test' label (only performance tests should run) if: | @@ -558,6 +566,18 @@ jobs: runs-on: ubuntu-latest needs: prebuild if: needs.prebuild.outputs.should_build == 'true' + # contents: read — checkout + the size-budget step's GET /releases/latest. + # actions: read — build.py resolves its own job URL via GET /actions/runs/{run_id}/jobs. + # pull-requests: write — live CI status comment upsert (issue-comment POST/PATCH/DELETE + # on the PR via .github/actions/ci-status-comment/upsert-ci-status.sh). + # Safe on this `pull_request` (not `pull_request_target`) trigger: a fork PR gets a + # read-only GITHUB_TOKEN that `permissions:` cannot escalate, and cannot run this job + # anyway (the build needs UNITY_CLOUD_API_KEY/ORG_ID/PROJECT_ID, unavailable to forks); + # a same-repo branch is pushed by someone who already holds this capability with write access. + permissions: + contents: read + actions: read + pull-requests: write # Safety ceiling around the 450m retry budget + surrounding steps. timeout-minutes: 510 strategy: @@ -605,6 +625,10 @@ jobs: QUEUE_TIMEOUT: 14400 BUILD_TIMEOUT: 10800 TARGET: t_${{ matrix.target }} + # For the live PR status-comment update the moment the Unity-side + # 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 }} @@ -1001,6 +1025,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 @@ -1026,6 +1063,8 @@ jobs: build-gate: name: Build Gate (Windows + macOS) runs-on: ubuntu-latest + # Pure bash over needs.* context — no checkout, no token use. + permissions: {} needs: [prebuild, build] if: always() && github.event_name == 'pull_request' steps: diff --git a/.github/workflows/ci-scripts-tests.yml b/.github/workflows/ci-scripts-tests.yml new file mode 100644 index 00000000000..b4fd7d4e424 --- /dev/null +++ b/.github/workflows/ci-scripts-tests.yml @@ -0,0 +1,34 @@ +# ci-scripts-tests.yml +--- +name: CI Scripts Tests + +# Tests for the CI plumbing itself: build.py's pure helpers (including the +# URL_RE drift guard against ucb-build-links) and the unified status comment's +# upsert script against a stubbed gh. Cheap and network-free, so it runs on +# any PR touching these paths. +on: + pull_request: + paths: + - "scripts/cloudbuild/**" + - ".github/actions/ci-status-comment/**" + - ".github/actions/ucb-build-links/**" + - ".github/workflows/ci-scripts-tests.yml" + +permissions: + contents: read + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + with: + persist-credentials: false + + - name: Unit tests (build.py helpers) + run: | + pip install --quiet -r scripts/cloudbuild/requirements.txt + python3 -m unittest discover -s scripts/cloudbuild -v + + - name: Functional tests (upsert-ci-status.sh, stubbed gh) + run: bash .github/actions/ci-status-comment/test-upsert-ci-status.sh diff --git a/.github/workflows/create-release-branch.yml b/.github/workflows/create-release-branch.yml index 396aafeaf97..ce2831a29da 100644 --- a/.github/workflows/create-release-branch.yml +++ b/.github/workflows/create-release-branch.yml @@ -52,8 +52,14 @@ jobs: # SHA was already built by the push to dev — reuse that run's # artifacts instead of rebuilding. PR_NUMBER=$(gh pr view "$BRANCH_NAME" --json number --jq '.number') + # This standalone build-links comment predates the unified CI status + # comment, whose build section ALSO carries an img.shields.io/badge/Build + # badge under github-actions[bot]. Exclude it explicitly (by its + # marker) or a release PR that has one would either + # short-circuit here on its "Build-Success!" badge, or have its whole + # unified comment PATCHed away by post_comment below. EXISTING=$(gh api "repos/${GITHUB_REPOSITORY}/issues/${PR_NUMBER}/comments?per_page=100" \ - --jq '[.[] | select(.user.login == "github-actions[bot]") | select(.body | contains("img.shields.io/badge/Build"))] | .[0] // empty') + --jq '[.[] | select(.user.login == "github-actions[bot]") | select(.body | contains("img.shields.io/badge/Build")) | select(.body | contains("") | not)] | .[0] // empty') # A "Build Not Found" comment is upgraded in place on re-run once the # build exists; only a success comment short-circuits. diff --git a/.github/workflows/in-world-tests.yml b/.github/workflows/in-world-tests.yml index 197ab294049..bbad189b65f 100644 --- a/.github/workflows/in-world-tests.yml +++ b/.github/workflows/in-world-tests.yml @@ -1,8 +1,9 @@ name: In-World Tests # Dispatcher for the InWorld NUnit suite. The mechanics — Explorer install, -# AltTester, `mf explorer test`, Allure upload, PR comment — live in -# decentraland/explorer-automation's `run-inworld-suite.yml`. This file only +# AltTester, `mf explorer test`, Allure upload, and reporting into the unified +# CI status comment's InWorld section (standalone comment as fallback) — live +# in decentraland/explorer-automation's `run-inworld-suite.yml`. This file only # decides when to run and against which build, and shares its build resolution # with visual-regression.yml via .github/actions/resolve-explorer-build. # @@ -188,8 +189,9 @@ jobs: # explorer-automation that touch it cannot change release validation here. uses: decentraland/explorer-automation/.github/workflows/run-inworld-suite.yml@main # Must be a superset of run-inworld-suite.yml's own `permissions:`, or the - # call dies at startup. The write is the PR comment it posts; this is the - # only job that needs one, hence per job rather than at the top of the file. + # call dies at startup. The write is the CI status comment section it + # upserts (or the standalone comment it falls back to); this is the only + # job that needs one, hence per job rather than at the top of the file. permissions: contents: read pull-requests: write diff --git a/.github/workflows/pr-comment-artifact-url.yml b/.github/workflows/pr-comment-artifact-url.yml index 4ae385ce0f0..e1a82e78328 100644 --- a/.github/workflows/pr-comment-artifact-url.yml +++ b/.github/workflows/pr-comment-artifact-url.yml @@ -2,8 +2,9 @@ --- name: Comment Artifact URL on PR -# Writes only the "build" section of the unified CI status comment via the -# ci-status-comment composite action (build / lint / tests live in one comment). +# Writes the "build" section of the unified CI status comment via the +# ci-status-comment composite action, plus the "performance" section's dispatch +# status (the comment holds build / lint / tests / performance / automation). # # 'requested' -> reset the build section to "pending" the moment a new build # starts, so last build's download links never linger as stale. @@ -20,8 +21,11 @@ on: - "main" workflow_dispatch: permissions: - contents: read + contents: read pull-requests: write + # ucb-build-links reads the build run's artifacts and jobs cross-run — the + # same reason the sibling comment workflows grant it. + actions: read jobs: pre-validation: @@ -61,9 +65,9 @@ jobs: section: build github-token: ${{ github.token }} body: |- - ![Build](https://img.shields.io/badge/Build-Pending!-ffff00?logo=github&style=for-the-badge) + [![Build](https://img.shields.io/badge/Build-Pending!-yellow?logo=github&style=for-the-badge)](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.event.workflow_run.id }}) - New build in progress, come back later! + New build in progress — per-platform Unity Cloud links land here as soon as each build is created. check-build-ran: needs: pre-validation @@ -71,6 +75,7 @@ jobs: runs-on: ubuntu-latest outputs: build-ran: ${{ steps.check.outputs.build-ran }} + player-artifacts: ${{ steps.check.outputs.player-artifacts }} steps: - name: Check if build jobs actually ran id: check @@ -80,11 +85,28 @@ jobs: REPO: ${{ github.event.repository.name }} RUN_ID: ${{ github.event.workflow_run.id }} run: | - # Check if any build artifacts exist (they only exist when Build jobs ran) - ARTIFACT_COUNT=$(gh api "/repos/$OWNER/$REPO/actions/runs/$RUN_ID/artifacts" \ - --jq '[.artifacts[] | select(.name | startswith("Decentraland_"))] | length') - echo "Build artifact count: $ARTIFACT_COUNT" - if [ "$ARTIFACT_COUNT" -gt 0 ]; then + # player-artifacts: Decentraland_* zips exist. comment-success interpolates + # their artifact ids into download URLs, so the success/skipped split must + # keep gating on this and only this. + # build-ran: any evidence a Unity-side build started. unity_build_info_* is + # uploaded as soon as the build id is known, so a build that failed before + # producing player artifacts still posts a failure comment (with the Unity + # Cloud link) instead of leaving the comment stuck on "Pending". + # per_page=100: the default page holds 30 and a two-target run already + # uploads ~a dozen artifacts — unity_build_info_* falling off page 1 + # would read as build-ran=false, the exact stuck-on-Pending bug the + # flag exists to prevent. + NAMES=$(gh api "/repos/$OWNER/$REPO/actions/runs/$RUN_ID/artifacts?per_page=100" \ + --jq '[.artifacts[].name]') + PLAYER_COUNT=$(jq 'map(select(startswith("Decentraland_"))) | length' <<< "$NAMES") + INFO_COUNT=$(jq 'map(select(startswith("unity_build_info_"))) | length' <<< "$NAMES") + echo "Player artifact count: $PLAYER_COUNT; build info artifact count: $INFO_COUNT" + if [ "$PLAYER_COUNT" -gt 0 ]; then + echo "player-artifacts=true" >> "$GITHUB_OUTPUT" + else + echo "player-artifacts=false" >> "$GITHUB_OUTPUT" + fi + if [ "$PLAYER_COUNT" -gt 0 ] || [ "$INFO_COUNT" -gt 0 ]; then echo "build-ran=true" >> "$GITHUB_OUTPUT" else echo "build-ran=false" >> "$GITHUB_OUTPUT" @@ -92,7 +114,7 @@ jobs: comment-skipped: needs: [pre-validation, check-build-ran] - if: github.event.workflow_run.conclusion == 'success' && needs.pre-validation.outputs.pr-number != '' && needs.check-build-ran.outputs.build-ran == 'false' + if: github.event.workflow_run.conclusion == 'success' && needs.pre-validation.outputs.pr-number != '' && needs.check-build-ran.outputs.player-artifacts == 'false' runs-on: ubuntu-latest steps: - name: Checkout CI status action @@ -109,19 +131,47 @@ jobs: section: build github-token: ${{ github.token }} body: |- - ![Build](https://img.shields.io/badge/Build-Skipped-yellow?logo=unity&logoColor=white&style=for-the-badge) + [![Build](https://img.shields.io/badge/Build-Skipped-yellow?logo=unity&logoColor=white&style=for-the-badge)](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.event.workflow_run.id }}) Build skipped — no changes detected under `Explorer/`. + # A cancelled run matches neither the success nor the failure gate, and the + # build job's live writer may have left an In-progress badge and rows up — + # without this the comment claims a build is running forever. + comment-cancelled: + needs: pre-validation + if: github.event.action == 'completed' && github.event.workflow_run.conclusion == 'cancelled' && needs.pre-validation.outputs.pr-number != '' + runs-on: ubuntu-latest + steps: + - name: Checkout CI status action + uses: actions/checkout@v6 + with: + sparse-checkout: .github/actions/ci-status-comment + sparse-checkout-cone-mode: false + persist-credentials: false + + - name: Post cancelled build section + uses: ./.github/actions/ci-status-comment + with: + pr-number: ${{ needs.pre-validation.outputs.pr-number }} + section: build + github-token: ${{ github.token }} + body: |- + [![Build](https://img.shields.io/badge/Build-Cancelled-lightgrey?logo=unity&logoColor=white&style=for-the-badge)](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.event.workflow_run.id }}) + + Build cancelled — push a new commit or re-run the workflow to refresh this section. + comment-success: needs: [pre-validation, check-build-ran] - if: github.event.workflow_run.conclusion == 'success' && needs.pre-validation.outputs.pr-number != '' && needs.check-build-ran.outputs.build-ran == 'true' + if: github.event.workflow_run.conclusion == 'success' && needs.pre-validation.outputs.pr-number != '' && needs.check-build-ran.outputs.player-artifacts == 'true' runs-on: ubuntu-latest steps: - name: Checkout CI status action uses: actions/checkout@v6 with: - sparse-checkout: .github/actions/ci-status-comment + sparse-checkout: | + .github/actions/ci-status-comment + .github/actions/ucb-build-links sparse-checkout-cone-mode: false persist-credentials: false @@ -167,6 +217,7 @@ jobs: SHORT_SHA=$(echo "$HEAD_SHA" | cut -c1-7) echo "Short SHA: $SHORT_SHA" + echo "SHORT_SHA=$SHORT_SHA" >> "$GITHUB_ENV" SAFE_BRANCH_NAME=$(jq -r '.pull_requests[0].head.ref // .head_branch' <<< "$WORKFLOW_RUN_EVENT_OBJ") echo "Safe Branch Name: $SAFE_BRANCH_NAME" @@ -196,6 +247,7 @@ jobs: echo "BUILD_DATE=$BUILD_DATE" >> "$GITHUB_ENV" - name: Download size reports + continue-on-error: true env: GITHUB_TOKEN: ${{ github.token }} OWNER: ${{ github.repository_owner }} @@ -230,6 +282,52 @@ jobs: echo "SIZE_REPORT=" >> "$GITHUB_ENV" fi + - name: Fetch Unity Cloud build links + continue-on-error: true + id: ucb + uses: ./.github/actions/ucb-build-links + with: + run-id: ${{ env.PREVIOUS_JOB_ID }} + github-token: ${{ github.token }} + install-source: launcher + + # One row per platform: GitHub job · Unity Cloud build · Unity log · + # build duration · zip downloads. Composed here rather than inline in the + # body so a link whose id could not be resolved is dropped instead of + # rendering broken. + - name: Compose platform rows + continue-on-error: true + env: + WINDOWS_CELL: ${{ steps.ucb.outputs.windows-cell }} + MAC_CELL: ${{ steps.ucb.outputs.mac-cell }} + S3_BASE: ${{ format('{0}/{1}', vars.EXPLORER_TEAM_S3_BUCKET_PUBLIC_URL, env.ARTIFACT_S3_DESTINATION_PATH) }} + # Set by "Get Artifact and Pull request info" above via $GITHUB_ENV; listed + # here so the coupling between the two steps is visible without tracing it. + SUITE_ID: ${{ env.SUITE_ID }} + WINDOWS_ARTIFACT_ID: ${{ env.WINDOWS_ARTIFACT_ID }} + MAC_ARTIFACT_ID: ${{ env.MAC_ARTIFACT_ID }} + GITHUB_SERVER_URL: ${{ github.server_url }} + GITHUB_REPOSITORY: ${{ github.repository }} + run: | + set -euo pipefail + compose_row() { + local label="$1" cell="$2" art_id="$3" file="$4" + local parts=() + [ -n "$cell" ] && parts+=("$cell") + [ -n "$art_id" ] && parts+=("[Download .zip](${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/suites/${SUITE_ID}/artifacts/${art_id})") + parts+=("[.zip via S3](${S3_BASE}/${file})") + local joined + joined=$(printf '%s · ' "${parts[@]}") + printf '| %s | %s |\n' "$label" "${joined% · }" + } + DELIM="PLATFORM_ROWS_EOF_${RANDOM}${RANDOM}_$$" + { + echo "PLATFORM_ROWS<<$DELIM" + compose_row "Windows" "$WINDOWS_CELL" "${WINDOWS_ARTIFACT_ID:-}" "Decentraland_windows64.zip" + compose_row "Mac" "$MAC_CELL" "${MAC_ARTIFACT_ID:-}" "Decentraland_macos.zip" + echo "$DELIM" + } >> "$GITHUB_ENV" + - name: Update build section uses: ./.github/actions/ci-status-comment with: @@ -237,19 +335,14 @@ jobs: section: build github-token: ${{ github.token }} body: |- - ![Build](https://img.shields.io/badge/Build-Success!-3fb950?logo=unity&logoColor=white&style=for-the-badge) + [![Build](https://img.shields.io/badge/Build-Success!-3fb950?logo=unity&logoColor=white&style=for-the-badge)](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ env.PREVIOUS_JOB_ID }}) - Windows and Mac build successful in Unity Cloud! You can find a link to the downloadable artifact below. + Windows and Mac built successfully in Unity Cloud. - | Name | Link | - | -------- | ----------------------- | - | Commit | ${{ env.HEAD_SHA }} | - | Logs | ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ env.PREVIOUS_JOB_ID }} | - | Download Windows | ${{ github.server_url }}/${{ github.repository }}/suites/${{ env.SUITE_ID }}/artifacts/${{ env.WINDOWS_ARTIFACT_ID }} | - | Download Windows S3 | ${{ format('{0}/{1}/Decentraland_windows64.zip', vars.EXPLORER_TEAM_S3_BUCKET_PUBLIC_URL, env.ARTIFACT_S3_DESTINATION_PATH) }} | - | Download Mac | ${{ github.server_url }}/${{ github.repository }}/suites/${{ env.SUITE_ID }}/artifacts/${{ env.MAC_ARTIFACT_ID }} | - | Download Mac S3 | ${{ format('{0}/{1}/Decentraland_macos.zip', vars.EXPLORER_TEAM_S3_BUCKET_PUBLIC_URL, env.ARTIFACT_S3_DESTINATION_PATH) }} | - | Built on | ${{ env.BUILD_DATE }} | + | Name | Links & timing | + | -------- | ----------------------- | + | Build | [`${{ env.SHORT_SHA }}`](${{ github.server_url }}/${{ github.repository }}/commit/${{ env.HEAD_SHA }}) · [Logs](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ env.PREVIOUS_JOB_ID }}) · built ${{ env.BUILD_DATE }} | + ${{ env.PLATFORM_ROWS }} ${{ env.SIZE_REPORT }} @@ -259,6 +352,7 @@ jobs: run: gh release view -R $GITHUB_REPOSITORY --json tagName --template "RELEASE_TAG={{.tagName}}" >> $GITHUB_ENV - name: Trigger performance test + id: perf_dispatch uses: peter-evans/repository-dispatch@v4 with: repository: decentraland/performance-testing @@ -278,6 +372,78 @@ jobs: "mac_base_build_url": "${{ vars.EXPLORER_TEAM_S3_BUCKET_PUBLIC_URL }}/@dcl/unity-explorer/releases/${{ env.RELEASE_TAG }}/Decentraland_macos.zip" } + # The PAT's remaining lifetime comes back as a response header on any API + # call it makes; surfacing it in the status comment replaces a separate + # expiry canary — an expiring token must warn somewhere a human looks. + - name: Probe performance PAT expiry + # Purely decorative — its whole output is one optional sentence, so it + # must never gate the dispatched/dispatch-failed section writes below. + if: steps.perf_dispatch.outcome == 'success' + continue-on-error: true + env: + PAT: ${{ secrets.PERFORMANCE_TESTING_PAT }} + run: | + set -euo pipefail + # The expiry header rides every authenticated call; /rate_limit spends + # no quota and names no repo that could drift. gh reads the token from + # the environment, keeping it out of any process's argv. head -1 keeps + # $GITHUB_ENV single-line even if the response ever repeats the header. + exp=$(GH_TOKEN="$PAT" timeout 15 gh api --include --method HEAD /rate_limit 2>/dev/null \ + | tr -d '\r' | grep -i '^github-authentication-token-expiration:' | head -1 | cut -d' ' -f2- || true) + msg="" + if [ -n "$exp" ]; then + exp_s=$(date -d "$exp" +%s 2>/dev/null || echo 0) + if [ "$exp_s" -gt 0 ]; then + days=$(( (exp_s - $(date +%s)) / 86400 )) + echo "PERFORMANCE_TESTING_PAT expires in $days days ($exp)" + if [ "$days" -lt 30 ]; then + msg="⚠️ \`PERFORMANCE_TESTING_PAT\` expires in **$days days** ($exp) — rotate it before benchmark dispatches start failing." + fi + fi + else + echo "::warning::No github-authentication-token-expiration header returned — PAT expiry cannot be monitored." + fi + echo "PAT_EXPIRY_WARNING=$msg" >> "$GITHUB_ENV" + + # repository_dispatch is fire-and-forget (no run id comes back), so this + # links the target workflow's run list; the benchmark itself rewrites + # this section with its report when it finishes (or falls back to a + # standalone perf-test-summary comment when the section write fails). + - name: Mark performance section as dispatched + if: steps.perf_dispatch.outcome == 'success' + uses: ./.github/actions/ci-status-comment + with: + pr-number: ${{ needs.pre-validation.outputs.pr-number }} + section: performance + github-token: ${{ github.token }} + body: |- + [![Performance](https://img.shields.io/badge/Performance-Dispatched!-yellow?logo=speedtest&logoColor=white&style=for-the-badge)](https://github.com/decentraland/performance-testing/actions/workflows/unity-explorer.yaml) + + 🏁 Bare-metal benchmark dispatched for this build ([run queue](https://github.com/decentraland/performance-testing/actions/workflows/unity-explorer.yaml)) — results across the runner fleet land in this section when it finishes. + + Latest build wins this section — it also replaces any earlier `perf_test`-suite verdict. + + ${{ env.PAT_EXPIRY_WARNING }} + + # A failing dispatch (e.g. an expired PERFORMANCE_TESTING_PAT) turns this + # job red but leaves no trace on the PR; surface it in the performance + # section so it cannot go unnoticed. + # outcome != 'success' rather than == 'failure': anything failing between + # the checkout and the dispatch (Find latest release, the build-section + # upsert) skips perf_dispatch, and a skipped dispatch is the same silence + # on the PR as a failed one. + - name: Mark performance section as dispatch-failed + if: failure() && steps.perf_dispatch.outcome != 'success' + uses: ./.github/actions/ci-status-comment + with: + pr-number: ${{ needs.pre-validation.outputs.pr-number }} + section: performance + github-token: ${{ github.token }} + body: |- + [![Performance](https://img.shields.io/badge/Performance-Dispatch%20failed-ff0000?logo=speedtest&logoColor=white&style=for-the-badge)](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) + + ❌ Could not dispatch the bare-metal benchmark (the job failed before or during the dispatch) — see the [step log](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}). If it says "Repository not found, OR token has insufficient permissions", the `PERFORMANCE_TESTING_PAT` secret has expired and needs to be rotated. + comment-failed: needs: [pre-validation, check-build-ran] if: github.event.workflow_run.conclusion == 'failure' && needs.pre-validation.outputs.pr-number != '' && needs.check-build-ran.outputs.build-ran == 'true' @@ -286,10 +452,21 @@ jobs: - name: Checkout CI status action uses: actions/checkout@v6 with: - sparse-checkout: .github/actions/ci-status-comment + sparse-checkout: | + .github/actions/ci-status-comment + .github/actions/ucb-build-links sparse-checkout-cone-mode: false persist-credentials: false + - name: Fetch Unity Cloud build links + continue-on-error: true + id: ucb + uses: ./.github/actions/ucb-build-links + with: + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ github.token }} + install-source: launcher + - name: Update build section uses: ./.github/actions/ci-status-comment with: @@ -297,7 +474,9 @@ jobs: section: build github-token: ${{ github.token }} body: |- - ![Build](https://img.shields.io/badge/Build-Failed!-ff0000?logo=unity&logoColor=white&style=for-the-badge) + [![Build](https://img.shields.io/badge/Build-Failed!-ff0000?logo=unity&logoColor=white&style=for-the-badge)](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.event.workflow_run.id }}) - Build failed! Check the logs to see what went wrong. + Build failed! Check the [logs](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.event.workflow_run.id }}) to see what went wrong. If the error repeats please consider the `clean-build` tag. + + ${{ steps.ucb.outputs.section }} diff --git a/.github/workflows/pr-comment-perf.yml b/.github/workflows/pr-comment-perf.yml new file mode 100644 index 00000000000..487fe9fd0e5 --- /dev/null +++ b/.github/workflows/pr-comment-perf.yml @@ -0,0 +1,133 @@ +# pr-comment-perf.yml +--- +name: Comment Performance Results on PR + +# Runs in the trusted base-repo context after the (possibly fork) "Unity +# Performance Test" run completes, and writes the "performance" section of the +# unified CI status comment. Mirrors pr-comment-test-failures.yml. +# +# "Unity Performance Test" fires on every PR event but its job gates on the +# 'perf_test' label, so most runs conclude 'success' with the job skipped — +# those must not touch the section (it belongs to the bare-metal benchmark +# status written by pr-comment-artifact-url.yml on label-less PRs). +on: + workflow_run: + types: + - "completed" + workflows: + - "Unity Performance Test" + workflow_dispatch: + +permissions: + contents: read + pull-requests: write + actions: read + +jobs: + comment: + runs-on: ubuntu-latest + steps: + - name: Resolve PR number + id: pr + env: + WORKFLOW_RUN_EVENT_OBJ: ${{ toJSON(github.event.workflow_run) }} + GITHUB_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + run: | + PR_NUMBER=$(jq -r '.pull_requests[0].number' <<< "$WORKFLOW_RUN_EVENT_OBJ") + # workflow_run leaves pull_requests empty for fork-origin PRs; the + # commit->PRs lookup still resolves those, so a fork's perf verdict + # is not silently dropped. + if [[ -z "$PR_NUMBER" || "$PR_NUMBER" == "null" ]]; then + HEAD_SHA=$(jq -r '.head_sha // empty' <<< "$WORKFLOW_RUN_EVENT_OBJ") + if [ -n "$HEAD_SHA" ]; then + PR_NUMBER=$(gh api "/repos/$REPO/commits/$HEAD_SHA/pulls" --jq '.[0].number // empty' 2>/dev/null || true) + fi + fi + echo "PR number: ${PR_NUMBER:-}" + if [[ -z "$PR_NUMBER" || "$PR_NUMBER" == "null" ]]; then + echo "No PR associated with this run, skipping." + echo "pr-number=" >> "$GITHUB_OUTPUT" + else + echo "pr-number=$PR_NUMBER" >> "$GITHUB_OUTPUT" + fi + + - name: Check the performance job actually ran + id: ran + if: steps.pr.outputs.pr-number != '' + env: + GITHUB_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + RUN_ID: ${{ github.event.workflow_run.id }} + run: | + set -euo pipefail + CONCLUSION=$(gh api "/repos/$REPO/actions/runs/$RUN_ID/jobs?per_page=100" \ + --jq '[.jobs[] | select(.name | startswith("Performance Test"))][0].conclusion // "absent"') + echo "Performance job conclusion: $CONCLUSION" + if [ "$CONCLUSION" = "skipped" ] || [ "$CONCLUSION" = "absent" ]; then + echo "ran=false" >> "$GITHUB_OUTPUT" + else + echo "ran=true" >> "$GITHUB_OUTPUT" + fi + + - name: Compose comment body + id: body + if: steps.pr.outputs.pr-number != '' && steps.ran.outputs.ran == 'true' + env: + GITHUB_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + RUN_ID: ${{ github.event.workflow_run.id }} + RUN_URL: ${{ github.event.workflow_run.html_url }} + CONCLUSION: ${{ github.event.workflow_run.conclusion }} + run: | + set -euo pipefail + BADGE_STYLE="?logo=speedtest&logoColor=white&style=for-the-badge" + case "$CONCLUSION" in + success) BADGE="https://img.shields.io/badge/Performance-Passed!-3fb950${BADGE_STYLE}"; MSG="✅ Unity performance suite finished." ;; + failure) BADGE="https://img.shields.io/badge/Performance-Failed!-ff0000${BADGE_STYLE}"; MSG="❌ Unity performance suite failed." ;; + *) BADGE="https://img.shields.io/badge/Performance-Cancelled-lightgrey${BADGE_STYLE}"; MSG="Unity performance suite did not finish (\`$CONCLUSION\`)." ;; + esac + + # Artifact download links come from the trusted Actions API. + ARTS_JSON=$(gh api "/repos/$REPO/actions/runs/$RUN_ID/artifacts?per_page=100" 2>/dev/null || echo '{"artifacts":[]}') + links="" + for entry in "results (JSON):Performance test results (JSON)" "benchmark report (PDF):Performance benchmark report (PDF)"; do + label="${entry%%:*}" + art_name="${entry#*:}" + art_id=$(jq -r --arg n "$art_name" '.artifacts[]? | select(.name==$n) | .id // empty' <<< "$ARTS_JSON" | head -1) + [ -n "$art_id" ] && links="$links · [$label](https://github.com/$REPO/actions/runs/$RUN_ID/artifacts/$art_id)" + done + + DELIM="EOF_${RANDOM}${RANDOM}_$$" + { + echo "body<<$DELIM" + echo "[![Performance]($BADGE)]($RUN_URL)" + echo "" + echo "$MSG The benchmark report is rendered on the [run summary]($RUN_URL)." + echo "" + echo "Written by the \`perf_test\`-label suite; the next build's benchmark dispatch replaces it." + echo "" + if [ -n "$links" ]; then + echo "Download:${links#" ·"}" + else + echo "No result artifacts were produced — check the [run]($RUN_URL)." + fi + echo "$DELIM" + } >> "$GITHUB_OUTPUT" + + - name: Checkout CI status action + if: steps.body.outcome == 'success' && steps.ran.outputs.ran == 'true' + uses: actions/checkout@v6 + with: + sparse-checkout: .github/actions/ci-status-comment + sparse-checkout-cone-mode: false + persist-credentials: false + + - name: Update performance section + if: steps.body.outcome == 'success' && steps.ran.outputs.ran == 'true' + uses: ./.github/actions/ci-status-comment + with: + pr-number: ${{ steps.pr.outputs.pr-number }} + section: performance + github-token: ${{ github.token }} + body: ${{ steps.body.outputs.body }} diff --git a/.github/workflows/pr-comment-test-failures.yml b/.github/workflows/pr-comment-test-failures.yml index e8dfb4cb94b..5b107cddcbc 100644 --- a/.github/workflows/pr-comment-test-failures.yml +++ b/.github/workflows/pr-comment-test-failures.yml @@ -72,9 +72,18 @@ jobs: run: | set -euo pipefail - # Job list of the originating "Unity Test" run, used to deep-link a - # crashed/timed-out suite straight to its job page in the warning line. - JOBS_JSON=$(gh api "/repos/$REPO/actions/runs/$WORKFLOW_RUN_ID/jobs" 2>/dev/null || echo '{"jobs":[]}') + # Job and artifact lists of the originating "Unity Test" run, used to + # deep-link each suite to its job page and the result artifacts + # (NUnit XML + editor logs) to their download links. + JOBS_JSON=$(gh api "/repos/$REPO/actions/runs/$WORKFLOW_RUN_ID/jobs?per_page=100" 2>/dev/null || echo '{"jobs":[]}') + ARTS_JSON=$(gh api "/repos/$REPO/actions/runs/$WORKFLOW_RUN_ID/artifacts?per_page=100" 2>/dev/null || echo '{"artifacts":[]}') + + # Same h/m/s convention as fmt_dur in ucb-build-links/action.yml and the + # lint footer in pr-comment-warnings.yml — keep the three in lockstep. + fmt_secs() { + awk -v s="$1" 'BEGIN { s=int(s+0.5); h=int(s/3600); m=int(s%3600/60); r=s%60; + if (h>0) printf("%dh %dm", h, m); else if (m>0) printf("%dm %ds", m, r); else printf("%ds", r) }' + } declare -A DISPLAY=( [editmode]=EditMode [playmode]=PlayMode ) @@ -82,6 +91,7 @@ jobs: rows="" warnings="" failed_list="" + slowest_list="" total_failed=0 for mode in editmode playmode; do @@ -89,13 +99,23 @@ jobs: [ -f "$file" ] || continue disp=${DISPLAY[$mode]} + job_url=$(jq -r --arg n "Test ($mode)" '.jobs[]? | select(.name==$n) | .html_url // empty' <<< "$JOBS_JSON" | head -1) + if [ -n "$job_url" ]; then disp_cell="[$disp]($job_url)"; else disp_cell="$disp"; fi + + # Wall time of the suite's job (checkout + Unity licensing + import + + # tests), from the trusted Actions API — pairs with the test-sum Time + # column so setup overhead is visible. + job_secs=$(jq -r --arg n "Test ($mode)" \ + '[.jobs[]? | select(.name==$n and .completed_at != null and .started_at != null) + | ((.completed_at|fromdateiso8601) - (.started_at|fromdateiso8601))] | first // empty' <<< "$JOBS_JSON") + if [[ "$job_secs" =~ ^[0-9]+$ ]]; then job_dur=$(fmt_secs "$job_secs"); else job_dur="—"; fi + # A suite that produced no result XML crashed or timed out before finishing. # Surface it as its own state instead of silently contributing 0 to a green total. if [ "$(jq -r '.hasResults' "$file")" != "true" ]; then status=incomplete - job_url=$(jq -r --arg n "Test ($mode)" '.jobs[]? | select(.name==$n) | .html_url' <<< "$JOBS_JSON" | head -1) [ -n "$job_url" ] || job_url="$WORKFLOW_RUN_URL" - rows="$rows| $disp | ⚠️ No results | — | — | — |"$'\n' + rows="$rows| $disp_cell | ⚠️ No results | — | — | — | — | $job_dur |"$'\n' warnings="$warnings⚠️ **$disp** produced no results — the run likely crashed or timed out before finishing. Check the [\`Unity Test / Test ($mode)\`]($job_url) job."$'\n\n' continue fi @@ -107,33 +127,64 @@ jobs: f=$(jq -r '.failed | length' "$file") s=$((t - p - f)); if [ "$s" -lt 0 ]; then s=0; fi + # Suite duration + slowest tests come from the same untrusted artifact: + # duration must be numeric before it reaches awk, and slowest entries are + # type-checked in jq with names flattened to a single line. Names render + # inside inline code (backticks/pipes stripped) so a name shaped like + # markdown — a link, an , a
— reads as text instead of + # rendering as first-party comment furniture. + d=$(jq -r '.duration // empty' "$file") + if [[ "$d" =~ ^[0-9]+(\.[0-9]+)?$ ]]; then dur=$(fmt_secs "$d"); else dur="—"; fi + slow=$(jq -r --arg mode "$mode" \ + '.slowest[]? | select((.seconds|type=="number") and (.name|type=="string")) | "- [\($mode)] \(.seconds)s `\(.name | gsub("[\r\n`|]"; " "))`"' \ + "$file" 2>/dev/null || true) + [ -n "$slow" ] && slowest_list="$slowest_list$slow"$'\n' + if [ "$f" -gt 0 ]; then if [ "$status" = "passed" ]; then status=failed; fi total_failed=$((total_failed + f)) - rows="$rows| $disp | ❌ $f failed | $p | $f | $s |"$'\n' - names=$(jq -r --arg mode "$mode" '.failed[] | "- [\($mode)] \(. | gsub("[\r\n]"; " "))"' "$file") + rows="$rows| $disp_cell | ❌ $f failed | $p | $f | $s | $dur | $job_dur |"$'\n' + names=$(jq -r --arg mode "$mode" '.failed[] | "- [\($mode)] `\(. | gsub("[\r\n`|]"; " "))`"' "$file") failed_list="$failed_list$names"$'\n' else - rows="$rows| $disp | ✅ Passed | $p | 0 | $s |"$'\n' + rows="$rows| $disp_cell | ✅ Passed | $p | 0 | $s | $dur | $job_dur |"$'\n' fi done + # The only unbounded list in this body. The composite's env transport + # rejects any single env string over 128KiB (E2BIG) before its own + # 20k truncation can run, so bound it at composition. + if [ "${#failed_list}" -gt 60000 ]; then + failed_list="${failed_list:0:60000}"$'\n'"- …list truncated — see the run for the full set."$'\n' + fi + case "$status" in incomplete) badge="https://img.shields.io/badge/Tests-Incomplete-d29922?logo=codecov&logoColor=white&style=for-the-badge"; headline="$warnings" ;; failed) badge="https://img.shields.io/badge/Tests-Failed!-ff0000?logo=codecov&logoColor=white&style=for-the-badge"; headline="Some Unity tests failed ❌" ;; *) badge="https://img.shields.io/badge/Tests-Passed!-3fb950?logo=codecov&logoColor=white&style=for-the-badge"; headline="All Unity tests passed ✅" ;; esac + # Artifact download links (ids come from the trusted Actions API). + art_footer="" + for entry in "editmode:Test results (editmode)" "playmode:Test results (playmode)"; do + mode="${entry%%:*}" + art_name="${entry#*:}" + art_id=$(jq -r --arg n "$art_name" '.artifacts[]? | select(.name==$n) | .id // empty' <<< "$ARTS_JSON" | head -1) + [ -n "$art_id" ] && art_footer="$art_footer · [$mode](https://github.com/$REPO/actions/runs/$WORKFLOW_RUN_ID/artifacts/$art_id)" + done + DELIM="EOF_$(uuidgen)" { echo "body<<$DELIM" - echo "![Tests]($badge)" + echo "[![Tests]($badge)]($WORKFLOW_RUN_URL)" echo "" printf '%s\n' "$headline" echo "" - echo "| TESTS SUITE | Result | Passed | Failed | Skipped |" - echo "| ----------- | ------ | -----: | -----: | ------: |" + echo "| TESTS SUITE | Result | Passed | Failed | Skipped | Tests time | Job time |" + echo "| ----------- | ------ | -----: | -----: | ------: | ---: | ---: |" printf '%s' "$rows" + echo "" + echo "Tests time sums the test cases; Job time is the job's wall clock including checkout, licensing and asset import." if [ "$total_failed" -gt 0 ]; then echo "" echo "
Failed tests ($total_failed)" @@ -142,6 +193,20 @@ jobs: echo "" echo "
" fi + if [ -n "$slowest_list" ]; then + echo "" + echo "
Slowest tests" + echo "" + printf '%s' "$slowest_list" + echo "" + echo "
" + fi + echo "" + if [ -n "$art_footer" ]; then + echo "Full report: [run summary]($WORKFLOW_RUN_URL) · results + editor logs:${art_footer#" ·"}" + else + echo "Full report: [run summary]($WORKFLOW_RUN_URL)" + fi echo "$DELIM" } >> "$GITHUB_OUTPUT" diff --git a/.github/workflows/pr-comment-warnings.yml b/.github/workflows/pr-comment-warnings.yml index d55336245b1..77175a1e41a 100644 --- a/.github/workflows/pr-comment-warnings.yml +++ b/.github/workflows/pr-comment-warnings.yml @@ -62,7 +62,7 @@ jobs: section: lint github-token: ${{ github.token }} body: |- - ![Lint](https://img.shields.io/badge/Lint-Pending!-ffff00?logo=jetbrains&logoColor=white&style=for-the-badge) + [![Lint](https://img.shields.io/badge/Lint-Pending!-yellow?logo=jetbrains&logoColor=white&style=for-the-badge)](${{ github.event.workflow_run.html_url }}) Lint in progress, come back later! @@ -123,6 +123,9 @@ jobs: ALLOW_EQUAL: ${{ steps.result.outputs.allow-equal }} CONCLUSION: ${{ github.event.workflow_run.conclusion }} RUN_URL: ${{ github.event.workflow_run.html_url }} + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + RUN_ID: ${{ github.event.workflow_run.id }} run: | DELIM="EOF_$(uuidgen)" BADGE_STYLE="?logo=jetbrains&logoColor=white&style=for-the-badge" @@ -130,9 +133,10 @@ jobs: BLOCKED=0 # COUNT/BASELINE come from the artifact produced by the untrusted pull_request job - # (fork-controlled). This step has no `set -e`, and bash's `[` errors out on a - # non-numeric operand and falls through - never let such a value reach a comparison - # or a bash arithmetic context ($(( )) evaluates its operands as expressions). + # (fork-controlled). This step runs under Actions' default `bash -e {0}`: `[` + # errors on a non-numeric operand (falling through inside `if`, aborting the step + # elsewhere) - never let such a value reach a comparison or a bash arithmetic + # context ($(( )) evaluates its operands as expressions). [[ "$COUNT" =~ ^[0-9]+$ ]] || COUNT=0 [[ "$BASELINE" =~ ^[0-9]+$ ]] || BASELINE="" @@ -205,12 +209,40 @@ jobs: fi fi + # Footer: the lint job run plus the full InspectCode report artifact (the + # findings list above is capped; the artifact has everything). The id comes + # from the trusted Actions API. + FOOTER="" + if [ "$FOUND" = "true" ]; then + ART_ID=$(gh api "/repos/$REPO/actions/runs/$RUN_ID/artifacts?per_page=100" \ + --jq '.artifacts[] | select(.name=="csharp-lint-reports") | .id' 2>/dev/null | head -1) + # Wall time of the Lint job, from the trusted Actions API. An API failure + # degrades to "no duration" (empty) instead of aborting the step under -e. + LINT_SECS=$(gh api "/repos/$REPO/actions/runs/$RUN_ID/jobs?per_page=100" \ + --jq '[.jobs[] | select(.name=="Lint" and .completed_at != null and .started_at != null) + | ((.completed_at|fromdateiso8601) - (.started_at|fromdateiso8601))] | first // empty' 2>/dev/null || true) + FOOTER="[Lint run]($RUN_URL)" + [ -n "$ART_ID" ] && FOOTER="$FOOTER · [full InspectCode report](https://github.com/$REPO/actions/runs/$RUN_ID/artifacts/$ART_ID)" + if [[ "$LINT_SECS" =~ ^[0-9]+$ ]]; then + # Same h/m/s convention as fmt_dur in ucb-build-links/action.yml. + if [ "$LINT_SECS" -ge 3600 ]; then took="$((LINT_SECS/3600))h $((LINT_SECS%3600/60))m" + elif [ "$LINT_SECS" -ge 60 ]; then took="$((LINT_SECS/60))m $((LINT_SECS%60))s" + else took="${LINT_SECS}s"; fi + FOOTER="$FOOTER · took $took" + fi + FOOTER="$FOOTER" + fi + { echo "body<<$DELIM" - echo "![Lint]($BADGE)" + echo "[![Lint]($BADGE)]($RUN_URL)" echo "" echo "$MSG" [ -n "$DETAILS" ] && cat "$DETAILS" + if [ -n "$FOOTER" ]; then + echo "" + echo "$FOOTER" + fi echo "$DELIM" } >> "$GITHUB_OUTPUT" diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 471bc563f9e..7c9c121653f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -814,7 +814,7 @@ jobs: TEST_MODE: ${{ matrix.testMode }} run: | python3 - <<'PY' - import glob, json, os, xml.etree.ElementTree as ET + import glob, json, math, os, xml.etree.ElementTree as ET artifacts_path = os.environ["ARTIFACTS_PATH"] test_mode = os.environ["TEST_MODE"] @@ -823,6 +823,8 @@ jobs: total = 0 passed = 0 failed = [] + duration = 0.0 + timings = [] for path in xml_files: try: @@ -834,16 +836,37 @@ jobs: if case_result is None: continue total += 1 + try: + seconds = float(test_case.get("duration") or 0) + except ValueError: + seconds = 0.0 + # float() admits nan/inf/1e999 without raising, and json.dump + # would then emit bare NaN/Infinity — invalid JSON that aborts + # the consumer's very first jq read of this file. + if not math.isfinite(seconds): + seconds = 0.0 + duration += seconds + timings.append((seconds, test_case.get("fullname") or test_case.get("name"))) if case_result == "Passed": passed += 1 elif case_result == "Failed": - failed.append(test_case.get("fullname") or test_case.get("name")) + # "(unnamed)" keeps the set homogeneous — one None among + # strings makes sorted() raise and kills the whole file. + failed.append(test_case.get("fullname") or test_case.get("name") or "(unnamed)") result = { "hasResults": len(xml_files) > 0, "total": total, "passed": passed, "failed": sorted(set(failed)), + # The per-case clamp keeps each addend (and the slowest list) + # finite, but a sum of finite doubles can still overflow to inf — + # clamp again at the one point the accumulator is serialised. + "duration": round(duration, 1) if math.isfinite(duration) else 0.0, + "slowest": [ + {"name": name, "seconds": round(seconds, 1)} + for seconds, name in sorted(timings, key=lambda t: -t[0])[:10] + ], } with open(f"failed-tests-{test_mode}.json", "w") as f: diff --git a/.github/workflows/visual-regression.yml b/.github/workflows/visual-regression.yml index 26e93bca211..e507e1cc5ec 100644 --- a/.github/workflows/visual-regression.yml +++ b/.github/workflows/visual-regression.yml @@ -129,10 +129,46 @@ jobs: github-token: ${{ secrets.GITHUB_TOKEN }} automation-token: ${{ secrets.REPOS_READ_ONLY_TOKEN }} + # Flip the unified CI status comment's automation section to "running" the + # moment the suite is dispatched, so the on-demand placeholder never lingers + # while a run is in flight. + automation-pending: + name: Mark automation running + needs: resolve + if: needs.resolve.outputs.authorized == 'true' && needs.resolve.outputs.pr_number != '' + runs-on: ubuntu-latest + # This job only writes the status comment; the workflow-level contents:write + # ceiling exists for the reusable suite call, not for it. + permissions: + contents: read + pull-requests: write + steps: + - name: Checkout CI status action + uses: actions/checkout@v6 + with: + sparse-checkout: .github/actions/ci-status-comment + sparse-checkout-cone-mode: false + persist-credentials: false + + - name: Set automation section to running + uses: ./.github/actions/ci-status-comment + with: + pr-number: ${{ needs.resolve.outputs.pr_number }} + section: automation + github-token: ${{ github.token }} + body: |- + [![Automation](https://img.shields.io/badge/Automation-Running!-yellow?logo=github&logoColor=white&style=for-the-badge)](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) + + Visual regression suite running for commit `${{ needs.resolve.outputs.head_short_sha }}` — [watch the run](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}). + run-suite: name: Run visual suite - needs: resolve - if: needs.resolve.outputs.authorized == 'true' + # automation-pending in needs: orders the two writers of the automation + # section — the "Running!" write must precede the suite (and so the final + # verdict), or a queue-delayed pending write can overwrite the verdict and + # stick. !cancelled() keeps the suite running when pending skips or fails. + needs: [resolve, automation-pending] + if: ${{ !cancelled() && needs.resolve.outputs.authorized == 'true' }} # @main pins us to the merged version of the reusable workflow so PRs to # explorer-automation that touch run-visual-suite.yml don't accidentally # affect every unity-explorer PR's visual run. @@ -145,3 +181,81 @@ jobs: commit_sha: ${{ needs.resolve.outputs.head_short_sha }} branch_label: ${{ needs.resolve.outputs.head_ref }} secrets: inherit + + # Final state of the automation section: badge + Allure report + run links. + # The reusable workflow still posts its own detailed per-platform comment; + # this section is the at-a-glance summary inside the unified CI status comment. + report: + name: Update CI status comment + needs: [resolve, run-suite] + if: always() && needs.resolve.outputs.authorized == 'true' && needs.resolve.outputs.pr_number != '' + runs-on: ubuntu-latest + # This job only writes the status comment; the workflow-level contents:write + # ceiling exists for the reusable suite call, not for it. + permissions: + contents: read + pull-requests: write + steps: + - name: Checkout CI status action + uses: actions/checkout@v6 + with: + sparse-checkout: .github/actions/ci-status-comment + sparse-checkout-cone-mode: false + persist-credentials: false + + - name: Compose automation section + id: compose + env: + RESULT: ${{ needs.run-suite.result }} + PR_NUMBER: ${{ needs.resolve.outputs.pr_number }} + COMMIT_SHA: ${{ needs.resolve.outputs.head_short_sha }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + PUBLIC_URL_PREFIX: ${{ vars.EXPLORER_TEAM_S3_BUCKET_PUBLIC_URL }} + REPO: ${{ github.repository }} + run: | + set -euo pipefail + BADGE_STYLE="?logo=github&logoColor=white&style=for-the-badge" + case "$RESULT" in + success) BADGE="https://img.shields.io/badge/Automation-Passed!-3fb950${BADGE_STYLE}"; MSG="✅ Visual regression suite passed." ;; + failure) BADGE="https://img.shields.io/badge/Automation-Failed!-ff0000${BADGE_STYLE}"; MSG="❌ Visual regression suite failed." ;; + *) BADGE="https://img.shields.io/badge/Automation-Cancelled-lightgrey${BADGE_STYLE}"; MSG="Visual regression suite did not finish (\`$RESULT\`)." ;; + esac + + # Mirrors run-visual-suite.yml's "Compute identifiers" S3 path. mode=test + # and platform=macos are the reusable workflow's defaults — this dispatcher + # passes neither, so keep the three in lockstep if that ever changes. + REPORT_URL="${PUBLIC_URL_PREFIX}/@dcl/${REPO//\//-}/visual-regression/test/macos/${PR_NUMBER}/${COMMIT_SHA}/index.html" + + # The callee only syncs a report to S3 when the suite produced one — + # probe before rendering the link so a dead run doesn't present a + # 404 as a working report. + if curl -sfIL --max-time 15 "$REPORT_URL" >/dev/null 2>&1; then + REPORT_ROW="| Allure report | [Open report]($REPORT_URL) |" + else + REPORT_ROW="| Allure report | not produced — see the workflow run |" + fi + + DELIM="EOF_${RANDOM}${RANDOM}_$$" + { + echo "body<<$DELIM" + echo "[![Automation]($BADGE)]($RUN_URL)" + echo "" + echo "$MSG" + echo "" + echo "| Name | Link |" + echo "| -------- | ----------------------- |" + echo "| Commit | [\`$COMMIT_SHA\`](${GITHUB_SERVER_URL:-https://github.com}/${REPO}/commit/${COMMIT_SHA}) |" + echo "$REPORT_ROW" + echo "| Workflow run | [View run]($RUN_URL) |" + echo "" + echo "Triggered via \`/visual-tests\` · the detailed per-platform comment is posted separately." + echo "$DELIM" + } >> "$GITHUB_OUTPUT" + + - name: Update automation section + uses: ./.github/actions/ci-status-comment + with: + pr-number: ${{ needs.resolve.outputs.pr_number }} + section: automation + github-token: ${{ github.token }} + body: ${{ steps.compose.outputs.body }} diff --git a/scripts/cloudbuild/build.py b/scripts/cloudbuild/build.py index b718c28adff..2eecc8dc9f5 100644 --- a/scripts/cloudbuild/build.py +++ b/scripts/cloudbuild/build.py @@ -8,6 +8,8 @@ import requests import datetime import argparse +import subprocess +import tempfile import collections from urllib3.util.retry import Retry from requests.adapters import HTTPAdapter @@ -61,6 +63,42 @@ def _extract_member(self, member, targetpath, pwd): build_healthy = True +# Deep link to this build in the Unity Cloud dashboard, captured from the first build +# response that carries one. Persisted to BUILD_LINK_INFO_PATH so the workflow can +# upload it and the PR status comment can link the build directly. +BUILD_LINK_INFO_PATH = 'unity_cloud_build_info.env' +# Mirror of URL_RE in .github/actions/ucb-build-links/action.yml (the consumer +# silently drops links failing it, so only persist links that will survive). +_DASHBOARD_LINK_RE = re.compile( + r'^https://(cloud\.unity\.com|developer\.cloud\.unity3d\.com|dashboard\.unity3d\.com)' + r'/[A-Za-z0-9./_%~?=&#-]*/builds/[0-9]+[A-Za-z0-9./_%~?=&#-]*$') +dashboard_url = None +_build_link_info_written = False +_final_elapsed = None # (queue_secs, build_secs), set once when the build reaches a terminal status + +# Live PR status-comment update: the artifact above only leaves the runner when +# this job ends, so the comment's build section is also written directly the +# moment the build id is known — the Unity Cloud link goes live while the build +# runs instead of after it. Purely cosmetic: every failure is swallowed. +CI_STATUS_SCRIPT = os.path.join('.github', 'actions', 'ci-status-comment', 'upsert-ci-status.sh') +LIVE_MARKER_PREFIX = '' in body: + start = body.find('') + end = body.find('') + return body[start:end] if 0 <= start < end else '' + if len(comments) < 100: + return '' + return None + + +def _platform_key(): + """windows64 | macos, surviving the per-branch TARGET rewrites this script does.""" + target = os.getenv('TARGET') or '' + if target.startswith('t_'): + target = target[2:] + for key in ('windows64', 'macos'): + if target.startswith(key): + return key + return target or 'unknown' + + +def _dashboard_build_url(build_id): + """Unity Cloud dashboard page for a build, constructible from the env + alone — the API's own dashboard_summary/dashboard_log deep links replace + it as soon as a poll response carries them.""" + org = os.getenv('ORG_ID') + project = os.getenv('PROJECT_ID') + target = os.getenv('TARGET') + if not (org and project and target): + return None + return (f'https://cloud.unity.com/home/organizations/{org}/projects/{project}' + f'/cloud-build/buildtargets/{target}/builds/{build_id}') + + +def _own_job_url(): + repo = os.getenv('GITHUB_REPOSITORY') + run_id = os.getenv('GITHUB_RUN_ID') + try: + resp = _github_api(f'/repos/{repo}/actions/runs/{run_id}/jobs?per_page=100') + if resp.status_code == 200: + for job in resp.json().get('jobs') or []: + if job.get('name') == f'Build ({_platform_key()})': + return job.get('html_url') + except requests.RequestException: + pass + return None + + +def upsert_live_comment(build_id, only_if_missing=False): + """Write this target's live links into the comment's build section. + + upsert-ci-status.sh's own retry loop only confirms that *this* write's body + landed — it has no way to notice a sibling row that arrived between this + function's read and that write. So each attempt here re-reads the section, + recomposes the row union from that fresh read, and after writing re-reads + once more to confirm every row it composed (including any sibling row it + carried along) survived; a mismatch means a concurrent write raced in and + it retries against a new read rather than trusting the stale union. + Bounded by LIVE_COMMENT_WRITE_ATTEMPTS. Returns True when a write landed + and its union was confirmed, False when the row was already present, and + None when a read/write failed or the union never settled. + """ + platform = _platform_key() + label = {'windows64': 'Windows', 'macos': 'Mac'}.get(platform, platform) + marker = f'{LIVE_MARKER_PREFIX}{platform} -->' + + parts = [] + job_url = _own_job_url() + if job_url: + parts.append(f'[GitHub job]({job_url})') + # Unlinked last-resort must not say "#N": GitHub autolinks bare #N in + # comments to issue N. + link = dashboard_url or _dashboard_build_url(build_id) + parts.append(f'[Unity Cloud #{build_id}]({link})' if link else f'Unity Cloud build {build_id}') + own_row = f'| {label} | {" · ".join(parts)} {marker} |' + + for _ in range(LIVE_COMMENT_WRITE_ATTEMPTS): + section = _build_section_of_status_comment() + if section is None: + return None + if only_if_missing and marker in section: + return False + + rows = [line for line in section.splitlines() if LIVE_MARKER_PREFIX in line and marker not in line] + rows.append(own_row) + rows.sort(key=lambda row: 0 if '| Windows |' in row else 1) + + server = os.getenv('GITHUB_SERVER_URL', 'https://github.com') + run_url = f"{server}/{os.getenv('GITHUB_REPOSITORY')}/actions/runs/{os.getenv('GITHUB_RUN_ID')}" + body = '\n'.join([ + f'[![Build](https://img.shields.io/badge/Build-In%20progress-1f6feb?logo=unity&logoColor=white&style=for-the-badge)]({run_url})', + '', + '| Platform | Links & timing |', + '| -------- | ----------------------- |', + *rows, + ]) + + body_file = None + try: + with tempfile.NamedTemporaryFile('w', suffix='.md', delete=False) as f: + f.write(body) + body_file = f.name + env = dict(os.environ, + REPO=os.getenv('GITHUB_REPOSITORY') or '', + SECTION='build', + SECTION_BODY='', + SECTION_BODY_FILE=body_file) + result = subprocess.run(['bash', CI_STATUS_SCRIPT], env=env, timeout=180, check=False) + finally: + if body_file: + os.unlink(body_file) + if result.returncode != 0: + return None + + post_section = _build_section_of_status_comment() + if post_section is not None and all(row in post_section for row in rows): + return True + + return None + + +def maybe_update_live_comment(build_id, reconcile=False, force=False): + """Gate and rate-limit the live comment write; never let it fail the build.""" + global _live_comment_asserts, _live_comment_last_attempt, _live_comment_confirms + if not os.getenv('PR_NUMBER') or not (os.getenv('GH_TOKEN') or os.getenv('GITHUB_TOKEN')): + return + if not os.path.exists(CI_STATUS_SCRIPT): + return + if reconcile: + # Re-assert a few times only: the Pending reset (or the other target's + # first write racing ours) can land after us and drop this row. A zero + # count still falls through, so a failed first write gets retried. + if _live_comment_asserts >= MAX_LIVE_ASSERTS: + return + # Every probe is a comments-API read drawn from the repo-shared rate + # budget; once the row has stayed put this many consecutive checks, + # stop probing for the rest of the build. + if _live_comment_confirms >= MAX_LIVE_CONFIRMS: + return + if time.time() - _live_comment_last_attempt < LIVE_RECONCILE_INTERVAL_SECS: + return + elif _live_comment_asserts > 0 and not force: + return + try: + _live_comment_last_attempt = time.time() + outcome = upsert_live_comment(build_id, only_if_missing=reconcile) + if outcome is True: + _live_comment_asserts += 1 + _live_comment_confirms = 0 + elif outcome is False and reconcile: + # Only a confirmed present row spends the probe budget; a failed + # read or write (None) must leave both counters for the retry. + _live_comment_confirms += 1 + except Exception as e: + print(f'note: live status-comment update failed: {e}') + + def write_step_summary(target, build_id, final_status, phase_durations, queue_reasons, queue_elapsed, build_elapsed): """Append a phase breakdown to $GITHUB_STEP_SUMMARY (best-effort).""" summary_path = os.environ.get('GITHUB_STEP_SUMMARY') @@ -635,6 +914,8 @@ def fmt(seconds): lines.append('') lines.append(f'- Target: `{target}`') lines.append(f'- Build ID: `{build_id}`') + if dashboard_url: + lines.append(f'- Unity Cloud build page: {dashboard_url}') lines.append(f'- Final outcome: `{final_status}`') if queue_reasons: lines.append(f"- Queue reasons seen: {', '.join(f'`{r}`' for r in sorted(queue_reasons))}") @@ -711,6 +992,13 @@ def run_poll_loop(id, build_already_active=False, resumed_build_elapsed=0): keep_polling, status, response_json = poll_build(id) + # Both run every poll: record keeps retrying until the info file lands + # AND a dashboard href arrives, and reconcile self-heals the live row + # whether or not an href ever qualifies (it rate-limits internally). + if dashboard_url is None or not _build_link_info_written: + record_build_link_info(id, response_json) + maybe_update_live_comment(id, reconcile=True) + queued_reason = response_json.get('queuedReason') if queued_reason and status in QUEUE_STATUSES: queue_reasons.add(queued_reason) @@ -782,150 +1070,160 @@ def run_poll_loop(id, build_already_active=False, resumed_build_elapsed=0): time.sleep(poll_interval) -args = parser.parse_args() -build_already_active = False -resumed_build_elapsed = 0 +if __name__ == '__main__': + args = parser.parse_args() -if args.delete: - delete_current_target() -elif args.resume or args.cancel: - build_info = utils.read_build_info() - if build_info is None: - sys.exit(1) + build_already_active = False + resumed_build_elapsed = 0 + + if args.delete: + delete_current_target() + elif args.resume or args.cancel: + build_info = utils.read_build_info() + if build_info is None: + sys.exit(1) + + os.environ['TARGET'] = build_info["target"] + id = build_info["id"] + + if args.cancel: + if id is None: + # The runner died between the build POST and the id write; the queued build is + # findable only as the target's latest build. Cancel it only while it is still + # in a queue status: targets are shared (release pool; consecutive runs on one + # branch), so an already-started build may belong to a concurrent run — leaving + # it is at worst one wasted build, cancelling it would kill someone else's. + # A missing/unknown status is treated as not-cancellable for the same reason. + latest = get_latest_build(os.getenv('TARGET')) + if latest and latest.get('buildStatus') in QUEUE_STATUSES: + id = latest['build'] + print(f'No build id persisted; cancelling latest queued build #{id} on {os.getenv("TARGET")}') + else: + print('No build id persisted and no queued build found; nothing to cancel.') + utils.delete_build_info() + sys.exit(0) + cancel_build(id) + utils.delete_build_info() + sys.exit(0) - os.environ['TARGET'] = build_info["target"] - id = build_info["id"] - - if args.cancel: - if id is None: - # The runner died between the build POST and the id write; the queued build is - # findable only as the target's latest build. Cancel it only while it is still - # in a queue status: targets are shared (release pool; consecutive runs on one - # branch), so an already-started build may belong to a concurrent run — leaving - # it is at worst one wasted build, cancelling it would kill someone else's. - # A missing/unknown status is treated as not-cancellable for the same reason. - latest = get_latest_build(os.getenv('TARGET')) - if latest and latest.get('buildStatus') in QUEUE_STATUSES: - id = latest['build'] - print(f'No build id persisted; cancelling latest queued build #{id} on {os.getenv("TARGET")}') - else: - print('No build id persisted and no queued build found; nothing to cancel.') - utils.delete_build_info() - sys.exit(0) - cancel_build(id) - utils.delete_build_info() - sys.exit(0) - -else: - branch_name = os.getenv('BRANCH_NAME') - validate_branch_name(branch_name) - - resumed = try_resume_build() - if resumed is not None: - target_name, id, resumed_status, resumed_elapsed = resumed - os.environ['TARGET'] = target_name - build_already_active = resumed_status in ACTIVE_STATUSES - if build_already_active: - resumed_build_elapsed = resumed_elapsed else: - try: - clone_current_target(True) - except Exception as e: - print(f"Operation failed: {e}") - - # Set parameters immediately before run_build to avoid races with concurrent - # builds on shared targets. - set_parameters(get_param_env_variables()) - - def get_clean_build_bool(): - value = os.getenv('CLEAN_BUILD', 'false').lower() - if value in ['true', '1']: - return True - elif value in ['false', '0']: - return False - else: - raise ValueError(f"Invalid boolean value for CLEAN_BUILD: {value}") - - # Persist the target before the POST: if the runner dies mid-request, --cancel can still - # find the queued build via the target's latest-build lookup. - utils.persist_build_info(os.getenv('TARGET'), None) - id = run_build(os.getenv('BRANCH_NAME'), get_clean_build_bool()) - utils.persist_build_info(os.getenv('TARGET'), id) - print(f'For more info and live logs, go to https://cloud.unity.com/ and search for target "{os.getenv('TARGET')}" and build ID "{id}"') - -final_outcome, phase_durations, queue_reasons, queue_elapsed, build_elapsed = run_poll_loop( - id, - build_already_active=build_already_active, - resumed_build_elapsed=resumed_build_elapsed, -) -write_step_summary(os.getenv('TARGET'), id, final_outcome, phase_durations, queue_reasons, queue_elapsed, build_elapsed) - -if final_outcome in ('queue_timeout', 'build_timeout', 'log_stall'): - if final_outcome in ('build_timeout', 'log_stall'): - # Build was cancelled; the persisted info points to a dead build. - # Delete it so the next retry creates a fresh build on a different VM. + branch_name = os.getenv('BRANCH_NAME') + validate_branch_name(branch_name) + + resumed = try_resume_build() + if resumed is not None: + target_name, id, resumed_status, resumed_elapsed = resumed + os.environ['TARGET'] = target_name + build_already_active = resumed_status in ACTIVE_STATUSES + if build_already_active: + resumed_build_elapsed = resumed_elapsed + else: + try: + clone_current_target(True) + except Exception as e: + print(f"Operation failed: {e}") + + # Set parameters immediately before run_build to avoid races with concurrent + # builds on shared targets. + set_parameters(get_param_env_variables()) + + def get_clean_build_bool(): + value = os.getenv('CLEAN_BUILD', 'false').lower() + if value in ['true', '1']: + return True + elif value in ['false', '0']: + return False + else: + raise ValueError(f"Invalid boolean value for CLEAN_BUILD: {value}") + + # Persist the target before the POST: if the runner dies mid-request, --cancel can still + # find the queued build via the target's latest-build lookup. + utils.persist_build_info(os.getenv('TARGET'), None) + id = run_build(os.getenv('BRANCH_NAME'), get_clean_build_bool()) + utils.persist_build_info(os.getenv('TARGET'), id) + # Write the link info file (target + id, no URL yet) immediately so it exists + # even if the runner dies before the first poll; the poll loop upgrades it + # with the dashboard URL once a response carries one. + record_build_link_info(id, {}) + print(f'For more info and live logs, go to https://cloud.unity.com/ and search for target "{os.getenv('TARGET')}" and build ID "{id}"') + + final_outcome, phase_durations, queue_reasons, queue_elapsed, build_elapsed = run_poll_loop( + id, + build_already_active=build_already_active, + resumed_build_elapsed=resumed_build_elapsed, + ) + write_step_summary(os.getenv('TARGET'), id, final_outcome, phase_durations, queue_reasons, queue_elapsed, build_elapsed) + + if final_outcome in ('queue_timeout', 'build_timeout', 'log_stall'): + if final_outcome in ('build_timeout', 'log_stall'): + # Build was cancelled; the persisted info points to a dead build. + # Delete it so the next retry creates a fresh build on a different VM. + utils.delete_build_info() + try: + download_log(id) + except Exception as e: + print(f'Warning: could not download log after {final_outcome}: {e}') + sys.exit(RETRYABLE_EXIT_CODE) + + if final_outcome == 'canceled': + # This run's own cancellations exit through the watchdog/timeout branches above, + # so 'canceled' here came from outside. Two different outsides, though: + # - UBA giving up on builder provisioning (observed: 9 min in sentToBuilder, then a + # platform-side cancel) — nothing else wants the target, so retry on a fresh build; + # - a concurrent run superseding us via run_build's `already a build pending` cancel. + # main/release/*/hotfix/* share one target but sit in different concurrency groups, + # so re-POSTing here would cancel *their* build and hand them the same exit 99 — + # both runs then burn a full queue+build cycle and one still ends red. + # Build numbers are monotonic per target: a newer build means we were superseded. + def probe_latest_build(): + # Fail-open: a transient socket error here must not traceback past the + # cleanup below - it degrades to the retry path, same as a non-200 probe. + try: + return get_latest_build(os.getenv('TARGET')) + except requests.exceptions.RequestException as e: + print(f'Warning: latest-build probe failed ({e})') + return None + + latest = probe_latest_build() + if latest and int(latest.get('build') or 0) <= int(id): + # run_build cancels the pending build and only re-POSTs ~30 s later, so a + # supersede can be invisible for that gap. Re-probe once past it before + # deciding to retry. + time.sleep(35) + latest = probe_latest_build() or latest utils.delete_build_info() try: download_log(id) except Exception as e: - print(f'Warning: could not download log after {final_outcome}: {e}') - sys.exit(RETRYABLE_EXIT_CODE) - -if final_outcome == 'canceled': - # This run's own cancellations exit through the watchdog/timeout branches above, - # so 'canceled' here came from outside. Two different outsides, though: - # - UBA giving up on builder provisioning (observed: 9 min in sentToBuilder, then a - # platform-side cancel) — nothing else wants the target, so retry on a fresh build; - # - a concurrent run superseding us via run_build's `already a build pending` cancel. - # main/release/*/hotfix/* share one target but sit in different concurrency groups, - # so re-POSTing here would cancel *their* build and hand them the same exit 99 — - # both runs then burn a full queue+build cycle and one still ends red. - # Build numbers are monotonic per target: a newer build means we were superseded. - def probe_latest_build(): - # Fail-open: a transient socket error here must not traceback past the - # cleanup below - it degrades to the retry path, same as a non-200 probe. - try: - return get_latest_build(os.getenv('TARGET')) - except requests.exceptions.RequestException as e: - print(f'Warning: latest-build probe failed ({e})') - return None + print(f'Warning: could not download log after external cancel: {e}') + if latest and int(latest.get('build') or 0) > int(id): + print( + f'Build {id} was superseded by #{latest["build"]} on shared target ' + f'{os.getenv("TARGET")} - not retrying (the successor owns the slot).' + ) + sys.exit(1) + print('Build was canceled outside this run - retrying with a fresh build.') + sys.exit(RETRYABLE_EXIT_CODE) - latest = probe_latest_build() - if latest and int(latest.get('build') or 0) <= int(id): - # run_build cancels the pending build and only re-POSTs ~30 s later, so a - # supersede can be invisible for that gap. Re-probe once past it before - # deciding to retry. - time.sleep(35) - latest = probe_latest_build() or latest utils.delete_build_info() - try: - download_log(id) - except Exception as e: - print(f'Warning: could not download log after external cancel: {e}') - if latest and int(latest.get('build') or 0) > int(id): - print( - f'Build {id} was superseded by #{latest["build"]} on shared target ' - f'{os.getenv("TARGET")} - not retrying (the successor owns the slot).' - ) - sys.exit(1) - print('Build was canceled outside this run - retrying with a fresh build.') - sys.exit(RETRYABLE_EXIT_CODE) -utils.delete_build_info() + print(f'Runner FINAL elapsed: queue {datetime.timedelta(seconds=int(queue_elapsed))} / build {datetime.timedelta(seconds=int(build_elapsed))}') + record_final_elapsed(id, queue_elapsed, build_elapsed) -print(f'Runner FINAL elapsed: queue {datetime.timedelta(seconds=int(queue_elapsed))} / build {datetime.timedelta(seconds=int(build_elapsed))}') + download_artifact(id) + download_log(id) -download_artifact(id) -download_log(id) - -if not build_healthy: - print(f'Build unhealthy - check the downloaded logs or go to https://cloud.unity.com/ and search for target "{os.getenv('TARGET')}" and build ID "{id}"') - sys.exit(1) + if not build_healthy: + # Dashboard URLs embed ORG_ID/PROJECT_ID, which are masked to *** in + # runner logs — the PR status comment carries the clickable link. + print(f'Build unhealthy - check the downloaded logs or the Unity Cloud build page ' + f'linked from the PR status comment (target "{os.getenv("TARGET")}", build {id}).') + sys.exit(1) -# Cleanup (only if build is healthy and not release) -# We only delete all artifacts, not the build target -if not is_release_workflow: - delete_build(id) + # Cleanup (only if build is healthy and not release) + # We only delete all artifacts, not the build target + if not is_release_workflow: + delete_build(id) -utils.delete_build_info() + utils.delete_build_info() diff --git a/scripts/cloudbuild/test_build_helpers.py b/scripts/cloudbuild/test_build_helpers.py new file mode 100644 index 00000000000..acbee04259e --- /dev/null +++ b/scripts/cloudbuild/test_build_helpers.py @@ -0,0 +1,284 @@ +"""Unit tests for build.py's pure helpers and the link-info file writer. + +Run from anywhere: python3 -m unittest scripts.cloudbuild.test_build_helpers +(or `python3 -m unittest discover -s scripts/cloudbuild`). build.py's build +flow is under a __main__ guard, so importing it here executes nothing. +""" +import os +import re +import sys +import tempfile +import unittest +from unittest import mock + +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) +import build # noqa: E402 + +REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), '..', '..')) +UCB_LINKS_ACTION = os.path.join(REPO_ROOT, '.github', 'actions', 'ucb-build-links', 'action.yml') + + +class EnvMixin: + def set_env(self, **pairs): + for key, value in pairs.items(): + old = os.environ.get(key) + self.addCleanup( + (lambda k, v: (os.environ.__setitem__(k, v) if v is not None else os.environ.pop(k, None))) + , key, old) + if value is None: + os.environ.pop(key, None) + else: + os.environ[key] = value + + +class PlatformKeyTest(EnvMixin, unittest.TestCase): + def check(self, target, expected): + self.set_env(TARGET=target) + self.assertEqual(build._platform_key(), expected) + + def test_template_targets(self): + self.check('t_windows64', 'windows64') + self.check('t_macos', 'macos') + + def test_branch_derived_targets(self): + self.check('windows64-feat-unity-cloud-build-link', 'windows64') + self.check('macos-release-epic', 'macos') + + def test_unknown(self): + self.check('linux64-foo', 'linux64-foo') + self.check('', 'unknown') + + +class DashboardUrlTest(EnvMixin, unittest.TestCase): + ENV = dict(ORG_ID='4673197905245', + PROJECT_ID='8c12744f-9e98-47b8-b40c-576d04cb8d5c', + TARGET='windows64-some-branch') + + def test_shape(self): + self.set_env(**self.ENV) + self.assertEqual( + build._dashboard_build_url(15), + 'https://cloud.unity.com/home/organizations/4673197905245' + '/projects/8c12744f-9e98-47b8-b40c-576d04cb8d5c' + '/buildtargets/windows64-some-branch/builds/15'.replace( + '/buildtargets', '/cloud-build/buildtargets')) + + def test_missing_env_returns_none(self): + for absent in ('ORG_ID', 'PROJECT_ID', 'TARGET'): + env = dict(self.ENV) + env[absent] = None + self.set_env(**env) + self.assertIsNone(build._dashboard_build_url(15), f'{absent} unset') + + def test_matches_consumer_url_re(self): + """Drift guard: the consumer drops URLs failing its allowlist silently, + so the producer's constructed URL must always pass it.""" + with open(UCB_LINKS_ACTION) as f: + match = re.search(r"URL_RE='([^']+)'", f.read()) + self.assertIsNotNone(match, 'URL_RE not found in ucb-build-links/action.yml') + url_re = re.compile(match.group(1)) + self.set_env(**self.ENV) + url = build._dashboard_build_url(42) + self.assertRegex(url, url_re) + # The producer's API-href filter must be the same rule verbatim, or a + # link it persists could still be dropped downstream. + self.assertEqual(build._DASHBOARD_LINK_RE.pattern, match.group(1)) + self.assertRegex(url, build._DASHBOARD_LINK_RE) + + +class LinkInfoFileTest(EnvMixin, unittest.TestCase): + def setUp(self): + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + old_cwd = os.getcwd() + self.addCleanup(os.chdir, old_cwd) + os.chdir(tmp.name) + # Silence build.py's prints: its ::notice:: line is a live workflow + # command when the test job itself runs on the Actions runner. + silencer = mock.patch('builtins.print') + silencer.start() + self.addCleanup(silencer.stop) + # PR_NUMBER unset keeps maybe_update_live_comment inert. + self.set_env(TARGET='windows64-x', ORG_ID='org1', PROJECT_ID='proj1', PR_NUMBER=None) + build.dashboard_url = None + build._build_link_info_written = False + build._final_elapsed = None + self.addCleanup(self._reset_module_state) + + @staticmethod + def _reset_module_state(): + build.dashboard_url = None + build._build_link_info_written = False + build._final_elapsed = None + + @staticmethod + def read_info(): + with open(build.BUILD_LINK_INFO_PATH) as f: + return dict(line.strip().split('=', 1) for line in f if '=' in line) + + def test_first_write_uses_constructed_url(self): + build.record_build_link_info(7, {}) + info = self.read_info() + self.assertEqual(info['BUILD_ID'], '7') + self.assertEqual(info['DASHBOARD_URL'], build._dashboard_build_url(7)) + self.assertNotIn('QUEUE_SECS', info) + + def test_api_href_replaces_constructed_and_survives_final_rewrite(self): + build.record_build_link_info(7, {}) + href = 'https://cloud.unity.com/some/deep/builds/7/link' + build.record_build_link_info(7, {'links': {'dashboard_summary': {'href': href}}}) + self.assertEqual(self.read_info()['DASHBOARD_URL'], href) + + build.record_final_elapsed(7, 63, 3725) + info = self.read_info() + self.assertEqual(info['DASHBOARD_URL'], href, 'final rewrite must keep the API deep link') + self.assertEqual(info['QUEUE_SECS'], '63') + self.assertEqual(info['BUILD_SECS'], '3725') + + def test_non_build_link_rejected(self): + build.record_build_link_info(7, {'links': {'dashboard_url': {'href': 'https://cloud.unity.com/'}}}) + self.assertEqual(self.read_info()['DASHBOARD_URL'], build._dashboard_build_url(7)) + + def test_link_failing_consumer_allowlist_rejected(self): + for href in ('https://example.com/deep/builds/7', # non-dashboard host + 'https://cloud.unity.com/deep/builds/none', # no numeric build id + 'https://cloud.unity.com/deep/builds/7?x=<'): # char outside the allowlist + build.record_build_link_info(7, {'links': {'dashboard_summary': {'href': href}}}) + self.assertEqual(self.read_info()['DASHBOARD_URL'], build._dashboard_build_url(7), href) + + def test_final_elapsed_clamps_negative(self): + build.record_final_elapsed(7, -5, -1) + info = self.read_info() + self.assertEqual(info['QUEUE_SECS'], '0') + self.assertEqual(info['BUILD_SECS'], '0') + + +class LiveCommentReconcileTest(EnvMixin, unittest.TestCase): + def setUp(self): + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + old_cwd = os.getcwd() + self.addCleanup(os.chdir, old_cwd) + os.chdir(tmp.name) + # CI_STATUS_SCRIPT is cwd-relative; it must exist for the gate to pass. + os.makedirs(os.path.dirname(build.CI_STATUS_SCRIPT)) + open(build.CI_STATUS_SCRIPT, 'w').close() + self.set_env(PR_NUMBER='1', GH_TOKEN='token') + self._reset_counters() + self.addCleanup(self._reset_counters) + self.addCleanup(setattr, build, 'upsert_live_comment', build.upsert_live_comment) + + @staticmethod + def _reset_counters(): + build._live_comment_asserts = 0 + build._live_comment_last_attempt = 0.0 + build._live_comment_confirms = 0 + + def stub_upsert(self, result): + calls = [] + + def fake(build_id, only_if_missing=False): + calls.append(only_if_missing) + if isinstance(result, Exception): + raise result + return result + build.upsert_live_comment = fake + return calls + + def test_reconcile_retries_after_failed_first_write(self): + self.stub_upsert(RuntimeError('transient')) + build.maybe_update_live_comment(7) # swallowed; no row asserted + self.assertEqual(build._live_comment_asserts, 0) + + calls = self.stub_upsert(True) + build.maybe_update_live_comment(7, reconcile=True) + self.assertEqual(calls, [], 'the 240s spacing must still hold') + + build._live_comment_last_attempt -= 241 + build.maybe_update_live_comment(7, reconcile=True) + self.assertEqual(calls, [True], 'reconcile must retry the failed first write') + self.assertEqual(build._live_comment_asserts, 1) + + def test_reconcile_caps_still_hold(self): + calls = self.stub_upsert(True) + build._live_comment_asserts = 3 + build.maybe_update_live_comment(7, reconcile=True) + build._live_comment_asserts = 1 + build._live_comment_confirms = 3 + build.maybe_update_live_comment(7, reconcile=True) + self.assertEqual(calls, []) + + +class UpsertLiveCommentRaceTest(EnvMixin, unittest.TestCase): + """Pins the race mikhail-dcl flagged in review: upsert-ci-status.sh's own + retry loop only confirms that *this* write's body landed, so a sibling + platform's row arriving between upsert_live_comment's read and that write + is invisible to it. upsert_live_comment must notice its composed union did + not survive and retry against a fresh read instead of reporting success on + a stale one.""" + + WIN_ROW = '| Windows | Unity Cloud build 7 |' + MAC_ROW = '| Mac | Unity Cloud build 9 |' + + def setUp(self): + self.set_env(TARGET='windows64-x', GITHUB_REPOSITORY='org/repo', GITHUB_RUN_ID='1', + GITHUB_SERVER_URL='https://github.com', ORG_ID=None, PROJECT_ID=None) + build.dashboard_url = None + self.addCleanup(setattr, build, 'dashboard_url', None) + + @staticmethod + def fake_run_capturing(bodies): + def fake_run(cmd, env, timeout, check): + with open(env['SECTION_BODY_FILE']) as f: + bodies.append(f.read()) + return mock.Mock(returncode=0) + return fake_run + + def test_sibling_row_landing_mid_write_is_recovered_on_retry(self): + # attempt 1 pre-write read: no rows yet. + # attempt 1 post-write verify: Mac's row raced in underneath us — the + # union this attempt wrote (Windows only) is now stale. The pre-fix + # code had no post-write read at all and would have reported success + # here, permanently dropping Mac's row from the next real write. + # attempt 2 pre-write read: fresh, carries Mac's row along. + # attempt 2 post-write verify: both rows confirmed present. + reads = ['', self.MAC_ROW, self.MAC_ROW, f'{self.MAC_ROW}\n{self.WIN_ROW}'] + bodies = [] + with mock.patch.object(build, '_build_section_of_status_comment', side_effect=reads), \ + mock.patch.object(build, '_own_job_url', return_value=None), \ + mock.patch.object(build.subprocess, 'run', side_effect=self.fake_run_capturing(bodies)): + result = build.upsert_live_comment(7) + + self.assertTrue(result) + self.assertEqual(len(bodies), 2, 'a stale-union write must be retried, not accepted') + self.assertIn(self.WIN_ROW, bodies[0]) + self.assertNotIn(self.MAC_ROW, bodies[0], "attempt 1's read had no sibling row yet") + self.assertIn(self.WIN_ROW, bodies[1]) + self.assertIn(self.MAC_ROW, bodies[1], "retry's union must carry the sibling row along") + + def test_gives_up_after_exhausting_attempts_instead_of_spinning(self): + # Pathological: the post-write read never reflects this attempt's own + # write (as if every attempt kept losing the race). Must terminate + # after LIVE_COMMENT_WRITE_ATTEMPTS, not retry forever. + reads = [''] * (2 * build.LIVE_COMMENT_WRITE_ATTEMPTS) + bodies = [] + with mock.patch.object(build, '_build_section_of_status_comment', side_effect=reads), \ + mock.patch.object(build, '_own_job_url', return_value=None), \ + mock.patch.object(build.subprocess, 'run', side_effect=self.fake_run_capturing(bodies)): + result = build.upsert_live_comment(7) + + self.assertIsNone(result) + self.assertEqual(len(bodies), build.LIVE_COMMENT_WRITE_ATTEMPTS) + + def test_only_if_missing_short_circuits_on_first_fresh_read(self): + with mock.patch.object(build, '_build_section_of_status_comment', return_value=self.WIN_ROW), \ + mock.patch.object(build, '_own_job_url', return_value=None), \ + mock.patch.object(build.subprocess, 'run') as run: + result = build.upsert_live_comment(7, only_if_missing=True) + + self.assertFalse(result) + run.assert_not_called() + + +if __name__ == '__main__': + unittest.main()