forked from heavy-duty/ceremony
cluade-reviewer-andresmgsl
a35a77f752
@codex-reviewer-andresmgsl caught a test that describes evidence it does not collect — mine, and it is the class this PR is about. Two cases were titled "naming the verb, path and status" and asserted only the substring "500". The PUT boundary happened to satisfy the contract because forgejo_write already passes "PUT $endpoint" to forgejo_http_ok. The GET boundary did not: the diagnostic was `HTTP 500 from 'repos/o/r/issues/5'`, with no verb at all — so a caller could not tell a failed READ from a failed WRITE of the same path, and #192's acceptance criterion asks for exactly that distinction. Reads now pass "GET $endpoint" on both non-paginated and paginated paths, and the two tests assert the complete expected diagnostic as one substring rather than a status code that any failure would contain. Reverting the verb reds the GET case. Also, per the same review: the failed GET is asserted to write nothing, and the failed PUT to have attempted exactly one write. forge-backends 117/117 (was 115), test/run.sh 22/22 under jq 1.7 and jq 1.6, shellcheck 0.10.0 and actionlint clean. Refs #192
47 lines
2.2 KiB
Markdown
47 lines
2.2 KiB
Markdown
### Fixed
|
|
|
|
- Label removal on Forgejo is a full-set `PUT`, not a per-label `DELETE`. The
|
|
workflow token gets HTTP 500 on every `DELETE .../labels/{id}` on this
|
|
instance, so the state machine could only ever ADD labels (#192).
|
|
|
|
- Every `state:*` transition that needs the previous state cleared, and every
|
|
`blocker:*` that should lift, can now actually clear. They were inert (#192).
|
|
|
|
- A label edit that fails is fatal to `labels-reconcile`, matching
|
|
`issueflow-reconcile`. One cause had two contradictory policies (#192).
|
|
|
|
- A failed write reaches the sweep's exit code: per-PR tolerance is kept for
|
|
READS, but a sweep that could not write exits non-zero and its output carries
|
|
no `reconciled.` token at all (#192).
|
|
|
|
- Every label mutation goes through one checked helper, so clearing
|
|
`merge-next` or either `stale` edit fails the sweep too — not only the
|
|
primary state edit (#192).
|
|
|
|
- A preserved label keeps the id the issue payload already carried, so
|
|
preservation does not depend on a repository-wide list that has nothing to do
|
|
with the issue (#192).
|
|
|
|
- A removal that changes nothing writes nothing, rather than replacing the set
|
|
with itself and opening a race for no state change (#192).
|
|
|
|
- Every failure diagnostic on the forgejo backend names the verb as well as the
|
|
path and the status. A read used to say `HTTP 500 from 'repos/…'`, which
|
|
cannot be told from a failed write of the same path (#192).
|
|
|
|
- The diagnostic names what was attempted and that it did not happen, instead
|
|
of blaming a missing label and telling the operator to bootstrap — a cause it
|
|
had not established (#192, #101).
|
|
|
|
- An add-label the repo does not carry refuses before any write, so a
|
|
replacement `PUT` can never drop a label nobody asked to remove (#192).
|
|
|
|
### Added
|
|
|
|
- `test/forge-backends.test.sh` pins the replacement contract: preserve
|
|
unrelated labels across a combined add+remove, an absent removal as a
|
|
successful no-op, the empty set as a full clear, and `forge_labels_add`
|
|
still `POST`-only, per ceremony#128 (#192).
|
|
|
|
- `test/labels-reconcile.test.sh` drives a failing write through `main()` — the
|
|
swallow was in the loop, where a fixture-level probe cannot reach (#192).
|