From fc3f045868ffc15803fd700b8b98eeb743fbbfaa Mon Sep 17 00:00:00 2001 From: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com> Date: Wed, 15 Jul 2026 08:59:26 +0000 Subject: [PATCH 1/2] =?UTF-8?q?feat:=20implement=20issue=20#739=20?= =?UTF-8?q?=E2=80=94=20[Fleet=20Monitor]=20petry-projects/.github=20?= =?UTF-8?q?=E2=80=94=20.github/workflows/pr-review-mention-tests.yml?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/pr-review-mention-tests.yml | 10 ++++-- .../pr-review-mention/install-resilience.bats | 34 +++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) create mode 100644 test/workflows/pr-review-mention/install-resilience.bats diff --git a/.github/workflows/pr-review-mention-tests.yml b/.github/workflows/pr-review-mention-tests.yml index 0c9ca8960..0ddd992fa 100644 --- a/.github/workflows/pr-review-mention-tests.yml +++ b/.github/workflows/pr-review-mention-tests.yml @@ -57,13 +57,17 @@ jobs: # --proto '=https' --proto-redir '=https' enforces HTTPS on both the # initial request and any redirects (GitHub releases redirect to # objects.githubusercontent.com). - curl -sSfL --proto '=https' --proto-redir '=https' \ + # --retry 5 --retry-delay 2 --retry-connrefused --retry-all-errors + # rides out the transient network/5xx blips from that redirect target + # that otherwise flake this suite (#739); the budget stays bounded so a + # genuinely-down mirror still fails the job fast. + curl -sSfL --proto '=https' --proto-redir '=https' --retry 5 --retry-delay 2 --retry-connrefused --retry-all-errors \ "https://github.com/mikefarah/yq/releases/download/${YQ_VERSION}/yq_linux_amd64" \ -o /tmp/yq_linux_amd64 - curl -sSfL --proto '=https' --proto-redir '=https' \ + curl -sSfL --proto '=https' --proto-redir '=https' --retry 5 --retry-delay 2 --retry-connrefused --retry-all-errors \ "https://github.com/mikefarah/yq/releases/download/${YQ_VERSION}/checksums" \ -o /tmp/yq_checksums - curl -sSfL --proto '=https' --proto-redir '=https' \ + curl -sSfL --proto '=https' --proto-redir '=https' --retry 5 --retry-delay 2 --retry-connrefused --retry-all-errors \ "https://github.com/mikefarah/yq/releases/download/${YQ_VERSION}/checksums_hashes_order" \ -o /tmp/yq_checksums_order # The checksums file puts filename first; use checksums_hashes_order to diff --git a/test/workflows/pr-review-mention/install-resilience.bats b/test/workflows/pr-review-mention/install-resilience.bats new file mode 100644 index 000000000..1d40aa814 --- /dev/null +++ b/test/workflows/pr-review-mention/install-resilience.bats @@ -0,0 +1,34 @@ +#!/usr/bin/env bats +# Regression guard for the flaky "Install bats, shellcheck, jq, and yq" step in +# .github/workflows/pr-review-mention-tests.yml. +# +# Pins issue #739: the suite intermittently fails (~14% of runs) because the +# pinned mikefarah/yq is fetched from GitHub Releases — which redirects to +# objects.githubusercontent.com — with a bare `curl` that aborts on the first +# transient network/5xx blip. AGENTS.md requires unit tests be deterministic; +# the one unavoidable network fetch must therefore retry with backoff. These +# tests assert every GitHub-Releases download in the workflow carries bounded +# curl retries, so a future edit can't silently reintroduce a bare download. + +load 'helpers/setup' + +TESTS_WORKFLOW="${TT_REPO_ROOT}/.github/workflows/pr-review-mention-tests.yml" + +@test "install: the tests workflow exists" { + [ -f "$TESTS_WORKFLOW" ] +} + +@test "install: every curl download uses bounded retries with backoff" { + [ -f "$TESTS_WORKFLOW" ] + mapfile -t curls < <(grep -E '^[[:space:]]*curl ' "$TESTS_WORKFLOW") + # yq binary, checksums, checksums_hashes_order — three fetches, all resilient. + [ "${#curls[@]}" -ge 3 ] + for line in "${curls[@]}"; do + # A finite retry budget (not unbounded) so a truly-down mirror still fails fast. + [[ "$line" == *"--retry "* ]] + # Retry on connection refused, which curl otherwise treats as non-transient. + [[ "$line" == *"--retry-connrefused"* ]] + # Retry on transient HTTP 5xx too, not just connection-level errors. + [[ "$line" == *"--retry-all-errors"* ]] + done +} From 758d20c7cabfded20f479c8ba13ed3b503186ab8 Mon Sep 17 00:00:00 2001 From: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com> Date: Wed, 15 Jul 2026 09:09:40 +0000 Subject: [PATCH 2/2] fix(reviews): address review comments [skip ci-relay] --- .../pr-review-mention/install-resilience.bats | 37 ++++++++++++++----- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/test/workflows/pr-review-mention/install-resilience.bats b/test/workflows/pr-review-mention/install-resilience.bats index 1d40aa814..126d98ff2 100644 --- a/test/workflows/pr-review-mention/install-resilience.bats +++ b/test/workflows/pr-review-mention/install-resilience.bats @@ -6,7 +6,7 @@ # pinned mikefarah/yq is fetched from GitHub Releases — which redirects to # objects.githubusercontent.com — with a bare `curl` that aborts on the first # transient network/5xx blip. AGENTS.md requires unit tests be deterministic; -# the one unavoidable network fetch must therefore retry with backoff. These +# the one unavoidable network fetch must therefore retry on transient errors. These # tests assert every GitHub-Releases download in the workflow carries bounded # curl retries, so a future edit can't silently reintroduce a bare download. @@ -18,17 +18,36 @@ TESTS_WORKFLOW="${TT_REPO_ROOT}/.github/workflows/pr-review-mention-tests.yml" [ -f "$TESTS_WORKFLOW" ] } -@test "install: every curl download uses bounded retries with backoff" { +@test "install: every curl download uses bounded retries" { [ -f "$TESTS_WORKFLOW" ] - mapfile -t curls < <(grep -E '^[[:space:]]*curl ' "$TESTS_WORKFLOW") - # yq binary, checksums, checksums_hashes_order — three fetches, all resilient. - [ "${#curls[@]}" -ge 3 ] - for line in "${curls[@]}"; do + + # Parse the workflow joining backslash-continued lines so that a curl command + # split across multiple lines is treated as a single logical invocation. + # This avoids false failures when flags appear on continuation lines, and avoids + # false passes when a bare `curl` is split from its flags. + local curls=() + local current="" + while IFS= read -r line || [[ -n "$line" ]]; do + line="${line%$'\r'}" + if [[ "$line" == *'\\' ]]; then + current+="${line%\\} " + else + current+="$line" + if [[ "$current" == *curl* ]] && ! [[ "$current" =~ ^[[:space:]]*'#' ]]; then + curls+=("$current") + fi + current="" + fi + done < "$TESTS_WORKFLOW" + + # At least one curl invocation must be present so the test remains meaningful. + [ "${#curls[@]}" -ge 1 ] + for cmd in "${curls[@]}"; do # A finite retry budget (not unbounded) so a truly-down mirror still fails fast. - [[ "$line" == *"--retry "* ]] + [[ "$cmd" == *"--retry "* ]] # Retry on connection refused, which curl otherwise treats as non-transient. - [[ "$line" == *"--retry-connrefused"* ]] + [[ "$cmd" == *"--retry-connrefused"* ]] # Retry on transient HTTP 5xx too, not just connection-level errors. - [[ "$line" == *"--retry-all-errors"* ]] + [[ "$cmd" == *"--retry-all-errors"* ]] done }