From 2168e4ef9a1f173cafc3db958f6fbd929c0f7f1e Mon Sep 17 00:00:00 2001 From: cluade-reviewer-andresmgsl Date: Sun, 2 Aug 2026 19:50:37 +0000 Subject: [PATCH] test(forge): hermetic cases for the two forgejo edit asymmetries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The coverage owed with the call-site port (@grok-reviewer-andresmgsl #4741 note 2, #4751 item 2). Live scratch-repo evidence proved these work; these pin the request SHAPE so they keep working. - a removal resolves name -> numeric id, and never sends the name as the path segment (measured: DELETE .../labels/probe:one -> 422, DELETE .../labels/149 -> 204); - a removal of a label the repo does not have writes nothing, matching gh: the reconcilers call --remove-label unconditionally to converge state; - adds take names directly, one request, comma-separated values split as gh splits them; - an assignee removal PATCHes the SURVIVING list, because Forgejo sets assignees rather than adding and removing them — a naive translation would have cleared every other assignee as a side effect of removing one, which is what the mutation test proves is caught. Payloads are now compact JSON. They were pretty-printed, which spread a single write across several lines — harder to read in a log, and it hid the shape from any assertion matching a line. Refs #188 --- lib/forge-forgejo.sh | 12 ++++---- test/forge-backends.test.sh | 56 +++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 6 deletions(-) diff --git a/lib/forge-forgejo.sh b/lib/forge-forgejo.sh index b962c5b..97b11c9 100644 --- a/lib/forge-forgejo.sh +++ b/lib/forge-forgejo.sh @@ -276,7 +276,7 @@ forge_issue_edit() { if [ "${#add_labels[@]}" -gt 0 ]; then local payload - payload="$(printf '%s\n' "${add_labels[@]}" | jq -R . | jq -s '{labels: .}')" + payload="$(printf '%s\n' "${add_labels[@]}" | jq -R . | jq -sc '{labels: .}')" forgejo_write POST "repos/$REPO/issues/$n/labels" "$payload" >/dev/null || return 1 fi @@ -305,14 +305,14 @@ forge_issue_edit() { if [ "${#rm_assignees[@]}" -gt 0 ]; then want="$(grep -vxF -f <(printf '%s\n' "${rm_assignees[@]}") <<<"$want" || true)" fi - payload="$(printf '%s' "$want" | jq -R . | jq -s '{assignees: [.[] | select(. != "")]}')" + payload="$(printf '%s' "$want" | jq -R . | jq -sc '{assignees: [.[] | select(. != "")]}')" forgejo_write PATCH "repos/$REPO/issues/$n" "$payload" >/dev/null || return 1 fi } forge_issue_comment() { local n="${1:?forge_issue_comment: number required}" body="${2?forge_issue_comment: body required}" - forgejo_write POST "repos/$REPO/issues/$n/comments" "$(jq -n --arg b "$body" '{body: $b}')" >/dev/null + forgejo_write POST "repos/$REPO/issues/$n/comments" "$(jq -nc --arg b "$body" '{body: $b}')" >/dev/null } forge_pr_list() { @@ -359,7 +359,7 @@ forge_label_list() { forge_api --paginate "repos/$REPO/labels" --jq '.[].name'; # existing name aborts the bootstrap under set -e. forge_label_create() { local name="${1:?}" color="${2:?}" desc="${3:-}" ids id payload - payload="$(jq -n --arg n "$name" --arg c "$color" --arg d "$desc" '{name:$n,color:$c,description:$d}')" + payload="$(jq -nc --arg n "$name" --arg c "$color" --arg d "$desc" '{name:$n,color:$c,description:$d}')" ids="$(forgejo_label_ids)" || return 1 id="$(awk -F '\t' -v want="$name" '$1 == want { print $2; exit }' <<<"$ids")" if [ -n "$id" ]; then @@ -417,7 +417,7 @@ forge_labels_add() { shift [ "$#" -gt 0 ] || return 0 forgejo_write POST "repos/$REPO/issues/$n/labels" \ - "$(printf '%s\n' "$@" | jq -R . | jq -s '{labels: .}')" >/dev/null + "$(printf '%s\n' "$@" | jq -R . | jq -sc '{labels: .}')" >/dev/null } # forge_request_reviewer — ask for a verdict. @@ -436,5 +436,5 @@ forge_labels_add() { forge_request_reviewer() { local n="${1:?}" user="${2:?}" forgejo_write POST "repos/$REPO/pulls/$n/requested_reviewers" \ - "$(jq -n --arg u "$user" '{reviewers: [$u]}')" >/dev/null + "$(jq -nc --arg u "$user" '{reviewers: [$u]}')" >/dev/null } diff --git a/test/forge-backends.test.sh b/test/forge-backends.test.sh index 72fd95a..c7f15e7 100644 --- a/test/forge-backends.test.sh +++ b/test/forge-backends.test.sh @@ -274,6 +274,62 @@ check "recreating an existing label PATCHes it" 0 "" \ check "...and does not POST a duplicate" 1 "" grep -q '^POST repos/o/r/labels ' "$WRITES" check "...carrying the updated description" 0 "" grep -q 'new text' "$WRITES" +# --- forge_issue_edit on forgejo: the two asymmetries, hermetically ------ +# Promised with the call-site port (@grok-reviewer-andresmgsl #4741 note 2, +# #4751 item 2). Live scratch-repo evidence proved these work; these prove +# they keep working, and pin the SHAPE of the requests. + +# Removal resolves name -> id, because Forgejo takes names on add and only a +# numeric id on remove. Measured: DELETE .../labels/probe:one -> 422, +# DELETE .../labels/149 -> 204. +FAKE_LABELS='[{"name":"stale","id":11},{"name":"ready","id":12}]' FAKE_LABEL_N=2 stub_writes +FAKE_LABELS='[{"name":"stale","id":11},{"name":"ready","id":12}]' FAKE_LABEL_N=2 REPO=o/r forge_issue_edit 5 --remove-label stale +check "removing a label resolves its numeric id" 0 "" grep -q '^DELETE repos/o/r/issues/5/labels/11 ' "$WRITES" +check "...and never sends the name as the path segment" 1 "" grep -q 'labels/stale' "$WRITES" + +# A label the repo does not have is a no-op, matching gh: the reconcilers +# call --remove-label unconditionally to converge state. +FAKE_LABELS='[{"name":"ready","id":12}]' FAKE_LABEL_N=1 stub_writes +FAKE_LABELS='[{"name":"ready","id":12}]' FAKE_LABEL_N=1 REPO=o/r forge_issue_edit 5 --remove-label nonexistent +check "removing an absent label writes nothing" 0 "" test ! -s "$WRITES" + +# Adding takes names directly — no lookup, one request. +FAKE_LABELS='[]' FAKE_LABEL_N=0 stub_writes +FAKE_LABELS='[]' FAKE_LABEL_N=0 REPO=o/r forge_issue_edit 5 --add-label "ready,stale" +check "adding labels posts them by name" 0 "" grep -q '^POST repos/o/r/issues/5/labels .*"ready"' "$WRITES" +check "...comma-separated values are split, as gh splits them" 0 "" grep -q '"stale"' "$WRITES" + +# Assignees are SET, not added/removed: PATCH takes the whole list. So a +# removal is a read-modify-write, and a naive translation would have cleared +# every OTHER assignee as a side effect of removing one. +assignee_stub() { + : >"$WRITES" + # shellcheck disable=SC2317 # invoked indirectly, by forge_issue_edit + curl() { + local hdr="" out="" method=GET url="" payload="" + while [ $# -gt 0 ]; do + case "$1" in + -D) hdr="$2"; shift ;; -o) out="$2"; shift ;; + -X) method="$2"; shift ;; -d) payload="$2"; shift ;; + -H) shift ;; -*) ;; *) url="$1" ;; + esac + shift + done + printf 'HTTP/1.1 200 OK\r\nX-Total-Count: 0\r\n\r\n' >"$hdr" + printf '{"assignees":[{"login":"alice"},{"login":"bob"}]}' >"$out" + [ "$method" = GET ] || printf '%s %s %s\n' "$method" "${url##*/api/v1/}" "$payload" >>"$WRITES" + return 0 + } +} +assignee_stub +REPO=o/r forge_issue_edit 5 --remove-assignee alice +check "removing one assignee PATCHes the surviving list" 0 "" grep -q '^PATCH repos/o/r/issues/5 .*"bob"' "$WRITES" +check "...and the removed one is gone from it" 1 "" grep -q '"alice"' "$WRITES" + +assignee_stub +REPO=o/r forge_issue_edit 5 --add-assignee carol +check "adding an assignee keeps the existing ones" 0 "" grep -qE '^PATCH repos/o/r/issues/5 .*"alice".*"bob".*"carol"|^PATCH repos/o/r/issues/5 .*"alice".*"carol".*"bob"' "$WRITES" + # --- forge_pr_view: newest verdict per context must win ------------------ # checks_state groups repeated contexts and selects the newest by # [.startedAt, .createdAt, .completedAt]. Mapping only {context,state} left