Add shared shell helpers, test framework, and shellcheck config

Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>

Author: fantapop

CLAUDE.md

@@ -35,5 +35,43 @@ scripts that set up temporary git repos and validate behavior.
   syntax highlighting and testability.
 - update CHANGELOG.md with each change.  If it's a breaking change, prefix
   with "Breaking Change: ". Try to keep change descriptions focused on user
-  outcome.
+  outcome. New entries go above older ones (the changelog grows upward).
 - CI runs on PRs: `test.yml` runs `./test.sh`, `actionlint.yml` lints workflow YAML files
+- Every shellcheck suppression (`disable`, `source`, etc.) must include a short
+  comment explaining why the suppression is needed. SC1091 (can't follow
+  sourced file) is disabled globally in `.shellcheckrc` since all sources use
+  dynamic `$SCRIPT_DIR` paths.
+- In test files, prefer `cd "$(dirname "${BASH_SOURCE[0]}")"` at the top and then use literal
+  relative paths for `source` (e.g., `source ../../actions_helpers.sh`). This
+  enables IDE go-to-definition via `source-path=SCRIPTDIR` in `.shellcheckrc`
+  without needing `# shellcheck source=` directives. In production scripts that
+  cannot `cd`, use `SCRIPT_DIR` with `# shellcheck source=` directives for
+  navigation.
+- `actions_helpers.sh` at the repo root provides shared helpers (`log_error`, `log_warning`,
+  `log_notice`, `set_output`, `set_output_multiline`). Scripts source it via
+  a relative path after `cd`-ing to their own directory.
+- In shell scripts, prefer long options over short flags for readability
+  (e.g., `grep --quiet --fixed-strings` instead of `grep -qF`,
+  `curl --silent --output /dev/null` instead of `curl -s -o /dev/null`).
+  Exceptions: flags with no long form (e.g., `git checkout -b`) and
+  universally understood short forms in test helpers (e.g., `rm -rf`).
+- Never discard stderr (e.g., `2>/dev/null`) in shell scripts or action
+  steps. Suppressing stderr hides real errors and makes debugging harder.
+  Using `2>&1` to merge stderr into stdout is acceptable in test helpers
+  that need to capture all output for assertion, but avoid it in
+  production scripts. Run each command on its own line so that `bash -e`
+  (the default for GitHub Actions `run` steps) halts on failure and the
+  return code is checked automatically.
+- In workflow YAML files, always use the latest major version of built-in
+  GitHub Actions (e.g., `actions/checkout@v5`, `actions/upload-artifact@v4`).
+- In autosolve workflows, install the Claude CLI (npm) BEFORE any cloud
+  authentication step, and move credential files out of the workspace
+  immediately after authentication. npm post-install scripts run with the
+  job's full environment, so installing after auth exposes credentials
+  (e.g., the OIDC bearer token in `gha-creds-*.json`) to arbitrary code.
+- Do not silently swallow errors. In shell scripts, avoid `|| return 0`,
+  `|| true`, or `|| :` to suppress failures without logging — use
+  `log_warning` to surface what went wrong. In Go code, avoid `return nil`
+  on error paths without logging or returning the error. If ignoring an
+  error is genuinely correct (e.g., best-effort cleanup), add a comment
+  explaining why it's safe to ignore.

actions_helpers.sh

@@ -1,7 +1,7 @@
 #!/usr/bin/env bash
 # Shared shell helpers for all GitHub Actions in this repo.
 
-# GitHub Actions log commands — structured annotations in CI, plain stderr locally.
+# GitHub Actions log commands — emit structured annotations via stdout.
 log_error()   { echo "::error::$*"; }
 log_warning() { echo "::warning::$*"; }
 log_notice()  { echo "::notice::$*"; }
@@ -21,3 +21,14 @@ set_output_multiline() {
     echo "$delim"
   } >> "${GITHUB_OUTPUT:-/dev/null}"
 }
+
+# Verify a command is on PATH: require_command <name>
+require_command() {
+  command -v "$1" >/dev/null || { log_error "$1 not found on PATH"; return 1; }
+}
+
+# Append content to the GitHub Actions step summary.
+# Usage: write_step_summary <<EOF ... EOF
+write_step_summary() {
+  cat >> "${GITHUB_STEP_SUMMARY:-/dev/null}"
+}

actions_helpers_test.sh

@@ -1,7 +1,6 @@
 #!/usr/bin/env bash
 # Tests for actions_helpers.sh helpers.
 set -euo pipefail
-trap 'echo "Error occurred at line $LINENO"; exit 1' ERR
 
 cd "$(dirname "${BASH_SOURCE[0]}")"
 source ./test_helpers.sh
@@ -39,15 +38,15 @@ test_set_output() {
   GITHUB_OUTPUT="$TMPDIR_TEST/gh_out_single"
   touch "$GITHUB_OUTPUT"
   set_output "mykey" "myvalue"
-  grep -q 'mykey=myvalue' "$GITHUB_OUTPUT"
+  check_contains 'mykey=myvalue' "$GITHUB_OUTPUT"
 }
 expect_success "set_output: writes key=value" test_set_output
 
 test_set_output_empty_value() {
   GITHUB_OUTPUT="$TMPDIR_TEST/gh_out_empty"
   touch "$GITHUB_OUTPUT"
   set_output "mykey" ""
-  grep -q 'mykey=$' "$GITHUB_OUTPUT"
+  check_contains 'mykey=' "$GITHUB_OUTPUT"
 }
 expect_success "set_output: handles empty value" test_set_output_empty_value
 
@@ -58,7 +57,7 @@ test_set_output_multiline() {
   touch "$GITHUB_OUTPUT"
   set_output_multiline "desc" "line one
 line two"
-  grep -q 'line one' "$GITHUB_OUTPUT" && grep -q 'line two' "$GITHUB_OUTPUT"
+  check_contains 'line one' "$GITHUB_OUTPUT" && check_contains 'line two' "$GITHUB_OUTPUT"
 }
 expect_success "set_output_multiline: writes multiline content" test_set_output_multiline
 
@@ -67,16 +66,24 @@ test_set_output_multiline_delimiters() {
   touch "$GITHUB_OUTPUT"
   set_output_multiline "desc" "content"
   # Should have opening delimiter (desc<<GHEOF_...) and closing delimiter (GHEOF_...)
-  grep -q '^desc<<GHEOF_' "$GITHUB_OUTPUT" && grep -q '^GHEOF_' "$GITHUB_OUTPUT"
+  check_contains_pattern '^desc<<GHEOF_' "$GITHUB_OUTPUT" && check_contains_pattern '^GHEOF_' "$GITHUB_OUTPUT"
 }
 expect_success "set_output_multiline: uses GHEOF delimiters" test_set_output_multiline_delimiters
 
 test_set_output_multiline_empty() {
   GITHUB_OUTPUT="$TMPDIR_TEST/gh_out_multi_empty"
   touch "$GITHUB_OUTPUT"
   set_output_multiline "desc" ""
-  grep -q '^desc<<GHEOF_' "$GITHUB_OUTPUT"
+  check_contains_pattern '^desc<<GHEOF_' "$GITHUB_OUTPUT"
 }
 expect_success "set_output_multiline: handles empty value" test_set_output_multiline_empty
 
+# --- require_command tests ---
+
+test_require_command_found() { require_command bash; }
+expect_success "require_command: finds bash" test_require_command_found
+
+test_require_command_missing() { require_command nonexistent_cmd_xyz; }
+expect_failure "require_command: fails for missing command" test_require_command_missing
+
 print_results

autotag-from-changelog/auto-tag-release.sh

@@ -11,11 +11,16 @@
 # not tagged) and the script fails.
 set -euo pipefail
 
+ORIG_DIR="$(pwd)"
+cd "$(dirname "${BASH_SOURCE[0]}")"
+source ../actions_helpers.sh
+cd "$ORIG_DIR"
+
 changelog="${CHANGELOG_PATH:-CHANGELOG.md}"
 
 # Check that the changelog file exists and is readable.
 if [ ! -r "$changelog" ]; then
-  echo "::error::Changelog file '$changelog' does not exist or is not readable."
+  log_error "Changelog file '$changelog' does not exist or is not readable."
   exit 1
 fi
 # Parse the changelog to extract the unreleased content and latest version.
@@ -68,7 +73,7 @@ if [ -n "$unreleased_content" ]; then
     echo "Content under [Unreleased] and ${tag} already tagged, nothing to do."
     exit 0
   else
-    echo "::error::CHANGELOG.md has content under [Unreleased] but ${tag} is not tagged. Tag the previous release before adding new entries."
+    log_error "CHANGELOG.md has content under [Unreleased] but ${tag} is not tagged. Tag the previous release before adding new entries."
     exit 1
   fi
 fi

autotag-from-changelog/auto-tag-release_test.sh

@@ -1,50 +1,22 @@
 #!/usr/bin/env bash
 # Tests for auto-tag-release.sh
 set -euo pipefail
-trap 'echo "Error occurred at line $LINENO"; exit 1' ERR
 
-SCRIPT_DIR="$(cd "$(dirname "$0")"; pwd)"
-PASS=0
-FAIL=0
-
-run_test() {
-  local name="$1"
-  local expected_exit="$2"
-  local expected_output="$3"
-  shift 3
-
-  local output exit_code
-  output=$("$@" 2>&1) && exit_code=0 || exit_code=$?
-
-  if [ "$exit_code" -ne "$expected_exit" ]; then
-    echo "FAIL: $name — expected exit $expected_exit, got $exit_code"
-    echo "  output: $output"
-    FAIL=$((FAIL + 1))
-    return
-  fi
-
-  if [ -n "$expected_output" ] && ! echo "$output" | grep -qF "$expected_output"; then
-    echo "FAIL: $name — expected output containing: $expected_output"
-    echo "  actual: $output"
-    FAIL=$((FAIL + 1))
-    return
-  fi
-
-  echo "PASS: $name"
-  PASS=$((PASS + 1))
-}
+cd "$(dirname "${BASH_SOURCE[0]}")"
+SCRIPT_DIR="$(pwd)"
+source ../test_helpers.sh
 
 # Set up a temporary bare repo to act as "origin" and a working clone.
 TMPDIR=$(mktemp -d)
 trap 'rm -rf "$TMPDIR"' EXIT
 
-git config user.email || git config --global user.email "test@test.com"
-git config user.name || git config --global user.name "test"
-git init --bare --initial-branch=main "$TMPDIR/origin.git"
-git clone "$TMPDIR/origin.git" "$TMPDIR/work"
+git config user.email >/dev/null || git config --global user.email "test@test.com"
+git config user.name >/dev/null || git config --global user.name "test"
+git init --bare --initial-branch=main "$TMPDIR/origin.git" >/dev/null
+git clone "$TMPDIR/origin.git" "$TMPDIR/work" >/dev/null
 cd "$TMPDIR/work"
-git commit --allow-empty -m "initial"
-git push origin main
+git commit --allow-empty -m "initial" >/dev/null
+git push origin main >/dev/null
 
 # =============================================
 # parse_changelog unit tests
@@ -167,7 +139,7 @@ cat <<'EOF' > CHANGELOG.md
 ## [1.0.0] - 2026-01-01
 EOF
 
-run_test "content under [Unreleased], untagged version" 1 "is not tagged" \
+expect_failure_output "content under [Unreleased], untagged version" "is not tagged" \
   env CHANGELOG_PATH=CHANGELOG.md "$SCRIPT_DIR/auto-tag-release.sh"
 
 # --- Empty [Unreleased], new version, no existing tag — should tag ---
@@ -177,11 +149,11 @@ cat <<'EOF' > CHANGELOG.md
 ## [1.0.0] - 2026-01-01
 EOF
 
-run_test "creates new tag" 0 "Tagged v1.0.0 successfully" \
+expect_success_output "creates new tag" "Tagged v1.0.0 successfully" \
   env CHANGELOG_PATH=CHANGELOG.md "$SCRIPT_DIR/auto-tag-release.sh"
 
 # Verify the tag was actually created
-if git rev-parse v1.0.0; then
+if git rev-parse v1.0.0 >/dev/null; then
   echo "PASS: tag v1.0.0 exists"
   PASS=$((PASS + 1))
 else
@@ -190,7 +162,7 @@ else
 fi
 
 # --- Tag already exists — should skip ---
-run_test "tag already exists" 0 "already exists" \
+expect_success_output "tag already exists" "already exists" \
   env CHANGELOG_PATH=CHANGELOG.md "$SCRIPT_DIR/auto-tag-release.sh"
 
 # --- Content under [Unreleased], previous version already tagged — should pass ---
@@ -204,23 +176,23 @@ cat <<'EOF' > CHANGELOG.md
 ## [1.0.0] - 2026-01-01
 EOF
 
-run_test "content under [Unreleased], tagged version" 0 "already tagged" \
+expect_success_output "content under [Unreleased], tagged version" "already tagged" \
   env CHANGELOG_PATH=CHANGELOG.md "$SCRIPT_DIR/auto-tag-release.sh"
 
 # --- No version sections at all — should skip ---
 cat <<'EOF' > CHANGELOG.md
 ## [Unreleased]
 EOF
 
-run_test "no released version" 0 "No released version" \
+expect_success_output "no released version" "No released version found" \
   env CHANGELOG_PATH=CHANGELOG.md "$SCRIPT_DIR/auto-tag-release.sh"
 
 # --- No [Unreleased] section — should skip ---
 cat <<'EOF' > CHANGELOG.md
 ## [2.0.0] - 2026-01-01
 EOF
 
-run_test "no [Unreleased] section" 0 "No released version" \
+expect_success "no [Unreleased] section" \
   env CHANGELOG_PATH=CHANGELOG.md "$SCRIPT_DIR/auto-tag-release.sh"
 
 # --- [Unreleased] with only blank lines (no content) — should tag ---
@@ -232,10 +204,7 @@ cat <<'EOF' > CHANGELOG.md
 ## [2.0.0] - 2026-01-01
 EOF
 
-run_test "blank lines under [Unreleased]" 0 "Tagged v2.0.0 successfully" \
+expect_success "blank lines under [Unreleased]" \
   env CHANGELOG_PATH=CHANGELOG.md "$SCRIPT_DIR/auto-tag-release.sh"
 
-# --- Summary ---
-echo ""
-echo "Results: $PASS passed, $FAIL failed"
-[ "$FAIL" -eq 0 ] || exit 1
+print_results

test.sh

@@ -2,13 +2,13 @@
 # Run all tests in the repository.
 set -euo pipefail
 
-SCRIPT_DIR="$(cd "$(dirname "$0")"; pwd)"
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")"; pwd)"
 FAILED=0
 
 test_files=()
 while IFS= read -r f; do
   test_files+=("$f")
-done < <(find "$SCRIPT_DIR" -mindepth 2 -name '*_test.sh' | sort)
+done < <(find "$SCRIPT_DIR" -mindepth 1 -name '*_test.sh' | sort)
 
 if [ "${#test_files[@]}" -eq 0 ]; then
   echo "No tests found."

test_helpers.sh

@@ -4,13 +4,39 @@
 # Usage:
 #   source test_helpers.sh
 #   expect_success "test name" command [args...]
-#   expect_failure "test name" "expected output substring" command [args...]
+#   expect_success_output "test name" "expected output" command [args...]
 #   expect_failure "test name" command [args...]
+#   expect_failure_output "test name" "expected output" command [args...]
 #   print_results
 
 PASS=0
 FAIL=0
 
+# Enable ERR trap for all test files that source this helper.
+trap 'echo "Error occurred at line $LINENO"; exit 1' ERR
+
+# Assert that input contains a given substring.
+# Usage: check_contains "expected" "$file"   (reads file)
+#        cmd | check_contains "expected"     (reads stdin)
+check_contains() {
+  if [ $# -ge 2 ]; then
+    grep --quiet --fixed-strings "$1" "$2"
+  else
+    grep --quiet --fixed-strings "$1"
+  fi
+}
+
+# Assert that input matches a given regex pattern.
+# Usage: check_contains_pattern "^prefix" "$file"   (reads file)
+#        cmd | check_contains_pattern "^prefix"      (reads stdin)
+check_contains_pattern() {
+  if [ $# -ge 2 ]; then
+    grep --quiet "$1" "$2"
+  else
+    grep --quiet "$1"
+  fi
+}
+
 _run_test() {
   local name="$1"
   local expected_exit="$2"  # exact code, or "nonzero" for any non-zero
@@ -34,7 +60,7 @@ _run_test() {
     return
   fi
 
-  if [ -n "$expected_output" ] && ! echo "$output" | grep -qF "$expected_output"; then
+  if [ -n "$expected_output" ] && ! echo "$output" | grep --quiet --fixed-strings "$expected_output"; then
     echo "FAIL: $name — expected output containing: $expected_output"
     echo "  actual: $output"
     FAIL=$((FAIL + 1))
@@ -45,22 +71,34 @@ _run_test() {
   PASS=$((PASS + 1))
 }
 
+# expect_success "test name" command [args...]
 # expect_success "test name" command [args...]
 # Asserts the command exits 0.
 expect_success() {
   local name="$1"; shift
   _run_test "$name" 0 "" "$@"
 }
 
-# expect_failure "test name" ["expected output substring"] command [args...]
-# Asserts the command exits non-zero. If the second arg is not a valid command,
-# it is treated as an expected output substring.
+# expect_success_output "test name" "expected output substring" command [args...]
+# Asserts the command exits 0 and output contains the expected substring.
+expect_success_output() {
+  local name="$1"; shift
+  local expected_output="$1"; shift
+  _run_test "$name" 0 "$expected_output" "$@"
+}
+
+# expect_failure "test name" command [args...]
+# Asserts the command exits non-zero.
 expect_failure() {
   local name="$1"; shift
-  local expected_output=""
-  if ! command -v "$1" >/dev/null 2>&1 && ! declare -F "$1" >/dev/null 2>&1; then
-    expected_output="$1"; shift
-  fi
+  _run_test "$name" "nonzero" "" "$@"
+}
+
+# expect_failure_output "test name" "expected output substring" command [args...]
+# Asserts the command exits non-zero and output contains the expected substring.
+expect_failure_output() {
+  local name="$1"; shift
+  local expected_output="$1"; shift
   _run_test "$name" "nonzero" "$expected_output" "$@"
 }