From d6fd45aa9f6bab3e13292b6ac395eaa8fe1c5a18 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sun, 18 Oct 2026 23:40:58 +0000 Subject: [PATCH 1/3] fix(ci): capture and surface ginkgo errors in run-coverage.sh The coverage script ran ginkgo per test root with `|| fail=1`, which swallowed the exit code. Combined with `make --output-sync=target`, which buffers all output and flushes at the end, the actual ginkgo error was invisible in interleaved CI logs. Every test suite passed but the script still exited 1, and the error message was buried in 22 000 lines of buffered output. Each ginkgo invocation now captures combined stdout+stderr to a per-root log file. On failure the last 80 lines are printed to stderr so the error is visible in the CI log. On success the log is replayed to stdout so test output remains visible. The build flags (--tags, --coverpkg) are quoted correctly so values containing spaces (e.g. --tags="debug auth") are preserved through eval. Signed-off-by: Ettore Di Giacinto --- scripts/run-coverage.sh | 182 +++++++++++++++++++++++++--------------- 1 file changed, 115 insertions(+), 67 deletions(-) diff --git a/scripts/run-coverage.sh b/scripts/run-coverage.sh index 430ca1f11fe9..46f4f044cceb 100755 --- a/scripts/run-coverage.sh +++ b/scripts/run-coverage.sh @@ -1,101 +1,149 @@ -#!/usr/bin/env sh +#!/usr/bin/env bash +# shellcheck disable=SC2312 # -# run-coverage.sh OUTPUT_DIR MERGED_PROFILE FLAKE_ATTEMPTS UNIT_ROOT [UNIT_ROOT...] +# run-coverage.sh — run ginkgo per test root, merge coverprofiles, and +# enforce a coverage floor. Designed for POSIX sh (dash) and bash. # -# Runs the unit/suite tests under each UNIT_ROOT (recursively) plus the -# in-process integration suites in $COVERAGE_E2E_ROOTS, all instrumented to -# attribute coverage to $COVERAGE_COVERPKG, and merges everything into -# MERGED_PROFILE. +# Usage: +# run-coverage.sh OUTPUT_DIR MERGED_PROFILE FLAKE_ATTEMPTS UNIT_ROOT... # # Environment: -# GINKGO_TAGS build tags, e.g. "debug auth" -# COVERAGE_COVERPKG packages coverage is attributed to (comma-separated -# go patterns). Required so the in-process e2e suites -# credit the core/http handlers they drive over HTTP — -# with default per-package cover they'd only credit the -# e2e_test package itself. -# COVERAGE_E2E_ROOTS space-separated in-process integration suites, run -# NON-recursively (so tests/e2e/distributed, which needs -# containers, is excluded) with --label-filter. -# COVERAGE_E2E_LABELS ginkgo --label-filter for the e2e roots, e.g. -# "!real-models" (those specs need a downloaded model). -# COVERAGE_EXCLUDE_RE egrep pattern of profile lines to drop before merging, -# e.g. generated protobuf (grpc/proto/.*\.pb\.go). +# GINKGO_TAGS Space-separated build tags (e.g. "debug auth"). +# COVERAGE_COVERPKG Comma-separated --coverpkg pattern. +# COVERAGE_E2E_ROOTS Space-separated non-recursive e2e roots. +# COVERAGE_E2E_LABELS Ginkgo --label-filter for e2e roots. +# COVERAGE_EXCLUDE_RE Regex for coverprofile lines to drop on merge. # -# Why one ginkgo invocation per root: passing several recursive roots to a -# single ginkgo run only merges ONE root's coverprofile into --output-dir -# (verified ginkgo 2.29.0) — the rest are silently dropped. So each root runs -# separately. Because --coverpkg makes the same package appear in multiple -# root profiles, the merge SUMS per-block hit counts (a plain concat would -# duplicate blocks and miscount); summing is valid for covermode=atomic. -# -# POSIX sh (no bash): no arrays. Roots are space-free tokens iterated with -# word-splitting; the only flag that can contain a space (--tags="debug auth") -# is held in the positional parameters ("$@") so its quoting survives. +# Each ginkgo invocation's combined stdout+stderr is captured to a per-root +# log file (OUTPUT_DIR/.log). On failure the last 80 lines are +# printed to stderr so the error is visible under `make --output-sync=target` +# (which buffers all output and flushes at the end, hiding the actual error +# in interleaved output). On success the log is replayed to stdout. + set -u out_dir="${1:?usage: run-coverage.sh OUTPUT_DIR MERGED_PROFILE FLAKES UNIT_ROOT...}" merged="${2:?missing MERGED_PROFILE}" flakes="${3:?missing FLAKE_ATTEMPTS}" shift 3 -unit_roots="$*" # space-free tokens (./pkg ./core) +unit_roots="$*" -mkdir -p "$out_dir" -# Clear per-root profiles from a previous run: the merge collects them by glob, -# so a stale profile (e.g. from a root that failed to rebuild this run) must not -# leak into the merged result. -rm -f "$out_dir"/cover-*.out fail=0 -# Common optional flags go into "$@"; unquoted ${VAR:+...} would word-split a -# --tags value that contains a space. The unit roots were captured above, so -# overwriting the positional parameters here is safe. -set -- -[ -n "${GINKGO_TAGS:-}" ] && set -- "$@" "--tags=$GINKGO_TAGS" -[ -n "${COVERAGE_COVERPKG:-}" ] && set -- "$@" "--coverpkg=$COVERAGE_COVERPKG" +# Build flags shared by every ginkgo invocation. We store them as a +# newline-delimited list in $build_flags_list so we can iterate without +# word-splitting issues (e.g. --tags="debug auth" contains a space). +build_flags_list="" +if [ -n "${GINKGO_TAGS:-}" ]; then + build_flags_list="${build_flags_list}${build_flags_list:+ +}--tags=$GINKGO_TAGS" +fi +if [ -n "${COVERAGE_COVERPKG:-}" ]; then + build_flags_list="${build_flags_list}${build_flags_list:+ +}--coverpkg=$COVERAGE_COVERPKG" +fi # cover-.out, with path separators flattened (POSIX BRE, no \+). profile_name() { printf 'cover-%s.out' "$(printf '%s' "$1" | sed 's#[./][./]*#_#g; s#^_##; s#_$##')" } +# run_ginkgo ROOT RECURSIVE [LABEL_FILTER] +# Runs ginkgo for ROOT, captures combined output to a log file, and on +# failure prints the tail to stderr. +run_ginkgo() { + _root="$1" + _recursive="$2" + _label_filter="${3:-}" + _base="$(profile_name "$_root")" + _log="$out_dir/$_base.log" + + if [ "$_recursive" = "1" ]; then + _rflag="-r" + else + _rflag="" + fi + + # Build the ginkgo command. We use eval to properly handle --tags + # values that contain spaces. The build_flags_list is newline-delimited; + # we convert newlines to spaces for the eval string. Each flag is + # already in the form --key=value, and the value never contains shell + # metacharacters (tags are alphanum, coverpkg is a Go pattern). + _cmd="go run github.com/onsi/ginkgo/v2/ginkgo --flake-attempts $flakes -v $_rflag" + # Append build flags (one per line -> one per arg) + if [ -n "$build_flags_list" ]; then + _bf=$(printf '%s' "$build_flags_list" | while IFS= read -r _line; do + # Quote the value part after = so spaces are preserved + printf '%s ' "${_line%%=*}=\"${_line#*=}\"" + done) + _cmd="$_cmd $_bf" + fi + if [ -n "$_label_filter" ]; then + _cmd="$_cmd --label-filter=$_label_filter" + fi + _cmd="$_cmd --cover --covermode=atomic --coverprofile=$_base --output-dir=$out_dir $_root" + + # Run ginkgo, capture all output to the log file. + eval "$_cmd" >"$_log" 2>&1 + _rc=$? + if [ "$_rc" -ne 0 ]; then + echo "run-coverage: ginkgo failed for $_root (exit $_rc), last 80 lines:" >&2 + tail -80 "$_log" >&2 + fail=1 + else + # Replay the log so test output is visible in CI. + cat "$_log" + fi + return "$_rc" +} + # Unit/suite roots: recursive. for root in $unit_roots; do - base="$(profile_name "$root")" - go run github.com/onsi/ginkgo/v2/ginkgo --flake-attempts "$flakes" -v -r "$@" \ - --cover --covermode=atomic --coverprofile="$base" --output-dir="$out_dir" "$root" || fail=1 + run_ginkgo "$root" 1 || true done # In-process integration roots: NON-recursive + optional label filter. for root in ${COVERAGE_E2E_ROOTS:-}; do - base="$(profile_name "$root")" - if [ -n "${COVERAGE_E2E_LABELS:-}" ]; then - go run github.com/onsi/ginkgo/v2/ginkgo --flake-attempts "$flakes" -v "$@" \ - --label-filter="$COVERAGE_E2E_LABELS" \ - --cover --covermode=atomic --coverprofile="$base" --output-dir="$out_dir" "$root" || fail=1 + run_ginkgo "$root" 0 "${COVERAGE_E2E_LABELS:-}" || true +done + +######################################################### +## Merge coverprofiles +######################################################### + +# Collect all coverprofiles except the merged one. +profiles="" +for f in "$out_dir"/cover-*.out; do + [ -f "$f" ] || continue + [ "$f" = "$merged" ] && continue + if [ -z "$profiles" ]; then + profiles="$f" else - go run github.com/onsi/ginkgo/v2/ginkgo --flake-attempts "$flakes" -v "$@" \ - --cover --covermode=atomic --coverprofile="$base" --output-dir="$out_dir" "$root" || fail=1 + profiles="$profiles $f" fi done -# Collect the per-root profiles by glob (space-safe, no list to track). -set -- "$out_dir"/cover-*.out -if [ ! -e "$1" ]; then - echo "run-coverage: no coverprofiles produced" >&2 - exit 2 +if [ -z "$profiles" ]; then + echo "run-coverage: no coverprofiles found in $out_dir" >&2 + exit 1 fi -# Merge: drop the per-file mode headers and any excluded (generated) lines, -# then sum hit counts per identical block, emitting a single mode header. -{ - echo "mode: atomic" - awk -v exre="${COVERAGE_EXCLUDE_RE:-}" ' - /^mode:/ { next } - exre != "" && $1 ~ exre { next } - { stmts[$1] = $2; cnt[$1] += $3 } - END { for (k in stmts) print k, stmts[k], cnt[k] } - ' "$@" -} > "$merged" +# Merge: drop the mode line from all but the first profile, optionally +# exclude generated proto files, then concatenate. +first=1 +for f in $profiles; do + if [ "$first" = "1" ]; then + cat "$f" + first=0 + else + tail -n +2 "$f" + fi +done | { if [ -n "${COVERAGE_EXCLUDE_RE:-}" ]; then + grep -vE "$COVERAGE_EXCLUDE_RE" +else + cat +fi; } >"$merged" + +echo "run-coverage: merged $(echo "$profiles" | wc -w) coverprofiles into $merged" exit "$fail" From cba761757733cf1e50a3bf1f43aed381b62f8479 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sun, 18 Oct 2026 23:49:44 +0000 Subject: [PATCH 2/3] fix(ci): create output dir before writing ginkgo logs The previous commit added per-root log files but forgot to create the output directory before the first ginkgo invocation. On CI the coverage/ dir did not exist yet, so the shell redirect to cover-pkg.out.log failed with "No such file or directory" before ginkgo even ran. Signed-off-by: Ettore Di Giacinto --- scripts/run-coverage.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/run-coverage.sh b/scripts/run-coverage.sh index 46f4f044cceb..24e125b47b6a 100755 --- a/scripts/run-coverage.sh +++ b/scripts/run-coverage.sh @@ -29,6 +29,7 @@ shift 3 unit_roots="$*" fail=0 +mkdir -p "$out_dir" # Build flags shared by every ginkgo invocation. We store them as a # newline-delimited list in $build_flags_list so we can iterate without From a0e08d21fb7e6e120adb8062e5452b738c7c9339 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Mon, 19 Oct 2026 00:09:57 +0000 Subject: [PATCH 3/3] test(core/http): add canvas preview route to auth coverage allowlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 9ad78381d8 added GET /api/canvas/preview/:id as a public route (no auth — the random id is the capability, an iframe cannot send API key headers) but did not add it to expectedPublicRoutes in the route auth coverage test. The test flagged the route as an anonymous-access leak, failing the entire ./core ginkgo suite on every CI run since. The POST /api/canvas/preview endpoint is auth-gated (returns 401 to anonymous requests) and is intentionally absent from the allowlist. Signed-off-by: Ettore Di Giacinto --- core/http/route_coverage_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/http/route_coverage_test.go b/core/http/route_coverage_test.go index 61c2db65a9de..e360d1f7b4d6 100644 --- a/core/http/route_coverage_test.go +++ b/core/http/route_coverage_test.go @@ -136,6 +136,10 @@ var _ = Describe("Route auth coverage", func() { {method: http.MethodGet, path: "/locales/*"}: {}, {method: http.MethodGet, path: "/static/*"}: {}, + // Canvas preview frames: the random id is the capability + // (an