diff --git a/.github/scripts/nightly-report.sh b/.github/scripts/nightly-report.sh new file mode 100755 index 00000000..209a191b --- /dev/null +++ b/.github/scripts/nightly-report.sh @@ -0,0 +1,175 @@ +#!/usr/bin/env bash +# Make a red nightly FINDABLE WITHOUT KNOWING TO LOOK (#973). +# +# The deep gate ran red two nights and nothing surfaced it; before that, 25 +# consecutive nights. Three properties compound and each is reasonable alone: a +# scheduled run has no pull request to be red on, the failing job is named like a +# reporting step rather than a gate, and GitHub notifies the *actor* of a +# schedule event -- whoever last touched the workflow file, not whoever broke it. +# +# So this puts the verdict where somebody already looks: one issue, opened on the +# first red, updated on every red after it, and CLOSED BY THE NEXT GREEN. +# +# ONE ISSUE, NOT ONE PER RUN. A notifier that files a new issue nightly is a +# notifier people filter, and a filtered notifier is the state this replaces. +# The issue is found by its exact title, which is why the title carries no run +# number, no date and no job name -- everything variable goes in the body. +# +# IT CLOSES ITSELF. An alert that must be closed by hand becomes an alert nobody +# closes, and then a stale one nobody believes. The green path is as load-bearing +# as the red path and has its own arm. +# +# DRY RUN IS THE TESTED PATH, not a debugging aid: PGC_NIGHTLY_REPORT_DRYRUN=1 +# prints the verdict, the title and the body to stdout and calls no `gh`. The +# selftest drives exactly this, so what the arms read is what CI composes. +set -euo pipefail + +TITLE="${PGC_NIGHTLY_REPORT_TITLE:-nightly deep gate is red}" + +# THE VERDICT IS DERIVED HERE, FROM THE `needs` CONTEXT, AND THAT IS THE POINT. +# +# The first version computed it in the workflow from four named environment +# variables, which made TWO lists: `needs: [...]`, guarded by a set-equality arm +# in selftest 450, and the env block, guarded by nothing. A contributor adding a +# gate was COMPELLED by the arm to update the guarded list and told nothing about +# the one the verdict actually came from -- so the new gate could burn while this +# reported green. #973 reintroduced inside the fix for #973, found by +# @OffgridwithJD, who added a fifth job to a copy of the branch and got a fully +# green tree with a nightly whose failure the reporter could not see. +# +# `toJSON(needs)` carries every entry, so there is one list and nothing to keep in +# agreement -- which beats a guard asserting that two lists agree. +# +# AND IT MAKES THE DECISION TESTABLE. The workflow can only be read; this can be +# DRIVEN, so selftest 450 feeds it tuples and the fire drill exercises the real +# decision path instead of overriding the answer. +verdict_from_needs() { # verdict_from_needs -> failure|success + local json="$1" results r + # `skipped` is not a failure: a fork run, and a fire drill, skip every gate. + results="$(printf '%s' "$json" | python3 -c ' +import json, sys +d = json.load(sys.stdin) +if not isinstance(d, dict) or not d: + sys.exit(3) +for v in d.values(): + print((v or {}).get("result", "missing")) +' 2>/dev/null)" || return 3 + [ -n "$results" ] || return 3 + for r in $results; do + case "$r" in + failure|cancelled|timed_out) echo failure; return 0 ;; + success|skipped) ;; + # A result this does not know is NOT a pass. It means the platform + # grew a state while this did not, and treating it as green restores + # the silence #973 is about. + *) echo failure; return 0 ;; + esac + done + echo success +} + +if [ "${1:-}" = "--from-needs" ]; then + # AN EMPTY OR UNPARSEABLE CONTEXT IS REFUSED, not defaulted. `needs` empty + # means the job list changed shape; reporting green on that is the failure + # mode this whole file exists to remove. + if ! verdict="$(verdict_from_needs "${2:?usage: --from-needs }")"; then + echo "nightly-report: the needs context was empty or unparseable" >&2 + exit 3 + fi + shift 2 +else + verdict="${1:?usage: nightly-report.sh [job ...]}" + shift || true +fi +failed=("$@") + +run_url="${PGC_NIGHTLY_RUN_URL:-${GITHUB_SERVER_URL:-https://github.com}/${GITHUB_REPOSITORY:-}/actions/runs/${GITHUB_RUN_ID:-}}" + +case "$verdict" in + failure|success) ;; + *) + # NOT a silent default. A verdict this script does not understand means the + # caller changed and this did not; treating it as success would restore the + # exact silence the issue is about. + echo "nightly-report: verdict [$verdict] is neither failure nor success" >&2 + exit 2 + ;; +esac + +body_failure() { + printf '%s\n' "The nightly deep gate failed." + printf '\n' + printf 'Run: %s\n' "$run_url" + printf '\n' + if [ "${#failed[@]}" -gt 0 ]; then + printf 'Jobs that did not succeed:\n\n' + printf ' - %s\n' "${failed[@]}" + else + # The caller reported a failure and named no job. Say so rather than + # printing an empty list, which reads as "nothing failed". + printf 'No job name was reported with this failure, which is itself worth looking at.\n' + fi + printf '\n' + printf 'This issue is opened by the nightly on its first red, updated on each red\n' + printf 'after it, and closed automatically by the next green run.\n' +} + +body_success() { + printf '%s\n' "The nightly deep gate is green again." + printf '\n' + printf 'Run: %s\n' "$run_url" + printf '\n' + printf 'Closing automatically. An alert that must be closed by hand becomes one\n' + printf 'nobody closes, and then a stale one nobody believes.\n' +} + +if [ "${PGC_NIGHTLY_REPORT_DRYRUN:-0}" = 1 ]; then + printf 'verdict: %s\n' "$verdict" + printf 'title: %s\n' "$TITLE" + printf -- '--- body ---\n' + if [ "$verdict" = failure ]; then body_failure; else body_success; fi + exit 0 +fi + +# EXACT TITLE MATCH, not a search-relevance match. `gh issue list --search` is a +# full-text query and would find any issue mentioning these words -- including +# the ones the two of us have filed ABOUT this mechanism. +# A HIGH LIMIT ON THE PLAIN LIST, NOT A SEARCH, AND BOTH HALVES OF THAT WERE +# MEASURED RATHER THAN CHOSEN. +# +# `--limit 100` alone silently misses the target once more than a hundred issues +# are open: the lookup returns empty and every red opens a NEW issue, which is +# precisely "a notifier people filter" -- the thing this design rests on not being +# (@OffgridwithJD). +# +# `--search` fixes that and introduces a worse one. GitHub's search index is +# EVENTUALLY CONSISTENT: measured here, immediately after closing an issue the +# search still reported it open, and caught up seconds later. A lag in the other +# direction misses a freshly-opened issue and opens a duplicate -- the same +# failure, arriving by a different route, and this mechanism can run twice in +# quick succession because `concurrency: nightly` queues rather than cancels. +# +# `gh issue list` without `--search` reads the REST collection, which is strongly +# consistent, and gh paginates past 100 on its own. So a high limit has neither +# problem. The exact match stays client-side: `--search` would also have matched +# the issues the two of us have filed ABOUT this mechanism. +existing="$(gh issue list --state open --limit 1000 --json number,title \ + --jq "map(select(.title == \"$TITLE\")) | .[0].number // empty")" + +if [ "$verdict" = failure ]; then + if [ -n "$existing" ]; then + gh issue comment "$existing" --body "$(body_failure)" + echo "nightly-report: commented on #$existing" + else + gh issue create --title "$TITLE" --body "$(body_failure)" + echo "nightly-report: opened a new issue" + fi +else + if [ -n "$existing" ]; then + gh issue comment "$existing" --body "$(body_success)" + gh issue close "$existing" --reason completed + echo "nightly-report: closed #$existing" + else + echo "nightly-report: green, and nothing open to close" + fi +fi diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 508b5324..39c623a7 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -32,6 +32,11 @@ on: schedule: - cron: "0 6 * * *" # nightly, 06:00 UTC workflow_dispatch: + inputs: + fire_drill: + description: "Exercise the red-nightly reporter without breaking anything" + type: boolean + default: false permissions: contents: read @@ -47,7 +52,12 @@ jobs: name: suites (PG ${{ matrix.pg }}, ${{ matrix.runner == 'ubuntu-24.04-arm' && 'aarch64' || 'x86_64' }}) runs-on: ${{ matrix.runner }} timeout-minutes: 60 - if: github.event_name == 'workflow_dispatch' || github.repository == 'commandprompt/pgcolumnar' + # A FIRE DRILL SKIPS THE HEAVY GATES. The drill exists to prove the reporter + # fires, and a proof that costs a full deep gate is one nobody re-runs -- which + # is how a notifier goes back to never having been seen to fire. `skipped` is + # not a failure to the reporter, so the drill's verdict comes from the input. + if: (github.event_name == 'workflow_dispatch' || github.repository == 'commandprompt/pgcolumnar') + && github.event.inputs.fire_drill != 'true' strategy: fail-fast: false # The full packaged matrix on x86_64, and the current major on aarch64. @@ -202,7 +212,12 @@ jobs: name: sanitizer gate (ASAN+UBSAN) runs-on: ubuntu-latest timeout-minutes: 120 - if: github.event_name == 'workflow_dispatch' || github.repository == 'commandprompt/pgcolumnar' + # A FIRE DRILL SKIPS THE HEAVY GATES. The drill exists to prove the reporter + # fires, and a proof that costs a full deep gate is one nobody re-runs -- which + # is how a notifier goes back to never having been seen to fire. `skipped` is + # not a failure to the reporter, so the drill's verdict comes from the input. + if: (github.event_name == 'workflow_dispatch' || github.repository == 'commandprompt/pgcolumnar') + && github.event.inputs.fire_drill != 'true' env: PG_VERSION: "18.4" SAN_PREFIX: /home/runner/pg_san @@ -329,7 +344,12 @@ jobs: name: coverage report (PG 18) runs-on: ubuntu-latest timeout-minutes: 90 - if: github.event_name == 'workflow_dispatch' || github.repository == 'commandprompt/pgcolumnar' + # A FIRE DRILL SKIPS THE HEAVY GATES. The drill exists to prove the reporter + # fires, and a proof that costs a full deep gate is one nobody re-runs -- which + # is how a notifier goes back to never having been seen to fire. `skipped` is + # not a failure to the reporter, so the drill's verdict comes from the input. + if: (github.event_name == 'workflow_dispatch' || github.repository == 'commandprompt/pgcolumnar') + && github.event.inputs.fire_drill != 'true' steps: - uses: actions/checkout@v4 @@ -488,7 +508,12 @@ jobs: name: extension upgrade guard (PG 18) runs-on: ubuntu-latest timeout-minutes: 45 - if: github.event_name == 'workflow_dispatch' || github.repository == 'commandprompt/pgcolumnar' + # A FIRE DRILL SKIPS THE HEAVY GATES. The drill exists to prove the reporter + # fires, and a proof that costs a full deep gate is one nobody re-runs -- which + # is how a notifier goes back to never having been seen to fire. `skipped` is + # not a failure to the reporter, so the drill's verdict comes from the input. + if: (github.event_name == 'workflow_dispatch' || github.repository == 'commandprompt/pgcolumnar') + && github.event.inputs.fire_drill != 'true' # Named ONCE. Two steps consume it, and a tag spelled separately in each is # the kind of duplication that goes stale in one place and not the other. env: @@ -575,3 +600,84 @@ jobs: echo "::error::the guard skipped for want of an old source; in this job that is a failure" fi exit "$rc" + + # ---- make a red nightly findable without knowing to look (#973) ----------- + # + # The deep gate ran red two nights and nothing surfaced it; before that, 25 + # consecutive nights. A scheduled run has no pull request to be red on, the job + # that failed is named like a reporting step rather than a gate, and GitHub + # notifies the ACTOR of a schedule event -- whoever last touched this file, not + # whoever broke it. + # + # `if: always()` and not `if: failure()`, because the GREEN path is half the + # mechanism: an alert that must be closed by hand becomes one nobody closes. + # + # THE VERDICT COMES FROM needs.*.result, WHICH CANNOT BE LATE. The failing job + # NAMES come from the run's own API, because `needs` keys say `coverage` where a + # reader needs `coverage report (PG 18)` -- and if that call fails the verdict + # still stands and the body says no name was reported, rather than reporting a + # green. + red-nightly: + name: report a red nightly + runs-on: ubuntu-latest + needs: [suites, sanitizer, coverage, upgrade-guard] + if: always() && (github.event_name == 'workflow_dispatch' || github.repository == 'commandprompt/pgcolumnar') + permissions: + contents: read + issues: write + steps: + - uses: actions/checkout@v4 + - name: open, update or close the nightly's issue + env: + GH_TOKEN: ${{ github.token }} + # ONE LIST, NOT TWO. An earlier version named the four results in four + # environment variables, which meant `needs:` was guarded by selftest 450 + # and the verdict was computed from a second list guarded by nothing -- + # so a contributor adding a gate was compelled to update the guarded list + # and told nothing about the one that decided the verdict + # (@OffgridwithJD, who added a fifth job and got a fully green tree with + # a nightly whose failure this could not see). + # + # toJSON(needs) carries every entry, so there is nothing to keep in + # agreement. + NEEDS_JSON: ${{ toJSON(needs) }} + DRILL: ${{ github.event.inputs.fire_drill }} + run: | + set -euo pipefail + # THE DRILL SUBSTITUTES THE INPUT, NOT THE ANSWER. An earlier version set + # verdict=failure directly, which meant the drill proved delivery and + # never touched the decision -- and the decision is the half that can + # fail the #973 way. It now feeds a synthetic context through the same + # derivation every real night uses. + # THE REAL CONTEXT IS PRINTED FIRST, ALWAYS, INCLUDING ON A DRILL. + # The whole fix rests on toJSON(needs) carrying every entry, and that was + # an assumption about the platform rather than something anyone had seen. + # Printing it makes every run -- including the cheap drill -- an + # observation of it: a drill shows all four gates as `skipped`, so a gate + # missing from this line is visible without waiting for a real red. + echo "needs context as given: $NEEDS_JSON" + needs_json="$NEEDS_JSON" + if [ "${DRILL:-false}" = "true" ]; then + needs_json='{"fire-drill":{"result":"failure"}}' + echo "fire drill: substituting a failing context through the same derivation" + # AND THE ISSUE MUST SAY IT IS A DRILL. Drill 3 did not: rewriting this + # step dropped the marker, the job list came back empty, and the body + # read exactly like a genuine red. An alert that cries wolf teaches a + # reader to discount it, which is the state #973 exists to leave. + drill_name="fire drill for #973: no gate actually failed" + fi + echo "verdict computed from: $needs_json" + + names=() + while IFS= read -r n; do [ -n "$n" ] && names+=("$n"); done < <( + # SELECT WHAT FAILED, NOT WHAT DID NOT SUCCEED. The first version + # filtered negatively and the first fire drill caught it: this job is + # still RUNNING when it asks, so it did not match "success" and listed + # ITSELF as a failing job. A positive list cannot admit a state nobody + # enumerated -- and where it OMITS one (neutral, action_required, + # stale), the reporter says no job name was reported rather than + # printing an empty list. + gh run view "$GITHUB_RUN_ID" --json jobs --jq '.jobs[] | select(.conclusion == "failure" or .conclusion == "cancelled" or .conclusion == "timed_out") | .name' 2>/dev/null || true) + + [ -n "${drill_name:-}" ] && names+=("$drill_name") + .github/scripts/nightly-report.sh --from-needs "$needs_json" ${names+"${names[@]}"} diff --git a/docs/testing.md b/docs/testing.md index eb1d9bf5..bafa3754 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -348,6 +348,10 @@ instrumented PostgreSQL, and the coverage report. It also runs the extension-upgrade guard. That guard builds the previous release on PostgreSQL 18, loads data into it, and upgrades it in place. The sanitizer build stays in a cache, because it takes longer to build than to run the suites against it. +A final job, red-nightly, opens an issue when any of those fail. The next green +run closes it. A scheduled run has no pull request to be red on, so nothing else +surfaces a failure here. This gate once ran red for 25 consecutive nights behind +a green per-PR gate. The aarch64 run executes the suites rather than only building them. Misaligned reads, the class most often expected to differ by architecture, are already diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index ef8e0c5c..b885d8b6 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -813,6 +813,36 @@ harness_selftest 440-a-count-grep-never-produced premise: nor is one inside a ge harness_selftest 440-a-count-grep-never-produced premise: the sweep has a corpus to read never - harness_selftest 440-a-count-grep-never-produced premise: while a numeric comparison is not an offence never - harness_selftest 440-a-count-grep-never-produced premise: while a valid pattern prints a number never - +harness_selftest 450-a-red-nightly-must-be-findable a cancelled gate is a red verdict, not a shrug never - +harness_selftest 450-a-red-nightly-must-be-findable a failure names the run so a reader can reach it never - +harness_selftest 450-a-red-nightly-must-be-findable a failure naming no job says so rather than printing an empty list never - +harness_selftest 450-a-red-nightly-must-be-findable a fire drill identifies itself in the issue it opens never - +harness_selftest 450-a-red-nightly-must-be-findable a gate nobody listed here still decides the verdict (#973) never - +harness_selftest 450-a-red-nightly-must-be-findable a green run composes the closing message, so the alert clears itself never - +harness_selftest 450-a-red-nightly-must-be-findable all skipped is green, which is a fork run and a fire drill never - +harness_selftest 450-a-red-nightly-must-be-findable an empty needs context is refused, not defaulted to green never - +harness_selftest 450-a-red-nightly-must-be-findable an unparseable context says so without a traceback never - +harness_selftest 450-a-red-nightly-must-be-findable an unrecognised result is refused rather than treated as green never - +harness_selftest 450-a-red-nightly-must-be-findable an unrecognised verdict is refused rather than treated as green never - +harness_selftest 450-a-red-nightly-must-be-findable and it is granted issues: write, which the workflow default withholds never - +harness_selftest 450-a-red-nightly-must-be-findable and it still says what went wrong, rather than saying nothing never - +harness_selftest 450-a-red-nightly-must-be-findable and names the job that failed, not just that something did never - +harness_selftest 450-a-red-nightly-must-be-findable and no per-job result variable survives to drift from the context never - +harness_selftest 450-a-red-nightly-must-be-findable and no search-index query survives in the code never - +harness_selftest 450-a-red-nightly-must-be-findable and that marker reaches the job list the reporter prints never - +harness_selftest 450-a-red-nightly-must-be-findable and the green path reuses the same title, or it would close nothing never - +harness_selftest 450-a-red-nightly-must-be-findable every gate green is a green verdict never - +harness_selftest 450-a-red-nightly-must-be-findable one gate failing is a red verdict never - +harness_selftest 450-a-red-nightly-must-be-findable premise: removing comments left the lookup itself in place never - +harness_selftest 450-a-red-nightly-must-be-findable premise: the nightly workflow and the reporter are both present never - +harness_selftest 450-a-red-nightly-must-be-findable premise: the other nightly jobs were parsed, so the comparison is real never - +harness_selftest 450-a-red-nightly-must-be-findable premise: the per-job-variable pattern sees that shape when it is present never - +harness_selftest 450-a-red-nightly-must-be-findable premise: the reporter's needs list was parsed never - +harness_selftest 450-a-red-nightly-must-be-findable the issue lookup reads past the first hundred open issues never - +harness_selftest 450-a-red-nightly-must-be-findable the reporter runs on every outcome, not only on failure never - +harness_selftest 450-a-red-nightly-must-be-findable the reporter waits on EVERY other nightly job (#973) never - +harness_selftest 450-a-red-nightly-must-be-findable the title carries no run number, so every red lands on one issue never - +harness_selftest 450-a-red-nightly-must-be-findable the verdict comes from the whole needs context, not a second list never - native_join_runtime_filter native_join_runtime_filter 3-table answer equals filter-off never - native_join_runtime_filter native_join_runtime_filter 3-table join order matches filter-off never - native_join_runtime_filter native_join_runtime_filter 3-table plan has coordinator never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 81741bf1..63961bdf 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -34,4 +34,4 @@ suites_not_covered 250 # Without that it is a hand-maintained count that drifts, which is the failure # this repository has spent a day proving. It is not a ceiling; it is a # measurement that must be true. -checks_never_observed_red 859 +checks_never_observed_red 889 diff --git a/test/selftest/450-a-red-nightly-must-be-findable.sh b/test/selftest/450-a-red-nightly-must-be-findable.sh new file mode 100644 index 00000000..7f24888d --- /dev/null +++ b/test/selftest/450-a-red-nightly-must-be-findable.sh @@ -0,0 +1,203 @@ +# ---- a red nightly must be findable without knowing to look ---------------- +# +# #973. The deep gate ran red two nights and nothing surfaced it; before that, +# 25 consecutive nights. Three properties compound and each is reasonable alone: +# a scheduled run has no pull request to be red on, the job that failed is named +# like a reporting step rather than a gate, and GitHub notifies the ACTOR of a +# schedule event -- whoever last touched the workflow file, not whoever broke it. +# +# So the workflow carries a `red-nightly` job that opens one issue on the first +# red, updates it on each red after, and CLOSES IT on the next green. +# +# WHAT THIS PART CAN AND CANNOT SEE, said first because the gap is the point of +# #973. It drives the reporter's composition through its dry run, and it asserts +# the WIRING structurally. It cannot see the issue being created, because that +# needs a live run -- and "a notifier never seen to fire is indistinguishable +# from none" is this issue's own sentence. The firing is demonstrated once, by a +# fire drill on a branch, and recorded in the PR rather than claimed here. + +_rn_wf="$TESTDIR/../.github/workflows/nightly.yml" +_rn_sh="$TESTDIR/../.github/scripts/nightly-report.sh" + +check "premise: the nightly workflow and the reporter are both present" \ + "$([ -r "$_rn_wf" ] && [ -x "$_rn_sh" ] && echo yes || echo no)" "yes" + +# ---- the wiring, derived from the workflow rather than listed here ---------- +# +# A job added later and NOT added to `needs` would be invisible to the reporter, +# and the reporter would report green while it burned. So the claim is +# set equality against every other job key, computed from the file. +_rn_jobs="$(awk '/^jobs:/{inj=1;next} inj && /^ [a-zA-Z_-]+:[[:space:]]*$/{ + k=$1; sub(/:$/,"",k); print k}' "$_rn_wf" | grep -v '^red-nightly$' | sort)" +_rn_n=$(printf '%s\n' "$_rn_jobs" | grep -c .) +check "premise: the other nightly jobs were parsed, so the comparison is real" \ + "$([ "${_rn_n:-0}" -ge 3 ] && echo yes || echo no)" "yes" + +# `needs: [a, b, c]` on one line, which is how the job is written. +_rn_needs="$(sed -n 's/^ needs: \[\(.*\)\]$/\1/p' "$_rn_wf" | + tr ',' '\n' | tr -d ' ' | grep -v '^$' | sort)" +check "premise: the reporter's needs list was parsed" \ + "$([ -n "$_rn_needs" ] && echo yes || echo empty)" "yes" + +check "the reporter waits on EVERY other nightly job (#973)" \ + "$(printf '%s' "$_rn_jobs" | md5sum | cut -c1-12)" \ + "$(printf '%s' "$_rn_needs" | md5sum | cut -c1-12)" + +# ---- ONE LIST, and the verdict driven rather than read --------------------- +# +# The first version computed the verdict in the workflow from four named +# environment variables. That made two lists: `needs:`, guarded by the arm above, +# and the env block, guarded by nothing -- so a contributor adding a gate was +# COMPELLED to update the guarded one and told nothing about the one that decided +# the verdict. @OffgridwithJD added a fifth job to a copy of this branch, +# registered it in `needs` as the arm above demands and named it in testing.md as +# 240 demands, and got a fully green tree with a nightly gate whose failure the +# reporter could not see. #973 reintroduced inside the fix for #973. +# +# `toJSON(needs)` carries every entry, so there is nothing to keep in agreement. +# The arms below drive the derivation, which the old shape could only be read. +check "the verdict comes from the whole needs context, not a second list" \ + "$(grep -c 'NEEDS_JSON: \${{ toJSON(needs) }}' "$_rn_wf")" "1" +# A ZERO-COUNT ARM CARRIES ITS OWN POSITIVE CASE. A pattern that matches nothing +# and a tree that contains nothing both report 0, and only one of those is the +# claim. This reintroduces the old shape into a copy and requires the pattern to +# see it -- which is not hypothetical care: probing this very file with an +# UNESCAPED `${{` read 0 against a line that plainly contains it, because `$` +# before `{` is not literal to GNU grep. That one reddened. In a zero-count arm +# the same slip passes. +_rn_probe="$(mktemp)"; cp "$_rn_wf" "$_rn_probe" +printf ' SUITES: ${{ needs.suites.result }}\n' >> "$_rn_probe" +check "premise: the per-job-variable pattern sees that shape when it is present" \ + "$(grep -cE '^ +[A-Z]+: \$\{\{ needs\.[a-z-]+\.result \}\}' "$_rn_probe")" "1" +check "and no per-job result variable survives to drift from the context" \ + "$(grep -cE '^ +[A-Z]+: \$\{\{ needs\.[a-z-]+\.result \}\}' "$_rn_wf")" "0" +rm -f "$_rn_probe" + +_rn_v() { # _rn_v JSON -> the derived verdict + PGC_NIGHTLY_REPORT_DRYRUN=1 bash "$_rn_sh" --from-needs "$1" 2>&1 | + sed -n 's/^verdict: //p' +} +check "every gate green is a green verdict" \ + "$(_rn_v '{"a":{"result":"success"},"b":{"result":"success"}}')" "success" +check "one gate failing is a red verdict" \ + "$(_rn_v '{"a":{"result":"success"},"b":{"result":"failure"}}')" "failure" +check "a cancelled gate is a red verdict, not a shrug" \ + "$(_rn_v '{"a":{"result":"success"},"b":{"result":"cancelled"}}')" "failure" +check "all skipped is green, which is a fork run and a fire drill" \ + "$(_rn_v '{"a":{"result":"skipped"},"b":{"result":"skipped"}}')" "success" + +# THE ARM THAT MAKES THE ONE-LIST CLAIM REAL. A job this file has never heard of +# is covered the day it is added, because the derivation reads the context rather +# than a list anyone maintains. Under the old shape this could not be expressed: +# the verdict came from four names and a fifth was invisible to it. +check "a gate nobody listed here still decides the verdict (#973)" \ + "$(_rn_v '{"a":{"result":"success"},"b":{"result":"success"},"brand-new-gate":{"result":"failure"}}')" \ + "failure" + +# A RESULT THE SCRIPT DOES NOT KNOW IS NOT A PASS. The platform growing a state +# while this did not is exactly how a reporter goes quiet. +check "an unrecognised result is refused rather than treated as green" \ + "$(_rn_v '{"a":{"result":"brand_new_state"}}')" "failure" + +# An empty context means the job list changed shape. Reporting green on that is +# the failure this whole file exists to remove. +PGC_NIGHTLY_REPORT_DRYRUN=1 bash "$_rn_sh" --from-needs '{}' >/dev/null 2>&1 && _rn_erc=0 || _rn_erc=$? +check "an empty needs context is refused, not defaulted to green" \ + "$_rn_erc" "3" + +# A REFUSAL MUST READ AS A REFUSAL, NOT AS A CRASH. The parse dumped a Python +# traceback before the script's own message (@OffgridwithJD). Behaviour was right +# -- rc 3, never green -- but in a CI log a traceback sends the reader after a bug +# in the reporter instead of after the job list changing shape, and this +# mechanism's whole value is that a reader can tell at a glance what happened. +_rn_noise="$(PGC_NIGHTLY_REPORT_DRYRUN=1 bash "$_rn_sh" --from-needs 'not json' 2>&1 || true)" +check "an unparseable context says so without a traceback" \ + "$(printf '%s' "$_rn_noise" | grep -c 'Traceback\|JSONDecodeError')" "0" +check "and it still says what went wrong, rather than saying nothing" \ + "$(printf '%s' "$_rn_noise" | grep -c 'empty or unparseable')" "1" + +# A DRILL MUST SAY IT IS A DRILL. Drill 3 did not: rewriting the step to derive +# the verdict from the needs context dropped the marker, the job list came back +# empty, and the issue body read exactly like a genuine red. An alert that cries +# wolf teaches a reader to discount it, which is the state #973 exists to leave. +check "a fire drill identifies itself in the issue it opens" \ + "$(grep -c 'fire drill for #973: no gate actually failed' "$_rn_wf")" "1" +check "and that marker reaches the job list the reporter prints" \ + "$(grep -c 'names+=("\$drill_name")' "$_rn_wf")" "1" + +# ---- the issue lookup must not go blind on a busy tracker ------------------ +# +# `--limit 100` silently misses the target once more than a hundred issues are +# open: the lookup returns empty and every red opens a NEW issue, the "notifier +# people filter" this design rests on not being (@OffgridwithJD). +# +# AND `--search` IS THE WRONG FIX, measured rather than reasoned: GitHub's search +# index is eventually consistent, and immediately after a close it still reported +# the issue open. A lag the other way misses a freshly-opened issue and opens a +# duplicate -- the same failure by another route. The plain list reads the REST +# collection, which is strongly consistent, and gh paginates past 100 itself. +# COUNTED OVER CODE, NOT OVER THE FILE. `--search` appears FOUR times in this +# script and zero times as a call -- the comments record why the obvious fix is +# wrong, which is the most valuable kind of comment and also the kind that +# satisfies a grep wanting the call (@OffgridwithJD, in the same file that taught +# us both that). Counting `in:title` over the whole file happens to give 0 only +# because the prose says `--search` and never says `in:title`. That is luck. +_rn_code="$(mktemp)" +grep -v '^[[:space:]]*#' "$_rn_sh" > "$_rn_code" +# AND THE STRIP MUST NOT HAVE STRIPPED THE CODE. A grep over an emptied file +# reports the same 0 as a grep that matched nothing. +check "premise: removing comments left the lookup itself in place" \ + "$(grep -c 'gh issue list --state open' "$_rn_code")" "1" +check "the issue lookup reads past the first hundred open issues" \ + "$(grep -c 'gh issue list --state open --limit 1000' "$_rn_code")" "1" +check "and no search-index query survives in the code" \ + "$(grep -cE -- '--search|in:title' "$_rn_code")" "0" +rm -f "$_rn_code" + +# It must run on failure AND on success, or the close path never happens. +check "the reporter runs on every outcome, not only on failure" \ + "$(grep -c '^ if: always() &&' "$_rn_wf")" "1" + +# Opening an issue needs the permission, and the workflow's default is read-only. +check "and it is granted issues: write, which the workflow default withholds" \ + "$(awk '/^ red-nightly:/{p=1} p&&/issues: write/{print "yes"; exit}' "$_rn_wf")" "yes" + +# ---- the reporter's own composition, driven rather than read --------------- + +_rn_fail="$(PGC_NIGHTLY_REPORT_DRYRUN=1 PGC_NIGHTLY_RUN_URL="https://example/run/7" \ + bash "$_rn_sh" failure "coverage report (PG 18)" 2>&1)" + +check "a failure names the run so a reader can reach it" \ + "$(printf '%s' "$_rn_fail" | grep -c 'https://example/run/7')" "1" +check "and names the job that failed, not just that something did" \ + "$(printf '%s' "$_rn_fail" | grep -c 'coverage report (PG 18)')" "1" + +# ONE ISSUE, NOT ONE PER RUN. A notifier that files a new issue nightly is one +# people filter, and a filtered notifier is the state this replaces. The title is +# what makes updates land on the same issue, so it must carry nothing variable. +check "the title carries no run number, so every red lands on one issue" \ + "$(printf '%s' "$_rn_fail" | sed -n 's/^title: //p' | grep -c '[0-9]')" "0" + +_rn_ok="$(PGC_NIGHTLY_REPORT_DRYRUN=1 PGC_NIGHTLY_RUN_URL="https://example/run/8" \ + bash "$_rn_sh" success 2>&1)" +check "a green run composes the closing message, so the alert clears itself" \ + "$(printf '%s' "$_rn_ok" | grep -c 'green again')" "1" +check "and the green path reuses the same title, or it would close nothing" \ + "$(printf '%s' "$_rn_ok" | sed -n 's/^title: //p')" \ + "$(printf '%s' "$_rn_fail" | sed -n 's/^title: //p')" + +# A VERDICT IT DOES NOT UNDERSTAND MUST NOT READ AS GREEN. If the caller changes +# and this does not, treating the unknown value as success restores exactly the +# silence #973 is about. +PGC_NIGHTLY_REPORT_DRYRUN=1 bash "$_rn_sh" probably >/dev/null 2>&1 && _rn_rc=0 || _rn_rc=$? +check "an unrecognised verdict is refused rather than treated as green" \ + "$_rn_rc" "2" + +# A failure with no job name must say so rather than print an empty list, which +# a reader parses as "nothing failed". +_rn_bare="$(PGC_NIGHTLY_REPORT_DRYRUN=1 bash "$_rn_sh" failure 2>&1)" +check "a failure naming no job says so rather than printing an empty list" \ + "$(printf '%s' "$_rn_bare" | grep -c 'No job name was reported')" "1" + +unset _rn_wf _rn_sh _rn_jobs _rn_n _rn_needs _rn_fail _rn_ok _rn_rc _rn_bare _rn_erc _rn_probe _rn_noise _rn_code +unset -f _rn_v diff --git a/test/selftest/parts.manifest b/test/selftest/parts.manifest index 05fa1d47..3d87c0a5 100644 --- a/test/selftest/parts.manifest +++ b/test/selftest/parts.manifest @@ -41,3 +41,4 @@ 420-a-deleted-part-must-be-visible.sh 430-the-self-test-must-not-report.sh 440-a-count-grep-never-produced.sh +450-a-red-nightly-must-be-findable.sh