From 763f6d4a176c460a9c4aae22d7926603ca592752 Mon Sep 17 00:00:00 2001 From: Kevin O'Donnell Date: Wed, 13 May 2026 02:21:35 -0400 Subject: [PATCH 1/5] fix: anchor @include parsing in pack-script (SUR-2842) Rewrite `::get_includes` to match a leading `@include[[:space:]]+` directive, strip any trailing `# comment`, and emit field 2 only. The previous `grep "^@include" | awk '{print $NF}'` surfaced trailing comment tokens, multi-arg trailing tokens, and bare `@include` lines as fake include names, leading `includer::find` to fail or produce noisy errors at release time. Also harden the companion Makefile filter: `grep -v ".sh$"` (regex; `.` matches any char) becomes `find ! -name '*.sh'` so the extension exclusion is a literal match. Adds `tests/pack-script.bats` with three regression cases (trailing comment, multi-arg, leading-only) and three fixtures under `tests/pack-script/`. --- Makefile | 2 +- bash/pack-script | 15 ++++- tests/pack-script.bats | 56 +++++++++++++++++++ tests/pack-script/fixture-include-comment.sh | 7 +++ .../fixture-include-leading-only.sh | 10 ++++ tests/pack-script/fixture-include-multiarg.sh | 9 +++ 6 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 tests/pack-script.bats create mode 100644 tests/pack-script/fixture-include-comment.sh create mode 100644 tests/pack-script/fixture-include-leading-only.sh create mode 100644 tests/pack-script/fixture-include-multiarg.sh diff --git a/Makefile b/Makefile index 7c9b3d7..967e84e 100644 --- a/Makefile +++ b/Makefile @@ -37,7 +37,7 @@ dist/doc-$(VERSION).tar.gz: $(DOC_SRC) bash/bashadoc dist/bin-$(VERSION).tar.gz: $(DOC_SRC) $(BIN_SRC) bash/pack-script mkdir -p dist/bin - for s in $$(find bash -type f -exec grep -q "includer" {} \; -print|grep -v ".sh$$"); do \ + for s in $$(find bash -type f ! -name '*.sh' -exec grep -q "includer" {} \; -print); do \ base=$$(basename $$s) ; \ bash/pack-script -v -f $$s -o dist/bin/$$base ; \ done diff --git a/bash/pack-script b/bash/pack-script index 2e81ea9..27bdb8e 100755 --- a/bash/pack-script +++ b/bash/pack-script @@ -29,7 +29,20 @@ shift "$((OPTIND - 1))" function ::get_includes() { local file=${1:?} - grep "^@include" "$file" | awk '{print $NF}' + # SUR-2842: anchor on a literal `@include` directive followed by at + # least one whitespace character, strip any trailing `# comment`, and + # emit the include name (field 2). The previous implementation + # (`grep "^@include" | awk '{print $NF}'`) misparsed three concrete + # shapes: trailing-comment lines surfaced the ticket-name token, + # multi-arg lines surfaced the trailing token, and a bare `@include` + # with no argument fed an empty string downstream into + # `includer::find`. + awk ' + /^@include[[:space:]]+/ { + sub(/[[:space:]]*#.*/, "") + if (NF >= 2) { print $2 } + } + ' "$file" } function get_all_includes() { diff --git a/tests/pack-script.bats b/tests/pack-script.bats new file mode 100644 index 0000000..b79c333 --- /dev/null +++ b/tests/pack-script.bats @@ -0,0 +1,56 @@ +#!/usr/bin/env bats +bats_require_minimum_version 1.5.0 +# SUR-2842: pack-script's `::get_includes` must anchor on a leading +# `@include[[:space:]]+` directive and strip trailing comments before +# extracting the include name. The previous +# grep "^@include" | awk '{print $NF}' +# misparsed three concrete cases: +# 1. trailing comment lines (`@include foo # SUR-...`) → comment text +# 2. multi-arg lines (`@include foo bar baz`) → trailing arg +# 3. lines mentioning `@include` mid-text (or `@includes`, or bare +# `@include` with no argument) leaked into the include list. + +setup() { + load 'helpers.bash' + helpers::isolate_home + PACK="$REPO_ROOT/bash/pack-script" + FIXTURES="$REPO_ROOT/tests/pack-script" +} + +@test "pack-script extracts include name despite trailing comment (SUR-2842)" { + out=$(mktemp -d) + run "$PACK" -f "$FIXTURES/fixture-include-comment.sh" -o "$out/packed" + [ "$status" -eq 0 ] + # The packed output must contain log.sh content (log::error is defined + # at the top of bash/log.sh) and MUST NOT mention the trailing-comment + # sentinel that would surface if `awk '{print $NF}'` were still in use. + grep -q "^function log::" "$out/packed" + run ! grep -q "ZZZTRAILINGSENTINELZZZ" "$out/packed" +} + +@test "pack-script ignores extra tokens after the include name (SUR-2842)" { + out=$(mktemp -d) + run "$PACK" -f "$FIXTURES/fixture-include-multiarg.sh" -o "$out/packed" + [ "$status" -eq 0 ] + grep -q "^function log::" "$out/packed" + # Trailing tokens are not includable names; if `::get_includes` ever + # surfaces them, includer::find will fail or their sentinel strings + # will appear in the packed output (impossible here because the + # @include directive line is stripped at pack time). + run ! grep -q "MULTIARGEXTRAONESENTINEL" "$out/packed" + run ! grep -q "MULTIARGEXTRATWOSENTINEL" "$out/packed" +} + +@test "pack-script only matches leading @include directives (SUR-2842)" { + out=$(mktemp -d) + run "$PACK" -f "$FIXTURES/fixture-include-leading-only.sh" -o "$out/packed" + [ "$status" -eq 0 ] + # Exactly one real `@include log` lives in the fixture; the packed + # output should contain log.sh contents exactly once even though the + # file mentions @include in several non-directive contexts. + grep -q "log::" "$out/packed" + # The mid-line mention and the `@includes log` line must not have + # caused a second log.sh inlining or any new include resolution. + occurrences=$(grep -c "^log::level()" "$out/packed" || true) + [ "$occurrences" -le 1 ] +} diff --git a/tests/pack-script/fixture-include-comment.sh b/tests/pack-script/fixture-include-comment.sh new file mode 100644 index 0000000..16ccb05 --- /dev/null +++ b/tests/pack-script/fixture-include-comment.sh @@ -0,0 +1,7 @@ +# shellcheck shell=bash disable=SC2034 +# Fixture for the @include trailing-comment regression: the previous +# awk extraction returned the trailing token (a ticket name) instead of +# the include name. Use a unique sentinel after the `#` so the test can +# assert it never propagates into the packed output. +@include log # ZZZTRAILINGSENTINELZZZ +echo "ok" diff --git a/tests/pack-script/fixture-include-leading-only.sh b/tests/pack-script/fixture-include-leading-only.sh new file mode 100644 index 0000000..50af2e0 --- /dev/null +++ b/tests/pack-script/fixture-include-leading-only.sh @@ -0,0 +1,10 @@ +# shellcheck shell=bash disable=SC2034 +# SUR-2842 fixture: only the leading `@include[[:space:]]` form should +# match. Lines that mention `@include` mid-line, or `@includes` (note +# the trailing `s`), or `@include` without any following whitespace +# argument, must not be picked up. +echo "this line mentions @include log but is not a directive" +# @include log +@includes log +@include +@include log diff --git a/tests/pack-script/fixture-include-multiarg.sh b/tests/pack-script/fixture-include-multiarg.sh new file mode 100644 index 0000000..e80c2f9 --- /dev/null +++ b/tests/pack-script/fixture-include-multiarg.sh @@ -0,0 +1,9 @@ +# shellcheck shell=bash disable=SC2034 +# Fixture for the multi-arg @include regression: the previous awk +# extraction returned the last token. Use unique sentinels so the test +# can assert neither propagates into the packed output as an include +# name. The non-@include comments in this file are deliberately free +# of those sentinel strings so a successful run can be detected by +# their absence in the packed binary. +@include log MULTIARGEXTRAONESENTINEL MULTIARGEXTRATWOSENTINEL +echo "ok" From cb4cfe009ea7d6c949635da7c3623aac10c0fad4 Mon Sep 17 00:00:00 2001 From: Kevin O'Donnell Date: Wed, 13 May 2026 02:26:59 -0400 Subject: [PATCH 2/5] test: add bats coverage for bashadoc (SUR-2835) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends `tests/bashadoc.bats` from the three SUR-2829 regression tests to the five contract points enumerated in SUR-2835: 1. `@package` header rendering. 2. No-`@package` fallback to filename + bare-name function list. 3. `@doc` then `@arg` ordering, with the declare-f trailing semicolons scrubbed by the `tr ';' ' '` step. 4. `function @package() {` non-pollution regression (the SUR-2829 anchor fix; kept here so future refactors of the awk extraction cannot silently regress it). 5. Non-`.sh` argument fails with exit 1 and the documented stderr. Fixtures live under `tests/fixtures/bashadoc/` so each case can be inspected directly rather than reading heredoc'd content inside the spec. No production change to `bash/bashadoc` was required — the SUR-2829 anchor fix already in place handles case 4 cleanly. --- tests/bashadoc.bats | 84 +++++++++++++------- tests/fixtures/bashadoc/annotations.sh | 18 +++++ tests/fixtures/bashadoc/at-package-noise.sh | 13 +++ tests/fixtures/bashadoc/no-package.sh | 8 ++ tests/fixtures/bashadoc/not-a-shell-file.txt | 1 + tests/fixtures/bashadoc/with-package.sh | 14 ++++ 6 files changed, 108 insertions(+), 30 deletions(-) create mode 100644 tests/fixtures/bashadoc/annotations.sh create mode 100644 tests/fixtures/bashadoc/at-package-noise.sh create mode 100644 tests/fixtures/bashadoc/no-package.sh create mode 100644 tests/fixtures/bashadoc/not-a-shell-file.txt create mode 100644 tests/fixtures/bashadoc/with-package.sh diff --git a/tests/bashadoc.bats b/tests/bashadoc.bats index b54671e..b3bc28f 100644 --- a/tests/bashadoc.bats +++ b/tests/bashadoc.bats @@ -1,50 +1,74 @@ #!/usr/bin/env bats -# SUR-2829: bashadoc must anchor @package extraction. The previous -# `grep "@package"` matched `function @package() {` in doc.sh, yielding -# `{` as the "package" and shipping garbage markdown into doc-*.tar.gz. +bats_require_minimum_version 1.5.0 +# SUR-2835: bashadoc has direct coverage for the five contract points +# called out in the issue: +# 1. `@package` header rendering. +# 2. No-`@package` fallback to filename + bare-name functions. +# 3. `@doc` + `@arg` ordering with semicolon stripping in @doc. +# 4. `function @package() {` non-pollution regression (SUR-2829). +# 5. Non-`.sh` argument fails with exit 1 and the documented stderr. +# +# Fixtures live under tests/fixtures/bashadoc/ rather than heredoc'd +# inline so each case can be inspected directly. setup() { load 'helpers.bash' helpers::isolate_home - FIXTURE_DIR=$(mktemp -d) BASHADOC="$REPO_ROOT/bash/bashadoc" + FIXTURES="$REPO_ROOT/tests/fixtures/bashadoc" } -teardown() { - rm -rf "$FIXTURE_DIR" -} - -@test "bashadoc renders a header from @package and lists pkg::* functions (SUR-2829)" { - cat >"$FIXTURE_DIR/mypkg.sh" <<'EOF' -source "$(dirname "${BASH_SOURCE[0]}")/../bash/includer.sh" -@include doc - -@package mypkg - -function mypkg::fn() { - @doc Does the thing. - return 0 -} -EOF - out=$(bash "$BASHADOC" "$FIXTURE_DIR/mypkg.sh") +@test "bashadoc renders @package header and namespaced functions (SUR-2835 case 1)" { + out=$(bash "$BASHADOC" "$FIXTURES/with-package.sh") [[ "$out" == *"# \`mypkg\` package"* ]] [[ "$out" == *"## \`mypkg::fn\`"* ]] [[ "$out" != *"\`{\`"* ]] } -@test "bashadoc falls back to filename for libraries without @package (SUR-2829)" { - cat >"$FIXTURE_DIR/nopkg.sh" <<'EOF' -function bare_fn() { - return 0 -} -EOF - out=$(bash "$BASHADOC" "$FIXTURE_DIR/nopkg.sh") - [[ "$out" == *"# $FIXTURE_DIR/nopkg.sh package"* ]] +@test "bashadoc falls back to filename when @package is missing (SUR-2835 case 2)" { + out=$(bash "$BASHADOC" "$FIXTURES/no-package.sh") + [[ "$out" == *"# $FIXTURES/no-package.sh package"* ]] [[ "$out" == *"## \`bare_fn\`"* ]] + [[ "$out" != *"::"* ]] +} + +@test "bashadoc renders @doc then @arg with semicolons stripped (SUR-2835 case 3)" { + out=$(bash "$BASHADOC" "$FIXTURES/annotations.sh") + [[ "$out" == *"## \`annpkg::fn\`"* ]] + # The doc body renders verbatim, with no trailing semicolon (the + # production script runs `tr ';' ' '` over the captured @doc/@arg + # text to strip the semicolons that `declare -f` inserts at the end + # of every statement in the function body). + [[ "$out" == *"One-line description of annpkg::fn."* ]] + # @arg lines appear under an `### Arguments` heading, as bullets, and + # carry no trailing semicolon from declare -f. + [[ "$out" == *"### Arguments"* ]] + [[ "$out" == *"- _1_ first positional arg"* ]] + [[ "$out" == *"- -o \"\" the -o flag"* ]] + [[ "$out" != *"first positional arg;"* ]] + [[ "$out" != *"the -o flag;"* ]] + # Doc body comes before the Arguments section. + doc_line=$(printf '%s\n' "$out" | grep -n "One-line description" | head -1 | cut -d: -f1) + args_line=$(printf '%s\n' "$out" | grep -n "### Arguments" | head -1 | cut -d: -f1) + [ "$doc_line" -lt "$args_line" ] +} + +@test "bashadoc ignores 'function @package() {' as a directive (SUR-2835 case 4)" { + out=$(bash "$BASHADOC" "$FIXTURES/at-package-noise.sh") + # The previous unanchored grep produced a title of "`{` package"; + # the anchored awk extracts no directive, so the fallback file-path + # title is used instead. [[ "$out" != *"\`{\`"* ]] + [[ "$out" == *"# $FIXTURES/at-package-noise.sh package"* ]] +} + +@test "bashadoc rejects non-.sh argument with exit 1 (SUR-2835 case 5)" { + run bash "$BASHADOC" "$FIXTURES/not-a-shell-file.txt" + [ "$status" -eq 1 ] + [[ "$output" == *"expected a .sh file"* ]] } -@test "bashadoc handles doc.sh (which defines function @package) without emitting '{' (SUR-2829)" { +@test "bashadoc handles doc.sh (defines function @package) without emitting '{' (SUR-2829)" { out=$(bash "$BASHADOC" "$REPO_ROOT/bash/doc.sh") [[ "$out" != *"\`{\`"* ]] [[ "$out" == *"package"* ]] diff --git a/tests/fixtures/bashadoc/annotations.sh b/tests/fixtures/bashadoc/annotations.sh new file mode 100644 index 0000000..e251d4c --- /dev/null +++ b/tests/fixtures/bashadoc/annotations.sh @@ -0,0 +1,18 @@ +# shellcheck shell=bash disable=SC1091,SC2218 +# SUR-2835 fixture: a function carrying `@doc` and `@arg` annotations. +# bashadoc must emit the doc text first, then an `### Arguments` block +# listing the @arg lines as a bullet list. The production script also +# translates `;` to ` ` to scrub the trailing semicolons that +# `declare -f` introduces at the end of every statement in the body. + +source "$(dirname "${BASH_SOURCE[0]}")/../../../bash/includer.sh" +@include doc + +@package annpkg + +function annpkg::fn() { + @doc One-line description of annpkg::fn. + @arg _1_ first positional arg + @arg -o "" the -o flag + return 0 +} diff --git a/tests/fixtures/bashadoc/at-package-noise.sh b/tests/fixtures/bashadoc/at-package-noise.sh new file mode 100644 index 0000000..b025b1f --- /dev/null +++ b/tests/fixtures/bashadoc/at-package-noise.sh @@ -0,0 +1,13 @@ +# shellcheck shell=bash disable=SC1091,SC2218 +# SUR-2835 / SUR-2829 fixture: a library that defines a function named +# `@package` (mirroring the shape of `doc.sh`). The bashadoc +# `@package`-name extraction must not pick up `function @package() {` +# as a directive — the regression that produced `{` as the resolved +# "package name" and shipped garbage markdown. + +source "$(dirname "${BASH_SOURCE[0]}")/../../../bash/includer.sh" +@include doc + +function @package() { + : +} diff --git a/tests/fixtures/bashadoc/no-package.sh b/tests/fixtures/bashadoc/no-package.sh new file mode 100644 index 0000000..759f4f7 --- /dev/null +++ b/tests/fixtures/bashadoc/no-package.sh @@ -0,0 +1,8 @@ +# shellcheck shell=bash +# SUR-2835 fixture: a library with no `@package` directive. bashadoc +# must fall back to using the file path as the title and list only +# bare-name functions (no `pkg::` prefix). + +function bare_fn() { + return 0 +} diff --git a/tests/fixtures/bashadoc/not-a-shell-file.txt b/tests/fixtures/bashadoc/not-a-shell-file.txt new file mode 100644 index 0000000..93d7963 --- /dev/null +++ b/tests/fixtures/bashadoc/not-a-shell-file.txt @@ -0,0 +1 @@ +Not a .sh file. bashadoc must refuse this with exit 1. diff --git a/tests/fixtures/bashadoc/with-package.sh b/tests/fixtures/bashadoc/with-package.sh new file mode 100644 index 0000000..8dae933 --- /dev/null +++ b/tests/fixtures/bashadoc/with-package.sh @@ -0,0 +1,14 @@ +# shellcheck shell=bash disable=SC1091,SC2218 +# SUR-2835 fixture: a library that declares `@package` and one +# namespaced function. The bashadoc header must read `# `mypkg` package` +# and the function section must list `mypkg::fn` (and only that). + +source "$(dirname "${BASH_SOURCE[0]}")/../../../bash/includer.sh" +@include doc + +@package mypkg + +function mypkg::fn() { + @doc Does the thing. + return 0 +} From b7f0c5f737a45bd7932e4dc1506959cc08651f0f Mon Sep 17 00:00:00 2001 From: Kevin O'Donnell Date: Wed, 13 May 2026 02:51:21 -0400 Subject: [PATCH 3/5] test: add helper-level bats coverage for daml-export (SUR-2838) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `DAML_EXPORT_SOURCE_ONLY` source-only guard near the bottom of `bash/daml-export`, mirroring the `K8S_SUPPORT_COLLECTOR_SOURCE_ONLY` seam already in use for the same pattern. Guard defaults to executing, so unwrapped invocations (including `tests/sur-1871-daml-export.sh`) are unchanged. Adds `tests/daml-export.bats` exercising the five contract points enumerated in SUR-2838 in isolation: 1. `hex_to_dec` / `dec_to_hex` round-trip + 16-char hex padding. 2. `verifyExport` three-state return code (2 for empty/missing, 1 for exported-not-built, 0 when dar present). 3. `correct_archives` sed rewrite (single match, multi-match, non-match passthrough) and idempotency on a second invocation. 4. `correct_export` dual-file rewrite (`daml.yaml --target=1.14 → 1.12`, `Export.daml` import drop, archive correction chained). 5. Smoke check that the SOURCE_ONLY guard short-circuits the main loop when the env var is set. Test bodies run inside `bash -c` to insulate the bats `set -e` from the trailing `&&` short-circuit in `options::add` triggered when the script's option table is built on source. --- bash/daml-export | 13 ++++ tests/daml-export.bats | 166 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 179 insertions(+) create mode 100644 tests/daml-export.bats diff --git a/bash/daml-export b/bash/daml-export index b89bbfd..9766c86 100755 --- a/bash/daml-export +++ b/bash/daml-export @@ -164,6 +164,19 @@ function build { popd >/dev/null || return 1 } +# SUR-2838 test seam: when sourced from bats, skip the main entry +# point so individual helpers can be exercised in isolation. Mirrors +# the K8S_SUPPORT_COLLECTOR_SOURCE_ONLY guard. Defaults to executing +# the main loop when the env var is unset, so unwrapped invocations +# (including tests/sur-1871-daml-export.sh) are unchanged. +if [ "${DAML_EXPORT_SOURCE_ONLY:-}" = "true" ]; then + if [[ "${BASH_SOURCE[0]}" != "${0}" ]]; then + return 0 + else + exit 0 + fi +fi + CUR_INT=$(hex_to_dec "$START_OFFSET") STOP_INT=$(hex_to_dec "$STOP_OFFSET") diff --git a/tests/daml-export.bats b/tests/daml-export.bats new file mode 100644 index 0000000..03575c8 --- /dev/null +++ b/tests/daml-export.bats @@ -0,0 +1,166 @@ +#!/usr/bin/env bats +bats_require_minimum_version 1.5.0 +# SUR-2838: direct helper-level coverage for bash/daml-export. +# Complements (does not replace) the end-to-end SUR-1871 regression +# under tests/sur-1871-daml-export.sh; this spec exercises the +# documented contract points in isolation: +# +# 1. hex_to_dec / dec_to_hex round-trip and padding. +# 2. verifyExport three-state return code. +# 3. correct_archives sed rewrite + idempotency. +# 4. correct_export dual-file rewrite (daml.yaml + Export.daml). +# +# Uses the DAML_EXPORT_SOURCE_ONLY guard added in this sprint to source +# the script without firing the main offset-walking loop. Test bodies +# run inside `bash -c` to keep bats' `set -e` from tripping on the +# trailing `&&` in `options::add` when sourced helpers are defined. + +setup() { + load 'helpers.bash' + helpers::isolate_home + DAML_EXPORT="$REPO_ROOT/bash/daml-export" + export LOGFILE_DISABLE=true LOG_DISABLE_DEBUG=true LOG_DISABLE_INFO=true + export DAML_EXPORT_SOURCE_ONLY=true +} + +@test "DAML_EXPORT_SOURCE_ONLY exits 0 when executed (no main body) (SUR-2838)" { + out=$(mktemp -d) + run env DAML_EXPORT_SOURCE_ONLY=true bash "$DAML_EXPORT" -d "$out" -e ffff + rm -rf "$out" + [ "$status" -eq 0 ] + # The main loop's "Exporting from offset" log lines must not appear. + [[ "$output" != *"Exporting from offset"* ]] +} + +@test "hex_to_dec converts hex strings to decimal (SUR-2838 case 1)" { + tmp=$(mktemp -d) + run bash -c " + source '$DAML_EXPORT' -d '$tmp' -e ffff + printf '%s|%s|%s\n' \"\$(hex_to_dec 0a)\" \"\$(hex_to_dec 0)\" \"\$(hex_to_dec ff)\" + " + rm -rf "$tmp" + [ "$status" -eq 0 ] + [[ "$output" == *"10|0|255"* ]] +} + +@test "dec_to_hex pads to 16 hex chars (SUR-2838 case 2)" { + tmp=$(mktemp -d) + run bash -c " + source '$DAML_EXPORT' -d '$tmp' -e ffff + printf '%s|%s|%s\n' \"\$(dec_to_hex 10)\" \"\$(dec_to_hex 0)\" \"\$(dec_to_hex 65535)\" + " + rm -rf "$tmp" + [ "$status" -eq 0 ] + [[ "$output" == *"000000000000000a|0000000000000000|000000000000ffff"* ]] +} + +@test "verifyExport returns 2 for an empty directory (SUR-2838 case 3)" { + tmp=$(mktemp -d) + run bash -c " + source '$DAML_EXPORT' -d '$tmp' -e ffff + verifyExport '$tmp' + echo rc=\$? + " + rm -rf "$tmp" + [ "$status" -eq 0 ] + [[ "$output" == *"rc=2"* ]] +} + +@test "verifyExport returns 2 when export.good missing (SUR-2838 case 3)" { + tmp=$(mktemp -d) + touch "$tmp/Export.daml" + run bash -c " + source '$DAML_EXPORT' -d '$tmp' -e ffff + verifyExport '$tmp' + echo rc=\$? + " + rm -rf "$tmp" + [ "$status" -eq 0 ] + [[ "$output" == *"rc=2"* ]] +} + +@test "verifyExport returns 1 when export.good + Export.daml present but no dar (SUR-2838 case 3)" { + tmp=$(mktemp -d) + touch "$tmp/export.good" "$tmp/Export.daml" + run bash -c " + source '$DAML_EXPORT' -d '$tmp' -e ffff + verifyExport '$tmp' + echo rc=\$? + " + rm -rf "$tmp" + [ "$status" -eq 0 ] + [[ "$output" == *"rc=1"* ]] +} + +@test "verifyExport returns 0 when export.good + Export.daml + dar all present (SUR-2838 case 3)" { + tmp=$(mktemp -d) + touch "$tmp/export.good" "$tmp/Export.daml" + mkdir -p "$tmp/.daml/dist" + touch "$tmp/.daml/dist/export-1.0.0.dar" + run bash -c " + source '$DAML_EXPORT' -d '$tmp' -e ffff + verifyExport '$tmp' + echo rc=\$? + " + rm -rf "$tmp" + [ "$status" -eq 0 ] + [[ "$output" == *"rc=0"* ]] +} + +@test "correct_archives rewrites exerciseCmd … DA.Internal.Template.Archive to archiveCmd (SUR-2838 case 4)" { + tmp=$(mktemp -d) + cat >"$tmp/Export.daml" <<'EOF' +exerciseCmd foo DA.Internal.Template.Archive +exerciseCmd bar DA.Internal.Template.Archive +something else entirely +EOF + run bash -c " + source '$DAML_EXPORT' -d '$tmp' -e ffff + correct_archives '$tmp' + " + [ "$status" -eq 0 ] + grep -q "^archiveCmd foo$" "$tmp/Export.daml" + grep -q "^archiveCmd bar$" "$tmp/Export.daml" + grep -q "^something else entirely$" "$tmp/Export.daml" + run ! grep -q "DA.Internal.Template.Archive" "$tmp/Export.daml" + rm -rf "$tmp" +} + +@test "correct_archives is idempotent on a second invocation (SUR-2838 case 4)" { + tmp=$(mktemp -d) + cat >"$tmp/Export.daml" <<'EOF' +exerciseCmd foo DA.Internal.Template.Archive +EOF + run bash -c " + source '$DAML_EXPORT' -d '$tmp' -e ffff + correct_archives '$tmp' + sum1=\$(cksum '$tmp/Export.daml') + correct_archives '$tmp' + sum2=\$(cksum '$tmp/Export.daml') + [ \"\$sum1\" = \"\$sum2\" ] && echo SAME || echo DIFFERENT + " + rm -rf "$tmp" + [ "$status" -eq 0 ] + [[ "$output" == *"SAME"* ]] +} + +@test "correct_export rewrites both daml.yaml and Export.daml (SUR-2838 case 5)" { + tmp=$(mktemp -d) + cat >"$tmp/daml.yaml" <<'EOF' +sdk-version: 1.13.1 +build-options: ["--target=1.14"] +EOF + cat >"$tmp/Export.daml" <<'EOF' +import qualified DA.Internal.Template +exerciseCmd foo DA.Internal.Template.Archive +EOF + run bash -c " + source '$DAML_EXPORT' -d '$tmp' -e ffff >/dev/null + correct_export '$tmp' >/dev/null + " + [ "$status" -eq 0 ] + grep -q "build-options: \[\"--target=1.12\"\]" "$tmp/daml.yaml" + run ! grep -q "import qualified DA.Internal.Template$" "$tmp/Export.daml" + grep -q "^archiveCmd foo$" "$tmp/Export.daml" + rm -rf "$tmp" +} From 60fad0ff47a269e47fcafc071ce5e2e438e4d5de Mon Sep 17 00:00:00 2001 From: Kevin O'Donnell Date: Wed, 13 May 2026 02:55:28 -0400 Subject: [PATCH 4/5] test: add bats coverage for pagerduty-alert (SUR-2836) Adds tests/pagerduty-alert.bats covering the entry script's own logic (`bash/pagerduty.sh` is already exercised in tests/pagerduty.bats): 1. Missing required flags (-a / -s / -i) each individually exit non-zero with the help banner. 2. With `-i incident`, `pagerduty::send_incident` receives `service_id alert_type alert_title alert_from alert_token incident_key` in that order. 3. Default ALERT_TITLE ("Test Alert") and default ALERT_FROM ("no-reply@blockchaintp.com") are applied when the corresponding flags are omitted. 4. Non-`incident` `-i` values trigger `error::exit` with the documented message. 5. A non-zero return from the stubbed send propagates as `Failed to send incident` and a non-zero exit code. The stub seam reserves pagerduty.sh's @include dedup guard before sourcing the entry script, then in-process `pagerduty::send_incident` records argv to a log file. exec::hide is reused as-is. No production change to bash/pagerduty-alert was required. --- tests/pagerduty-alert.bats | 102 +++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 tests/pagerduty-alert.bats diff --git a/tests/pagerduty-alert.bats b/tests/pagerduty-alert.bats new file mode 100644 index 0000000..6aab071 --- /dev/null +++ b/tests/pagerduty-alert.bats @@ -0,0 +1,102 @@ +#!/usr/bin/env bats +bats_require_minimum_version 1.5.0 +# SUR-2836: direct coverage of bash/pagerduty-alert (the executable +# wrapper). The supporting library bash/pagerduty.sh has its own spec +# (tests/pagerduty.bats); this file targets the entry script's own +# logic: option validation, ALERT_TYPE dispatch, default ALERT_FROM +# and ALERT_TITLE fallbacks, the non-`incident` error::exit path, and +# propagation of a send failure. +# +# Stub seam: we suppress the real @include of pagerduty.sh by setting +# its dedup guard variable up-front, then define `pagerduty::send_incident` +# in-process to capture argv. exec.sh's `exec::hide` is reused as-is — +# it just runs its args, so the stubbed send still records the call. + +setup() { + load 'helpers.bash' + helpers::isolate_home + ALERT="$REPO_ROOT/bash/pagerduty-alert" + export LOGFILE_DISABLE=true LOG_DISABLE_DEBUG=true LOG_DISABLE_INFO=true +} + +# Run pagerduty-alert with a pre-installed `pagerduty::send_incident` +# stub that records argv to $log and returns $rc. Stubs are stamped in +# before sourcing pagerduty-alert by reserving pagerduty.sh's @include +# dedup guard. +_run_alert() { + local log=${1:?} + local rc=${2:?} + shift 2 + bash -c ' + set +e + log="$1" + rc="$2" + shift 2 + # Reserve pagerduty.sh dedup guard so @include skips it. + cksum=$(cksum "'"$REPO_ROOT"'/bash/pagerduty.sh" | awk "{print \$1}") + declare -g "include_${cksum}=include_${cksum}" + # Stub records argv and returns the requested exit code so we can + # exercise both the success path and the error::exit failure path. + pagerduty::send_incident() { + printf "%s\n" "$*" >>"$log" + return "$rc" + } + export -f pagerduty::send_incident + source "'"$ALERT"'" "$@" + ' _runner "$log" "$rc" "$@" +} + +@test "pagerduty-alert exits non-zero when -a is missing (SUR-2836 case 1)" { + run bash "$ALERT" -s svc -i incident + [ "$status" -ne 0 ] + [[ "$output" == *"Missing required option: -a"* ]] || [[ "$output" == *"-a"* ]] +} + +@test "pagerduty-alert exits non-zero when -s is missing (SUR-2836 case 1)" { + run bash "$ALERT" -a token -i incident + [ "$status" -ne 0 ] + [[ "$output" == *"Missing required option: -s"* ]] || [[ "$output" == *"-s"* ]] +} + +@test "pagerduty-alert exits non-zero when -i is missing (SUR-2836 case 1)" { + run bash "$ALERT" -a token -s svc + [ "$status" -ne 0 ] + [[ "$output" == *"Missing required option: -i"* ]] || [[ "$output" == *"-i"* ]] +} + +@test "pagerduty-alert dispatches incident with the documented argv order (SUR-2836 case 2)" { + log=$(mktemp) + _run_alert "$log" 0 \ + -a my-token -s SVC1 -i incident -t "My Title" -f "me@example.invalid" -k key-1 + [ "$(wc -l <"$log")" -eq 1 ] + read -r call <"$log" + # Order per pagerduty-alert: SERVICE_ID ALERT_TYPE ALERT_TITLE ALERT_FROM ALERT_TOKEN INCIDENT_KEY + [[ "$call" == "SVC1 incident My Title me@example.invalid my-token key-1" ]] + rm -f "$log" +} + +@test "pagerduty-alert supplies default ALERT_TITLE and ALERT_FROM (SUR-2836 case 3)" { + log=$(mktemp) + _run_alert "$log" 0 -a token -s SVC1 -i incident -k key-2 + read -r call <"$log" + [[ "$call" == *"Test Alert"* ]] + [[ "$call" == *"no-reply@blockchaintp.com"* ]] + rm -f "$log" +} + +@test "pagerduty-alert rejects unknown -i alert types via error::exit (SUR-2836 case 4)" { + run bash "$ALERT" -a token -s SVC1 -i event + [ "$status" -ne 0 ] + [[ "$output" == *"Unknown -i alert type"* ]] || [[ "$output" == *"only 'incident' is supported"* ]] +} + +@test "pagerduty-alert propagates send_incident failure (SUR-2836 case 5)" { + log=$(mktemp) + # rc=22 mirrors the curl --fail HTTP-error code used elsewhere in + # the pagerduty specs; any non-zero return must surface as the + # `Failed to send incident` error::exit path. + run _run_alert "$log" 22 -a token -s SVC1 -i incident + [ "$status" -ne 0 ] + [[ "$output" == *"Failed to send incident"* ]] + rm -f "$log" +} From 4119dc37cf8ed33ec590fb20106d7dff590dbb99 Mon Sep 17 00:00:00 2001 From: Kevin O'Donnell Date: Wed, 13 May 2026 02:59:39 -0400 Subject: [PATCH 5/5] docs(k8s-support-collector): clarify pod/ dir is load-bearing (SUR-2844) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SUR-2844 proposed that `describe_pods` creates an unused `$out_dir_ns/pod` directory that ships empty inside every collected support tarball. Investigation showed the premise is incorrect. `k8s::get_pod_names` wraps `kubectl get pods -o name`, which emits names in the `pod/` form. The subsequent redirect k8s::describe -n "$ns" "$pod" >"${out_dir_ns}/${pod}.describe" therefore expands to `${out_dir_ns}/pod/.describe` — files DO land under `pod/`, and the `dirs::ensure "$out_dir_ns/pod"` is needed for the redirect's parent dir to exist. Removing it (per the issue's suggested option a) causes the redirect to fail with "No such file or directory" and the collector misses every pod's describe output. This commit makes that constraint explicit in a comment so a future "cleanup" PR cannot regress this code path. No behavioural change. --- bash/k8s-support-collector | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/bash/k8s-support-collector b/bash/k8s-support-collector index c633b1e..053d241 100755 --- a/bash/k8s-support-collector +++ b/bash/k8s-support-collector @@ -71,6 +71,11 @@ function describe_pods { local out_dir_ns="${OUT_DIR}/$ns" log::notice "Describing pods for namespace $ns" dirs::ensure "$out_dir_ns" + # SUR-2844: load-bearing. `k8s::get_pod_names` returns the literal + # `pod/` form from `kubectl get pods -o name`, so the redirect + # below expands to `$out_dir_ns/pod/.describe`. Dropping this + # `dirs::ensure` (as the original issue suggested) breaks the redirect + # with "No such file or directory" — keep it. dirs::ensure "$out_dir_ns/pod" for pod in $(k8s::get_pod_names -n "$ns"); do k8s::describe -n "$ns" "$pod" >"${out_dir_ns}/$pod.describe"