mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-09 08:04:45 +00:00
Merge pull request #3797 from davotoula/fix/sonar-case-default-clause
Harden workflow-ship.sh and pin it with a regression harness
This commit is contained in:
Vitor Pamplona
@@ -25,12 +25,19 @@ die() { printf 'error: %s\n' "$*" >&2; exit 1; }
|
||||
cd "$BUZZ_WORKTREE" || die "cannot cd into worktree $BUZZ_WORKTREE"
|
||||
|
||||
# The PR base = the repo's default branch. Never operate on it directly.
|
||||
base_branch="$(gh repo view --json defaultBranchRef -q .defaultBranchRef.name 2>/dev/null || echo main)"
|
||||
# `gh` can also succeed while printing nothing (or a literal "null") for a repo it can't
|
||||
# resolve, so fall back on the value, not just on the exit status.
|
||||
base_branch="$(gh repo view --json defaultBranchRef -q .defaultBranchRef.name 2>/dev/null || true)"
|
||||
[[ -n "$base_branch" && "$base_branch" != "null" ]] || base_branch="main"
|
||||
case "$BUZZ_BRANCH" in
|
||||
"$base_branch" | main | master) die "refusing to operate on the default branch ($BUZZ_BRANCH)" ;;
|
||||
# Anything else is a per-run branch — the only thing this script is allowed to push.
|
||||
*) : ;;
|
||||
esac
|
||||
|
||||
title="$(git log -1 --format='%s' 2>/dev/null | cut -c1-72)"
|
||||
# `|| true` keeps `set -e` from killing the script when the worktree has no commits yet —
|
||||
# without it the fallback below is unreachable and the run fails with an empty stderr.
|
||||
title="$(git log -1 --format='%s' 2>/dev/null | cut -c1-72)" || true
|
||||
[[ -n "$title" ]] || title="Buzz run ${BUZZ_RUN:-}"
|
||||
|
||||
log "[workflow-ship] pushing $BUZZ_BRANCH"
|
||||
|
||||
@@ -98,7 +98,23 @@ merges it on GitHub.
|
||||
|
||||
## Testing
|
||||
|
||||
`agent-exec.sh` is verified end-to-end against a throwaway repo with a stubbed `gh` + agent
|
||||
### `./test-workflow-ship.sh` — the ship step, end to end
|
||||
|
||||
```bash
|
||||
./tools/buzz-agent/test-workflow-ship.sh # the sibling workflow-ship.sh
|
||||
./tools/buzz-agent/test-workflow-ship.sh /path/to/other.sh # or any other copy
|
||||
```
|
||||
|
||||
Runs the real script against a throwaway repo with a local bare `origin` (so `git push` genuinely
|
||||
pushes) and a stub `gh` each case configures. No network, no GitHub, no credentials; exits non-zero
|
||||
on the first failing case. It covers the happy path, PR reuse, all four default-branch refusals, the
|
||||
empty-worktree failure mode, and `base_branch` resolution when `gh` answers oddly (`null`, empty, or
|
||||
non-zero) — the paths that otherwise only fail on someone else's machine, unattended, via a
|
||||
kind-46007 whose stderr is the only clue.
|
||||
|
||||
### `agent-exec.sh`
|
||||
|
||||
Verified end-to-end against a throwaway repo with a stubbed `gh` + agent
|
||||
(reads task → runs agent → commits the diff → pushes the feature branch → prints the PR URL;
|
||||
and errors cleanly when the agent makes no changes). Point `AGENT_CMD` at a stub to dry-run the
|
||||
git/PR plumbing without invoking a real agent.
|
||||
|
||||
Executable
+133
@@ -0,0 +1,133 @@
|
||||
#!/usr/bin/env bash
|
||||
#
|
||||
# test-workflow-ship.sh — regression harness for workflow-ship.sh.
|
||||
#
|
||||
# Runs the real ship script end to end against a throwaway git repo with a local
|
||||
# bare "origin" (so `git push` genuinely pushes) and a stub `gh` on PATH whose
|
||||
# behaviour each case controls. No network, no GitHub, no credentials.
|
||||
#
|
||||
# ./test-workflow-ship.sh # test the sibling workflow-ship.sh
|
||||
# ./test-workflow-ship.sh /path/to/other.sh # or any other copy
|
||||
#
|
||||
# Exits 0 when every case passes, 1 otherwise — safe to wire into CI on any host
|
||||
# that has bash + git.
|
||||
#
|
||||
# Why this exists: the ship step runs unattended under `amy buzz workflow run` and
|
||||
# only ever reports through stdout/stderr, so a `set -e` abort or a bad `--base`
|
||||
# surfaces as a mystery failure on someone else's machine. These cases pin the
|
||||
# behaviour that made those failures diagnosable.
|
||||
|
||||
set -uo pipefail
|
||||
|
||||
SCRIPT="${1:-$(dirname "$0")/workflow-ship.sh}"
|
||||
[[ -f "$SCRIPT" ]] || { echo "no such script: $SCRIPT" >&2; exit 2; }
|
||||
SCRIPT="$(cd "$(dirname "$SCRIPT")" && pwd)/$(basename "$SCRIPT")"
|
||||
pass=0
|
||||
fail=0
|
||||
|
||||
# --- stub gh ---------------------------------------------------------------
|
||||
STUB_DIR="$(mktemp -d)"
|
||||
trap 'rm -rf "$STUB_DIR"' EXIT
|
||||
cat > "$STUB_DIR/gh" <<'STUB'
|
||||
#!/usr/bin/env bash
|
||||
case "$1 $2" in
|
||||
"repo view")
|
||||
case "${GH_DEFAULT_BRANCH_MODE:-ok}" in
|
||||
ok) printf '%s\n' "${GH_DEFAULT_BRANCH:-main}" ;;
|
||||
null) printf 'null\n' ;; # what jq prints for a missing defaultBranchRef
|
||||
empty) printf '' ;; # exits 0, says nothing
|
||||
fail) exit 1 ;; # gh itself errors
|
||||
esac
|
||||
;;
|
||||
"pr list") printf '%s\n' "${GH_EXISTING_PR:-}" ;;
|
||||
"pr create")
|
||||
# The script runs `gh pr create` inside $( … 2>/dev/null ), so the stub
|
||||
# cannot report through stderr — write to the log the harness reads.
|
||||
base=""
|
||||
while [[ $# -gt 0 ]]; do [[ "$1" == "--base" ]] && base="${2-}"; shift; done
|
||||
printf 'pr-create base=<%s>\n' "$base" >> "$STUB_LOG"
|
||||
printf 'https://github.com/acme/repo/pull/7\n'
|
||||
;;
|
||||
*) exit 1 ;;
|
||||
esac
|
||||
STUB
|
||||
chmod +x "$STUB_DIR/gh"
|
||||
export PATH="$STUB_DIR:$PATH"
|
||||
export STUB_LOG="$STUB_DIR/calls.log"
|
||||
|
||||
# --- a fresh repo per case -------------------------------------------------
|
||||
new_repo() { # $1 = "commit" | "empty"
|
||||
local root
|
||||
root="$(mktemp -d)"
|
||||
git init -q --bare "$root/origin.git"
|
||||
git init -q -b main "$root/work"
|
||||
git -C "$root/work" config user.email harness@example.test
|
||||
git -C "$root/work" config user.name Harness
|
||||
git -C "$root/work" config commit.gpgsign false
|
||||
git -C "$root/work" remote add origin "$root/origin.git"
|
||||
if [[ "$1" == "commit" ]]; then
|
||||
echo hi > "$root/work/f.txt"
|
||||
git -C "$root/work" add f.txt
|
||||
git -C "$root/work" commit -qm "feat: teach the widget to fly"
|
||||
fi
|
||||
printf '%s\n' "$root/work"
|
||||
}
|
||||
|
||||
check() { # name, expected-exit, expected-substring, [VAR=value ...]
|
||||
local name="$1" want_exit="$2" want_text="$3"
|
||||
shift 3
|
||||
local wt out code
|
||||
wt="$(new_repo "${REPO_STATE:-commit}")"
|
||||
: > "$STUB_LOG"
|
||||
out="$(cd "$wt" && env "$@" BUZZ_WORKTREE="$wt" BUZZ_BRANCH="${BUZZ_BRANCH:-claude/job-1}" \
|
||||
BUZZ_RUN=run123 bash "$SCRIPT" 2>&1)"
|
||||
code=$?
|
||||
out="$out $(cat "$STUB_LOG")"
|
||||
if [[ "$code" == "$want_exit" && "$out" == *"$want_text"* ]]; then
|
||||
printf ' PASS %s\n' "$name"
|
||||
pass=$((pass + 1))
|
||||
else
|
||||
printf ' FAIL %s\n want exit=%s containing %q\n got exit=%s: %s\n' \
|
||||
"$name" "$want_exit" "$want_text" "$code" "${out//$'\n'/ | }"
|
||||
fail=$((fail + 1))
|
||||
fi
|
||||
rm -rf "$(dirname "$wt")"
|
||||
# bash keeps `VAR=x func` assignments set after the call — clear them so one
|
||||
# case cannot configure the next.
|
||||
unset REPO_STATE BUZZ_BRANCH GH_EXISTING_PR
|
||||
}
|
||||
|
||||
printf '\nworkflow-ship harness: %s\n\n' "$SCRIPT"
|
||||
|
||||
# --- the happy paths -------------------------------------------------------
|
||||
check "opens a PR and prints its url" 0 "Opened PR: https://github.com/acme/repo/pull/7"
|
||||
GH_EXISTING_PR="https://github.com/acme/repo/pull/3" \
|
||||
check "reuses an already-open PR instead of creating another" 0 \
|
||||
"Opened PR: https://github.com/acme/repo/pull/3" \
|
||||
GH_EXISTING_PR="https://github.com/acme/repo/pull/3"
|
||||
|
||||
# --- the default-branch guard (the script's one safety property) -----------
|
||||
BUZZ_BRANCH=main check "refuses to push main" 1 "refusing to operate on the default branch"
|
||||
BUZZ_BRANCH=master check "refuses to push master" 1 "refusing to operate on the default branch"
|
||||
BUZZ_BRANCH=develop check "refuses to push whatever gh reports as default" 1 \
|
||||
"refusing to operate on the default branch" GH_DEFAULT_BRANCH=develop
|
||||
BUZZ_BRANCH=main check "still refuses main when gh is unusable" 1 \
|
||||
"refusing to operate on the default branch" GH_DEFAULT_BRANCH_MODE=fail
|
||||
|
||||
# --- a worktree with no commits --------------------------------------------
|
||||
# Nothing to push, so this must fail — the point is that it fails *with a
|
||||
# message*. `git log` used to trip `set -e` at the `title=` line, killing the run
|
||||
# at exit 128 with completely empty output (git's own error swallowed by the
|
||||
# 2>/dev/null), which the runner then reported as a failure with empty stderr.
|
||||
REPO_STATE=empty check "empty worktree fails loudly, not silently" 1 "push failed"
|
||||
|
||||
# --- base_branch resolution ------------------------------------------------
|
||||
# Only the "gh exited non-zero" case used to be handled; a gh that succeeds while
|
||||
# printing `null` or nothing sent that value straight to `gh pr create --base`.
|
||||
check "gh prints literal null -> base falls back to main" 0 "base=<main>" GH_DEFAULT_BRANCH_MODE=null
|
||||
check "gh prints nothing -> base falls back to main" 0 "base=<main>" GH_DEFAULT_BRANCH_MODE=empty
|
||||
check "gh exits non-zero -> base falls back to main" 0 "base=<main>" GH_DEFAULT_BRANCH_MODE=fail
|
||||
check "gh answers normally -> base is what gh said" 0 "base=<develop>" GH_DEFAULT_BRANCH=develop
|
||||
|
||||
printf '\n %d passed, %d failed\n\n' "$pass" "$fail"
|
||||
[[ "$fail" -eq 0 ]]
|
||||
@@ -25,12 +25,19 @@ die() { printf 'error: %s\n' "$*" >&2; exit 1; }
|
||||
cd "$BUZZ_WORKTREE" || die "cannot cd into worktree $BUZZ_WORKTREE"
|
||||
|
||||
# The PR base = the repo's default branch. Never operate on it directly.
|
||||
base_branch="$(gh repo view --json defaultBranchRef -q .defaultBranchRef.name 2>/dev/null || echo main)"
|
||||
# `gh` can also succeed while printing nothing (or a literal "null") for a repo it can't
|
||||
# resolve, so fall back on the value, not just on the exit status.
|
||||
base_branch="$(gh repo view --json defaultBranchRef -q .defaultBranchRef.name 2>/dev/null || true)"
|
||||
[[ -n "$base_branch" && "$base_branch" != "null" ]] || base_branch="main"
|
||||
case "$BUZZ_BRANCH" in
|
||||
"$base_branch" | main | master) die "refusing to operate on the default branch ($BUZZ_BRANCH)" ;;
|
||||
# Anything else is a per-run branch — the only thing this script is allowed to push.
|
||||
*) : ;;
|
||||
esac
|
||||
|
||||
title="$(git log -1 --format='%s' 2>/dev/null | cut -c1-72)"
|
||||
# `|| true` keeps `set -e` from killing the script when the worktree has no commits yet —
|
||||
# without it the fallback below is unreachable and the run fails with an empty stderr.
|
||||
title="$(git log -1 --format='%s' 2>/dev/null | cut -c1-72)" || true
|
||||
[[ -n "$title" ]] || title="Buzz run ${BUZZ_RUN:-}"
|
||||
|
||||
log "[workflow-ship] pushing $BUZZ_BRANCH"
|
||||
|
||||
Reference in New Issue
Block a user