Skip to content

Add readiness warmup to restricted-config test to fix CI flakiness #154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kwsantiago merged 1 commit into from
Jul 12, 2026
Merged

Add readiness warmup to restricted-config test to fix CI flakiness #154

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 32 additions & 5 deletions tests/integration_restrict.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,45 @@ chk() { # desc expected actual
}

# publish and report ok|reject based on whether the relay accepted the event.
# Retries up to 3 times: a transient connect/read timeout under CI load (a fresh
# Retries up to 4 times: a transient connect/read timeout under CI load (a fresh
# TCP connection per attempt) should not fail the suite, while a real rejection
# or a deterministic bug still reports reject after all attempts.
# or a deterministic bug still reports reject after all attempts. The warmup gate
# below already establishes readiness, so this only absorbs the rarer mid-run blip.
pubres() { # noz-args...
for attempt in 1 2 3; do
if timeout 10 noz event "$@" "$R" 2>&1 | grep -q 'success'; then echo ok; return; fi
for attempt in 1 2 3 4; do
if timeout 12 noz event "$@" "$R" 2>&1 | grep -q 'success'; then echo ok; return; fi
[ -n "${NOZ_DEBUG_RETRIES:-}" ] && echo "pubres: attempt $attempt failed [noz event $*]" >&2
[ "$attempt" -lt 3 ] && sleep 1
[ "$attempt" -lt 4 ] && sleep 1
done
echo reject
}

# Block until the relay actually accepts an event of this mode's accepted class,
# not just until the TCP port is open. In CI the relay is started fresh per mode
# and `nc -z` only proves the listener is up, not that the event pipeline is
# ready; grading the first assertion against a still-warming relay is what made
# this suite flaky on main (a transient miss on an expected-accept assertion
# reports 'reject' and fails CI). Publishing an acceptable throwaway event until
# it succeeds converts "port open" into "pipeline ready", and is independent of
# noz's error-output format so it needs no reject-vs-timeout parsing.
warmup() {
local args
case "$MODE" in
auth) args=(--sec "$SEC1" --auth -c warmup) ;;
protected) args=(--sec "$SEC1" -c warmup) ;;
pow) args=(--sec "$SEC1" --pow 8 -c warmup) ;;
*) return 0 ;; # unknown mode is reported by the assertion case below
esac
for i in $(seq 1 40); do
timeout 12 noz event "${args[@]}" "$R" 2>&1 | grep -q 'success' && return 0
sleep 0.5
done
echo "warmup: relay never accepted an event in '$MODE' mode after ~20s" >&2
return 1
}

warmup || exit 1

Comment on lines +42 to +67

@coderabbitai coderabbitai Bot Jul 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Worst-case warmup duration far exceeds the ~20s comment and "fail fast" goal.

Each of the 40 iterations can take up to 12s (timeout 12) plus 0.5s sleep, yielding a ~500s worst case if the relay accepts connections but never responds. The comment on line 62 says "~20s" but that only accounts for sleep time. If the relay is in a bad state, CI will hang for 8+ minutes before failing — contradicting the PR's "fail fast" objective.

Consider using a shorter per-attempt timeout for the warmup (readiness probe, not a full operation) or tracking total elapsed time with a hard deadline.

⏱️ Suggested fix: shorter warmup timeout 
   for i in $(seq 1 40); do
-    timeout 12 noz event "${args[@]}" "$R" 2>&1 | grep -q 'success' && return 0
+    timeout 3 noz event "${args[@]}" "$R" 2>&1 | grep -q 'success' && return 0
     sleep 0.5
   done
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Block until the relay actually accepts an event of this mode's accepted class,
# not just until the TCP port is open. In CI the relay is started fresh per mode
# and `nc -z` only proves the listener is up, not that the event pipeline is
# ready; grading the first assertion against a still-warming relay is what made
# this suite flaky on main (a transient miss on an expected-accept assertion
# reports 'reject' and fails CI). Publishing an acceptable throwaway event until
# it succeeds converts "port open" into "pipeline ready", and is independent of
# noz's error-output format so it needs no reject-vs-timeout parsing.
warmup() {
local args
case "$MODE" in
auth) args=(--sec "$SEC1" --auth -c warmup) ;;
protected) args=(--sec "$SEC1" -c warmup) ;;
pow) args=(--sec "$SEC1" --pow 8 -c warmup) ;;
*) return 0 ;; # unknown mode is reported by the assertion case below
esac
for i in $(seq 1 40); do
timeout 12 noz event "${args[@]}" "$R" 2>&1 | grep -q 'success' && return 0
sleep 0.5
done
echo "warmup: relay never accepted an event in '$MODE' mode after ~20s" >&2
return 1
}
warmup || exit 1
# Block until the relay actually accepts an event of this mode's accepted class,
# not just until the TCP port is open. In CI the relay is started fresh per mode
# and `nc -z` only proves the listener is up, not that the event pipeline is
# ready; grading the first assertion against a still-warming relay is what made
# this suite flaky on main (a transient miss on an expected-accept assertion
# reports 'reject' and fails CI). Publishing an acceptable throwaway event until
# it succeeds converts "port open" into "pipeline ready", and is independent of
# noz's error-output format so it needs no reject-vs-timeout parsing.
warmup() {
local args
case "$MODE" in
auth) args=(--sec "$SEC1" --auth -c warmup) ;;
protected) args=(--sec "$SEC1" -c warmup) ;;
pow) args=(--sec "$SEC1" --pow 8 -c warmup) ;;
*) return 0 ;; # unknown mode is reported by the assertion case below
esac
for i in $(seq 1 40); do
timeout 3 noz event "${args[@]}" "$R" 2>&1 | grep -q 'success' && return 0
sleep 0.5
done
echo "warmup: relay never accepted an event in '$MODE' mode after ~20s" >&2
return 1
}
warmup || exit 1
🧰 Tools
🪛 Shellcheck (0.11.0)

[warning] 58-58: i appears unused. Verify use (or export if used externally).

(SC2034)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration_restrict.sh` around lines 42 - 67, Reduce the per-attempt
timeout in the warmup function so its 40 retries, including sleeps, complete
within the documented approximately 20-second limit. Keep the existing success
detection, retry count, and failure behavior unchanged, and update the nearby
duration comment if needed to accurately reflect the enforced deadline.

case "$MODE" in
auth)
chk "NIP-42 publish without auth is rejected" reject "$(pubres --sec $SEC1 -c noauth)"
Expand Down
Loading