From 4c64aa2106765c68974c17524b99517817f293cf Mon Sep 17 00:00:00 2001 From: glenn schrooyen Date: Tue, 1 Sep 2026 03:36:19 +0200 Subject: [PATCH] CI-02 probe E: the PROPOSED gate, on a pull request that does not merge --- .gitea/ci/Dockerfile | 14 +- .gitea/workflows/checks.yml | 312 +++++++++++++++++++++++++++++++----- 2 files changed, 286 insertions(+), 40 deletions(-) diff --git a/.gitea/ci/Dockerfile b/.gitea/ci/Dockerfile index 2e8c778..d87adf1 100644 --- a/.gitea/ci/Dockerfile +++ b/.gitea/ci/Dockerfile @@ -18,5 +18,17 @@ # python3, no aiohttp, no pip (`No module named pip`; checked on the box, not # assumed). An image is not gold-plating here, it is the only option that does # not mutate a shared long-lived runner. +# ⚠️ `git` IS A TEST DEPENDENCY, NOT A CONVENIENCE (CI-02). test_ci_gate.py +# grades the gate's own checkout by EXTRACTING the shell out of +# .gitea/workflows/checks.yml and RUNNING it - against two throwaway local +# repositories it builds itself, one whose pull request merges and one whose +# does not. Reading those commands as a string would be the thing this +# repository keeps finding: a check that never observed its subject. Running +# them needs git in the image the checks run in. +# +# It costs one apk package and no network at test time: the fixtures are local +# paths, and the token substitution in the workflow's `git remote add` is a +# no-op on a path with no `://` in it, which is exactly why the test can use +# the shipped line verbatim instead of a paraphrase of it. FROM alpine:3 -RUN apk add --no-cache python3 py3-aiohttp py3-paho-mqtt +RUN apk add --no-cache python3 py3-aiohttp py3-paho-mqtt git diff --git a/.gitea/workflows/checks.yml b/.gitea/workflows/checks.yml index 3ddf4a2..bbaeea7 100644 --- a/.gitea/workflows/checks.yml +++ b/.gitea/workflows/checks.yml @@ -1,9 +1,26 @@ -# CI-02 PROBE - THROWAWAY. Trial 2: CI-01's per-PR concurrency group kept, but -# cancel-in-progress FLIPPED TO false. Trial 1 (no block at all) left a queued -# run alive across two further pushes. This trial asks the narrower question the -# fix actually turns on: does the group ALONE kill a queued run, the way GitHub -# documents ("any previously pending job in the group will be cancelled"), or -# was cancel-in-progress: true the whole cause? +# CI-01: this repository's FIRST automated check. Before this file, admin/goodwe-addon +# had no `.gitea/workflows/` directory at all - eight runnable test files, ~100 s +# of real coverage including a wall-clock cadence test, and not one machine ever +# ran them. Every merge here was a human reading a diff. +# +# ⚠️ NO `paths:` FILTER, AND ITS ABSENCE IS LOAD-BEARING. Branch protection on +# release/1.0 requires the context `checks / unit (pull_request)`. Gitea leaves a +# required context it never receives PENDING forever, so the moment this workflow +# learns to sit a PR out, that PR becomes unmergeable. Condition a STEP if you +# must; never the workflow. +# +# ⚠️ THE RUNNER IS SHARED WITH THE OTHER REPO'S RIG. `goodwe-runner` on +# 192.168.2.147 carries the labels `self-hosted` and `goodwe-rig` and is +# registered `:host`, and it is the only runner there is - so this job wears the +# rig label without wanting a rig. Nothing below starts, stops, or reads the +# `goodwe-sim` compose project or takes `sim/riglock.py`; it runs one short +# container and exits. It does occupy the runner's single slot (`capacity: 1`), +# so it QUEUES behind a rig campaign - a delayed verdict, never a destroyed one. +# +# NO JS ACTIONS: the runner image has no node, so `actions/checkout` cannot run. +# Checkout is plain git below - and since CI-02 it is a checkout of the MERGE of +# the pull request into its base, not of the branch tip. See the long note on +# that step; a green from this workflow is a claim about content that can land. name: checks on: @@ -11,57 +28,274 @@ on: branches: [release/1.0, master] workflow_dispatch: -concurrency: - group: addon-checks-${{ github.event.pull_request.number || github.ref }} - cancel-in-progress: false +# ⚠️ NO `concurrency:` BLOCK, AND ITS ABSENCE IS AS LOAD-BEARING AS THE MISSING +# `paths:` FILTER ABOVE. CI-01 put one here - `group: addon-checks-`, +# `cancel-in-progress: true` - to stop a second pull request superseding a +# first one's run. It did stop that. It also did this, twice, on PR #14: +# +# run 531 created 17:08:19 `pending / Waiting to run` +# cancelled 17:10:44 <- the exact second run 532 was created +# run 533 created 17:45:11 `pending / Waiting to run` +# cancelled 18:11:22 <- the exact second run 534 was created +# +# Both cancelled runs have `started_at` at the Unix epoch: they never executed. +# A cancelled run posts `failure` on the required context, described +# `Canceled after 0s`. Branch protection compares STATE, not description, so +# "nothing ran" and "your tests are broken" arrive at the gate as the same red, +# and the correct operator response to each is the opposite of the other. +# +# CI-02 MEASURED THE THREE CANDIDATE SETTINGS ON A THROWAWAY PR (#29) RATHER +# THAN READING THE DOCS, because the docs and this Gitea disagree about which +# knob does it. In each trial one run was made to hold the single slot and two +# further pushes followed: +# +# trial 1, NO concurrency block - runs 561 and 562 both sat queued together +# and both executed. Nothing was cancelled. +# trial 2, group kept, cancel-in-progress: FALSE +# - run 564 sat `pending / Blocked by required +# conditions` and was cancelled at 0s the +# second run 565 was created. +# +# So `cancel-in-progress` IS NOT THE KNOB. A shared group cancels a run that has +# not started no matter what that flag says - GitHub documents this ("any +# previously pending run in the group will be cancelled") and Gitea 1.27 +# implements it. The only configuration that leaves a queued run alive is having +# no group at all, which is what is written here. +# +# ⚠️ DO NOT REPLACE THIS WITH A SHA-KEYED GROUP as a compromise. It looks +# tempting - two runs on different commits could never share a group, so nothing +# would be cancelled - but the only cancellations it still permits are the +# same-sha ones, and those are precisely the dangerous half: a red for a run +# that never started, landing on a commit that IS still the head. It +# concentrates the failure instead of removing it. +# +# WHAT THIS COSTS, stated honestly: the runner is `capacity: 1` and shared with +# the other repo's rig, so a developer who pushes four times in a minute now +# occupies it four times instead of once. That is CI-01's own trade written the +# other way round - it accepted "a delayed verdict, never a destroyed one" for +# the rig, and this accepts the same for pushes. A destroyed verdict here is +# worse than a slow one, because it is indistinguishable from a real failure. jobs: unit: runs-on: [self-hosted, goodwe-rig] - timeout-minutes: 20 + timeout-minutes: 70 # ~4 to run; the rest is queueing for the single slot env: - WS: /home/homelab/ci/ci02probe-${{ github.run_number }} - PRNUM: ${{ github.event.pull_request.number }} - BASEREF: ${{ github.base_ref }} - TOKEN: ${{ secrets.GITHUB_TOKEN }} - URL: ${{ github.server_url }}/${{ github.repository }}.git + # ⚠️ UNDER /home/homelab, NOT $GITHUB_WORKSPACE (ENV-01, and re-measured + # during CI-01). act's workspace is `/root/.cache/act//hostexecutor`, a + # path that exists only INSIDE the runner container. Hand it to the host's + # snap-confined dockerd and the build fails with "failed to read + # dockerfile", or - worse, and this is the documented case - a bind mount + # resolves to an EMPTY DIRECTORY with no error at all. /home/homelab is + # mirrored 1:1 into the runner and is under $HOME, where snap confinement + # accepts it. The same probe under /tmp failed and under $HOME succeeded. + WS: /home/homelab/ci/addon-checks-${{ github.run_number }} steps: - - name: Q00 an empty clone + # ⚠️ THE CHECKOUT IS TWO LEVELS BELOW THE MOUNT ROOT, DELIBERATELY. + # test_freeze.py's find_hwsim() reads `HERE.parents[1] / "goodwe" / "sim"`, + # looking for the infra repo as a SIBLING of this one. Check out flat - + # goodwe_controller directly at the mount root - and `parents[1]` does not + # exist, so the test dies on `IndexError: 1` before its first assertion. + # Measured during CI-01; the flat layout was tried first and did exactly + # that. $WS/goodwe-addon/goodwe_controller gives parents[1] == $WS. + # + # ⚠️ WHAT THIS CHECKS OUT IS THE MERGE, NOT THE BRANCH, AND THAT IS THE + # WHOLE OF CI-02. What was here before was one line: + # + # git fetch -q --depth 1 origin "$GITHUB_REF" \ + # || git fetch -q --depth 1 origin "$GITHUB_SHA" + # + # CI-02 measured what those two names actually are on this Gitea, on a + # deliberately conflicted throwaway PR (#29, runs 558 and 559), by asking + # one question per STEP - the Actions log endpoints 404 here, so a step's + # own conclusion is the only channel out of a job. The answers: + # + # GITHUB_REF == refs/pull//head (step P01 passed) + # GITHUB_REF != refs/pull//merge (step P02 failed) + # GITHUB_SHA == the pull request head sha (step P03 passed) + # fetching GITHUB_REF succeeds (step P04 passed) + # ...and lands on the bare head (step P05 passed) + # refs/pull//merge does not exist (step P06 failed) + # + # ⚠️ SO THE FALLBACK NEVER FIRED, AND THE DEFECT IS WORSE THAN THE ONE + # REPORTED. The ticket says a conflicted PR loses its merge ref and the + # `||` quietly grades the bare head instead. It does not: this Gitea has + # no merge refs at all - `git ls-remote origin 'refs/pull/*'` advertises + # only `/head`, for open and closed pull requests alike - so the FIRST + # half always succeeds, and both halves name the same commit anyway + # (GITHUB_REF resolves to GITHUB_SHA). The gate has never once graded + # merged content, on any pull request in this repository's history. Green + # has meant "this branch passed on its own", which on a base that has + # moved - and `block_on_outdated_branch` is off here, so it usually has - + # is not the content that lands. PR #14 reporting success while + # `mergeable: false` was not the fallback misfiring; it was the gate + # working exactly as written. + # + # A fallback that cannot fire cannot be "kept for the case it was for", so + # it is gone from this path and kept, ANNOUNCED, on the one path that has + # no pull request to merge (workflow_dispatch, below). + # + # ⚠️ NOT `--depth 1` ANY MORE. A merge needs the merge base, and a shallow + # pair does not have one. Measured before changing it: a full fetch of + # this repository is 6.8 MB and 1.1 s against this server, over 95 + # commits. The depth flag was buying nothing worth a wrong verdict. + # + # ⚠️ THE WORKFLOW AND THE CONTENT NOW COME FROM DIFFERENT TREES. Gitea + # picks the workflow file from refs/pull//head, so THIS file is always + # the branch's; the tree graded below - including .gitea/ci/Dockerfile - + # is the merge. That is the right way round (a pull request must not be + # able to grade itself with a gate the base has not seen) but it means a + # change to this file is tested by the branch it is on, which is how you + # are reading this. + - name: check out the base branch and the pull request head (git; no node for actions/checkout) + env: + URL: ${{ github.server_url }}/${{ github.repository }}.git + TOKEN: ${{ secrets.GITHUB_TOKEN }} + PRNUM: ${{ github.event.pull_request.number }} + BASEREF: ${{ github.base_ref }} run: | set -eu - rm -rf "$WS" && mkdir -p "$WS" && cd "$WS" + rm -rf "$WS" && mkdir -p "$WS/goodwe-addon" && cd "$WS/goodwe-addon" git init -q . git remote add origin "$(echo "$URL" | sed "s#://#://x-access-token:${TOKEN}@#")" - echo "BASEREF=[$BASEREF] PRNUM=[$PRNUM]" + if [ -n "$PRNUM" ]; then + # Two fetches, not one, so that a failure to reach either ref fails + # HERE with the ref in the message, rather than inside the merge + # step where it would read as a conflict. `github.base_ref` being + # populated at all was checked on the probe PR too (step Q01). + git fetch -q origin "+refs/heads/$BASEREF:refs/remotes/origin/base" + git fetch -q origin "+refs/pull/$PRNUM/head:refs/remotes/origin/pr" + git checkout -qf refs/remotes/origin/base + echo "base $BASEREF $(git rev-parse refs/remotes/origin/base)" + echo "head refs/pull/$PRNUM/head $(git rev-parse refs/remotes/origin/pr)" + else + # NO PULL REQUEST (workflow_dispatch). There is nothing to merge + # into, so this path grades a bare ref and SAYS SO - which is all + # the old `||` ever needed to do. It is kept here because a branch + # can be deleted or renamed between the dispatch and the fetch, and + # unlike the pull_request path that is a case nobody has ruled out. + if git fetch -q origin "$GITHUB_REF"; then + echo "GRADED: the bare ref $GITHUB_REF - NOT merged content (no pull request in this event)." + else + git fetch -q origin "$GITHUB_SHA" + echo "GRADED: the bare commit $GITHUB_SHA - NOT merged content, and NOT $GITHUB_REF either, which would not fetch." + fi + git checkout -qf FETCH_HEAD + fi + echo "checked out $(git log --oneline -1)" - - name: Q01 github.base_ref is populated and its branch fetches ALONE - if: always() + # ⚠️ THIS STEP IS THE ONE THAT MAKES A GREEN MEAN SOMETHING. It exists as + # its OWN step rather than as three lines of the checkout because the + # Actions log endpoints 404 on this Gitea and + # /api/v1/repos/.../actions/runs//jobs - which carries a per-step + # conclusion - is the only machine-readable account of what a run did. A + # reader who sees this step red knows the pull request does not apply and + # that NO TEST RAN; a reader who sees the next two red knows the tests ran + # and spoke. Collapse them back together and that distinction is gone + # again, which is the shape of defect this whole ticket is about. + # + # The refusal is a FAILURE rather than a skip on purpose. "Reports + # nothing" would leave the required context pending forever, which this + # file's own header explains is unmergeable-forever; and a conflicted pull + # request cannot be merged by Gitea anyway, so the red blocks nothing that + # was not already blocked. What it does is stop a green being posted about + # content that cannot land. + - name: grade the merge, not the branch - refuse a pull request that does not apply + env: + PRNUM: ${{ github.event.pull_request.number }} + BASEREF: ${{ github.base_ref }} run: | set -eu - cd "$WS" - test -n "$BASEREF" - git fetch -q origin "+refs/heads/$BASEREF:refs/remotes/origin/base" - git rev-parse refs/remotes/origin/base + cd "$WS/goodwe-addon" + if [ -z "$PRNUM" ]; then + echo "no pull request in this event - nothing to merge, and the previous step said what it graded." + exit 0 + fi + if git -c user.email=checks@localhost -c user.name=checks \ + merge --no-edit refs/remotes/origin/pr; then + echo "GRADED: refs/pull/$PRNUM/head merged into $BASEREF" + echo " result $(git rev-parse HEAD)" + echo " tree $(git rev-parse 'HEAD^{tree}')" + else + git merge --abort 2>/dev/null || true + echo "REFUSED: refs/pull/$PRNUM/head does not merge into $BASEREF." + echo " NOTHING HAS BEEN TESTED. This is not a test failure - no test ran." + echo " Rebase or merge $BASEREF into the branch and push again." + exit 1 + fi - - name: Q02 the pull request head fetches ALONE - if: always() + - name: build the test image (shipped deps, from apk - see .gitea/ci/Dockerfile) run: | set -eu - cd "$WS" - git fetch -q origin "+refs/pull/$PRNUM/head:refs/remotes/origin/pr" - git rev-parse refs/remotes/origin/pr + docker build -q -t goodwe-addon-checks:1 "$WS/goodwe-addon/.gitea/ci" >/dev/null + docker run --rm goodwe-addon-checks:1 python3 -c \ + 'import aiohttp, paho.mqtt.client as m, sys; print("python", sys.version.split()[0], "aiohttp", aiohttp.__version__)' - - name: Q03 and only THEN does the merge itself refuse - if: always() + # ⚠️ THE FILE LIST IS DERIVED FROM THE TREE, NOT TYPED HERE. A hand-written + # list is how a new test file joins the repo and is never run - the exact + # shape of failure this ticket exists to close. The step globs `test_*.py`, + # prints the count, and REFUSES A COUNT OF ZERO: a loop over an empty glob + # exits 0 and would report a green gate that examined nothing. + # + # ⚠️ test_loop.py TAKES ~93 SECONDS OF WALL CLOCK ON PURPOSE (its own + # docstring says so, and says why: it drives the real `run_control()` + # against a real wall-clock floor, and mocking the clock would leave the + # assertion testing the mock). It is not hung. Do not "fix" it with a + # shorter timeout - a 60 s limit was tried during CI-01 and reported + # exactly the false hang the docstring predicts. + # + # ⚠️ WHAT THIS RUN DOES NOT COVER, stated because a pass here would + # otherwise over-claim. test_freeze.py's third section wants the infra + # repo's `sim/hwsim.py` to drive a real HTTP meter through the real + # transport. admin/goodwe is PRIVATE (checked via the API) and this + # workflow's GITHUB_TOKEN is minted for THIS repository only, so hwsim is + # absent here and `find_hwsim()` returns None. The rest of the file runs + # and passes. It does so SILENTLY - the file prints nothing to say the + # hwsim section was skipped - so "test_freeze.py: PASS" below is a weaker + # claim than it looks. That belongs to whoever owns test_freeze.py next; + # naming it here is the least this gate can do about it. + # ⚠️ PYTHONDONTWRITEBYTECODE=1 IS BELT AND BRACES, NOT A FIX FOR A LIVE + # HOLE. TEST-15 measured this runner's actual exposure and found none: the + # checkout below is bind mounted `:ro`, so the interpreter physically + # cannot write a __pycache__ into the tree it is grading - it fails + # SILENTLY, no error and exit status 0 - and every job `rm -rf`s $WS and + # clones afresh, so nothing survives from a previous job on this shared + # runner either. The variable is here because the `:ro` is the load-bearing + # part and it is one character wide: delete it while debugging a write and + # the gate quietly starts caching bytecode across a rebuild again. Note + # what it does NOT do, per TEST-15 part 2 - it stops bytecode being + # WRITTEN, never a stale one being READ. It is not a substitute for the + # purge in goodwe_controller/tools/mutation_run.sh. + - name: the add-on's own tests run: | set -eu - cd "$WS" - git fetch -q origin "+refs/heads/$BASEREF:refs/remotes/origin/base" - git fetch -q origin "+refs/pull/$PRNUM/head:refs/remotes/origin/pr" - git checkout -qf refs/remotes/origin/base - git -c user.email=ci@local -c user.name=ci merge --no-edit refs/remotes/origin/pr + docker run --rm -e PYTHONDONTWRITEBYTECODE=1 -v "$WS:/w:ro" goodwe-addon-checks:1 sh -eu -c ' + cd /w/goodwe-addon/goodwe_controller + set -- test_*.py + if [ "$1" = "test_*.py" ]; then + echo "NO test_*.py FOUND - the mount is empty or the layout moved. NOTHING has been checked." + exit 1 + fi + echo "$# test file(s) to run" + rc=0 + for f in "$@"; do + start=$(date +%s) + if python3 "$f" > /tmp/out 2>&1; then + echo "PASS $f ($(( $(date +%s) - start ))s)" + else + echo "FAIL $f ($(( $(date +%s) - start ))s)" + tail -40 /tmp/out + rc=1 + fi + done + exit $rc + ' - - name: Q99 clean up + - name: clean up if: always() - run: rm -rf "$WS" || true -# probe D3 - third push; does the WAITING run 564 survive under cancel-in-progress: false? + run: | + rm -rf "$WS" || true + # Root-owned by this runner inside a directory the `homelab` user owns, + # and sudo on that box is interactive-only, so the user cannot clean + # this up and the job that made it has to. + ls -1dt /home/homelab/ci/addon-checks-*/ 2>/dev/null | tail -n +6 | xargs -r rm -rf || true