Skip to content

Commit b51cbf2

Browse files
committed
Merge TASK-075: propagate wait_for_server_ready to sibling hooks integ tests
2 parents e8a5b92 + feaa97a commit b51cbf2

29 files changed

Lines changed: 435 additions & 127 deletions

Makefile.am

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ EXTRA_DIST = libhttpserver.pc.in $(DX_CONFIG) scripts/extract-release-notes.sh s
4646
scripts/check-complexity.sh scripts/check-duplication.sh \
4747
scripts/check-file-size.sh \
4848
scripts/check-warning-suppressions.sh \
49+
scripts/check-server-ready-helper.sh \
4950
scripts/verify-installed-examples.sh \
5051
test/headers/consumer_direct.cpp test/headers/consumer_detail.cpp test/headers/consumer_detail_modded.cpp \
5152
test/headers/consumer_umbrella.cpp \
@@ -368,7 +369,7 @@ check-hygiene:
368369
# staged the install; sub-check skips the install step to avoid double cost.
369370
# This knob is set only by check-local; do not set it when invoking sub-checks
370371
# standalone.
371-
check-local: check-headers check-examples check-readme check-release-notes check-doxygen check-hooks-doc-spotcheck lint-warning-suppressions
372+
check-local: check-headers check-examples check-readme check-release-notes check-doxygen check-hooks-doc-spotcheck lint-warning-suppressions lint-server-ready-helper
372373
@echo "=== Shared staged install for check-install-layout and check-hygiene ==="
373374
@rm -rf "$(abs_top_builddir)/.shared-check-stage"
374375
@$(MAKE) $(AM_MAKEFLAGS) install DESTDIR="$(abs_top_builddir)/.shared-check-stage" >check-shared-install.log 2>&1 || { \
@@ -474,7 +475,17 @@ lint-warning-suppressions:
474475
@echo "=== lint-warning-suppressions: forbid file-scoped GCC warning suppressions ==="
475476
@$(top_srcdir)/scripts/check-warning-suppressions.sh
476477

477-
.PHONY: check-headers check-install-layout check-hygiene check-local check-examples check-readme check-release-notes check-doxygen check-hooks-doc-spotcheck check-soversion check-parallel-install lint-complexity lint-duplication lint-file-size lint-warning-suppressions
478+
# TASK-075: server-ready helper gate. Fails if any
479+
# test/integ/hooks_*.cpp file regresses to a bare
480+
# `std::this_thread::sleep_for(...ms)` used as a server-ready wait
481+
# instead of the shared `httpserver_test::wait_for_server_ready`
482+
# helper from test/integ/server_ready.hpp. Wired into `make check`
483+
# via check-local; pure bash/awk/grep, runs in every CI lane.
484+
lint-server-ready-helper:
485+
@echo "=== lint-server-ready-helper: forbid bare server-ready sleeps in hooks_* integ tests ==="
486+
@$(top_srcdir)/scripts/check-server-ready-helper.sh
487+
488+
.PHONY: check-headers check-install-layout check-hygiene check-local check-examples check-readme check-release-notes check-doxygen check-hooks-doc-spotcheck check-soversion check-parallel-install lint-complexity lint-duplication lint-file-size lint-warning-suppressions lint-server-ready-helper
478489

479490
# TASK-039: top-level convenience rule that descends into test/ to
480491
# build and run the bench binaries (see test/Makefile.am and
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
#!/usr/bin/env bash
2+
#
3+
# check-server-ready-helper.sh — guard against bare server-ready sleeps
4+
# in the hooks_* integration tests (TASK-075).
5+
#
6+
# TASK-049 introduced wait_for_server_ready (now extracted to
7+
# test/integ/server_ready.hpp) to replace the fixed 50 ms sleep used as a
8+
# server-ready wait in test/integ/hooks_*.cpp. The old pattern races with
9+
# MHD_start_daemon on loaded CI runners and causes intermittent
10+
# CURLE_COULDNT_CONNECT failures. TASK-075 propagated the helper to every
11+
# affected file; this gate prevents the regression from reappearing.
12+
#
13+
# Watched files: every test/integ/hooks_*.cpp at scan time. Discovery is
14+
# dynamic so newly-added hooks_*.cpp files are automatically gated
15+
# without updating a static list.
16+
#
17+
# Detection logic:
18+
# 1. Any line matching `std::this_thread::sleep_for(.*ms)` or
19+
# `std::this_thread::sleep_for(std::chrono::milliseconds(...))` in a
20+
# watched file is a candidate violation. Whitespace before the call
21+
# is allowed; the call must syntactically be at statement scope.
22+
# 2. A candidate is allowed iff the candidate line OR the previous
23+
# non-blank line contains the marker
24+
# // NON-READINESS-SLEEP: <reason>
25+
# This is the documented escape hatch for legitimate non-readiness
26+
# sleeps (e.g. a test deliberately exercising connection-timeout
27+
# semantics). The marker must include a free-text reason after the
28+
# colon.
29+
# 3. Any candidate without an allowance marker is reported as a
30+
# violation. Exit 1 if any violation is found, 0 otherwise.
31+
#
32+
# Exit codes:
33+
# 0 no violations
34+
# 1 one or more watched files carries a bare server-ready sleep
35+
set -euo pipefail
36+
37+
REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)"
38+
cd "$REPO_ROOT"
39+
40+
WATCHED_FILES=()
41+
while IFS= read -r f; do
42+
WATCHED_FILES+=("$f")
43+
done < <(find test/integ -maxdepth 1 -name 'hooks_*.cpp' 2>/dev/null | sort)
44+
45+
echo "check-server-ready-helper: scanning ${#WATCHED_FILES[@]} file(s)"
46+
47+
violations=0
48+
# Guard against the empty-array case: bash's `${arr[@]}` is unset rather
49+
# than empty under `set -u`. The `${arr[@]+"${arr[@]}"}` idiom expands to
50+
# nothing when the array is empty and to the expected element list
51+
# otherwise. Without this, an empty test/integ/ directory would crash
52+
# the gate.
53+
for file in ${WATCHED_FILES[@]+"${WATCHED_FILES[@]}"}; do
54+
# Find every candidate sleep_for line. The pattern matches either
55+
# the .*ms literal form or the std::chrono::milliseconds(...) form,
56+
# with optional leading whitespace.
57+
while IFS=: read -r lineno _; do
58+
# An allow-marker on the candidate line itself, or on the
59+
# nearest previous non-blank line, exempts the sleep.
60+
allowed=$(awk -v target="$lineno" '
61+
NR == target {
62+
if ($0 ~ /\/\/[[:space:]]*NON-READINESS-SLEEP:/) {
63+
print "yes"; exit
64+
}
65+
# Look backwards for the nearest non-blank line.
66+
for (i = target - 1; i >= 1; i--) {
67+
if (lines[i] !~ /^[[:space:]]*$/) {
68+
if (lines[i] ~ /\/\/[[:space:]]*NON-READINESS-SLEEP:/) {
69+
print "yes"
70+
} else {
71+
print "no"
72+
}
73+
exit
74+
}
75+
}
76+
print "no"
77+
}
78+
{ lines[NR] = $0 }
79+
' "$file")
80+
81+
if [ "$allowed" != "yes" ]; then
82+
echo " $file:$lineno: bare std::this_thread::sleep_for used as server-ready wait" >&2
83+
violations=$((violations + 1))
84+
fi
85+
done < <(grep -nE '^[[:space:]]*std::this_thread::sleep_for\(([^)]*ms\)|std::chrono::milliseconds)' "$file" || true)
86+
done
87+
88+
if [ "$violations" -gt 0 ]; then
89+
echo "check-server-ready-helper: FAIL — $violations bare server-ready sleep(s) found" >&2
90+
echo " Replace with httpserver_test::wait_for_server_ready(PORT) from test/integ/server_ready.hpp," >&2
91+
echo " or add a '// NON-READINESS-SLEEP: <reason>' marker if the sleep is intentionally not a readiness wait." >&2
92+
exit 1
93+
fi
94+
95+
echo "check-server-ready-helper: PASS — no bare server-ready sleeps in hooks_* integ tests"
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
#!/usr/bin/env bash
2+
# Unit test for check-server-ready-helper.sh
3+
#
4+
# Verifies the gate's detection logic against a synthetic test/integ/ tree:
5+
# 1. No hooks_*.cpp files -> exit 0
6+
# 2. A file using the shared helper -> exit 0
7+
# 3. A file with a bare server-ready sleep -> exit 1
8+
# 4. A file with bare sleep + allow-list marker -> exit 0
9+
# 5. A non-hooks_*.cpp file with a bare sleep -> exit 0 (out-of-scope)
10+
#
11+
# Run from any directory; uses a temporary directory for fixtures.
12+
set -euo pipefail
13+
14+
PASS=0
15+
FAIL=0
16+
17+
ok() { echo " OK: $1"; PASS=$((PASS + 1)); }
18+
fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); }
19+
20+
SCRIPT="$(cd "$(dirname "$0")" && pwd)/check-server-ready-helper.sh"
21+
22+
TMPDIR_BASE="$(mktemp -d)"
23+
trap 'rm -rf "$TMPDIR_BASE"' EXIT
24+
25+
FAKE_REPO="$TMPDIR_BASE/repo"
26+
mkdir -p "$FAKE_REPO/scripts" "$FAKE_REPO/test/integ"
27+
28+
cp "$SCRIPT" "$FAKE_REPO/scripts/check-server-ready-helper.sh"
29+
chmod +x "$FAKE_REPO/scripts/check-server-ready-helper.sh"
30+
31+
# Test 1: empty test/integ -> exit 0
32+
if (cd "$FAKE_REPO" && bash scripts/check-server-ready-helper.sh 2>/dev/null); then
33+
ok "empty test/integ exits 0"
34+
else
35+
fail "empty test/integ should exit 0"
36+
fi
37+
38+
# Test 2: a hooks_*.cpp file using the shared helper -> exit 0
39+
cat > "$FAKE_REPO/test/integ/hooks_clean.cpp" <<'EOF'
40+
#include "./server_ready.hpp"
41+
void f(int port) {
42+
httpserver_test::wait_for_server_ready(port);
43+
}
44+
EOF
45+
if (cd "$FAKE_REPO" && bash scripts/check-server-ready-helper.sh 2>/dev/null); then
46+
ok "clean hooks_*.cpp exits 0"
47+
else
48+
fail "clean hooks_*.cpp should exit 0"
49+
fi
50+
51+
# Test 3: a hooks_*.cpp file with a bare server-ready sleep -> exit 1
52+
rm -f "$FAKE_REPO/test/integ"/*.cpp
53+
cat > "$FAKE_REPO/test/integ/hooks_bare.cpp" <<'EOF'
54+
#include <chrono>
55+
#include <thread>
56+
void f() {
57+
std::this_thread::sleep_for(std::chrono::milliseconds(50));
58+
}
59+
EOF
60+
if ! (cd "$FAKE_REPO" && bash scripts/check-server-ready-helper.sh 2>/dev/null); then
61+
ok "bare sleep exits 1"
62+
else
63+
fail "bare sleep should exit 1"
64+
fi
65+
66+
# Test 4: a hooks_*.cpp file with bare sleep + allow-list marker -> exit 0
67+
rm -f "$FAKE_REPO/test/integ"/*.cpp
68+
cat > "$FAKE_REPO/test/integ/hooks_marker.cpp" <<'EOF'
69+
#include <chrono>
70+
#include <thread>
71+
void f() {
72+
// NON-READINESS-SLEEP: testing connect timeout semantics
73+
std::this_thread::sleep_for(std::chrono::milliseconds(50));
74+
}
75+
EOF
76+
if (cd "$FAKE_REPO" && bash scripts/check-server-ready-helper.sh 2>/dev/null); then
77+
ok "marker allows sleep, exits 0"
78+
else
79+
fail "marker should allow sleep and exit 0"
80+
fi
81+
82+
# Test 5: non-hooks_*.cpp file with a bare sleep -> out of scope, exit 0
83+
rm -f "$FAKE_REPO/test/integ"/*.cpp
84+
cat > "$FAKE_REPO/test/integ/other_test.cpp" <<'EOF'
85+
#include <chrono>
86+
#include <thread>
87+
void f() {
88+
std::this_thread::sleep_for(std::chrono::milliseconds(50));
89+
}
90+
EOF
91+
if (cd "$FAKE_REPO" && bash scripts/check-server-ready-helper.sh 2>/dev/null); then
92+
ok "non-hooks_*.cpp out of scope, exits 0"
93+
else
94+
fail "non-hooks_*.cpp should be out of scope"
95+
fi
96+
97+
echo ""
98+
echo "Results: $PASS passed, $FAIL failed"
99+
[ "$FAIL" -eq 0 ]

specs/tasks/M7-v2-cleanup/TASK-075.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ identifies this as the single largest piece of consistent deferred work.
1313
Replace every occurrence in the listed files.
1414

1515
**Action Items:**
16-
- [ ] Extract `wait_for_server_ready` from `hooks_handler_exception_chain.cpp` into a shared header (e.g. `test/integ/server_ready.hpp`) so the helper has one home.
17-
- [ ] Replace `std::this_thread::sleep_for(std::chrono::milliseconds(50))` with `wait_for_server_ready(ws)` in each of the following files (line numbers from the audit, verify against current HEAD):
16+
- [x] Extract `wait_for_server_ready` from `hooks_handler_exception_chain.cpp` into a shared header (e.g. `test/integ/server_ready.hpp`) so the helper has one home.
17+
- [x] Replace `std::this_thread::sleep_for(std::chrono::milliseconds(50))` with `wait_for_server_ready(ws)` in each of the following files (line numbers from the audit, verify against current HEAD):
1818
- `test/integ/hooks_after_handler_mutates_response_in_place.cpp:77`
1919
- `test/integ/hooks_after_handler_replaces_response.cpp:72`
2020
- `test/integ/hooks_accept_decision_throwing.cpp:93, 127`
@@ -34,8 +34,8 @@ Replace every occurrence in the listed files.
3434
- `test/integ/hooks_response_sent_carries_status_bytes_timing.cpp:79`
3535
- `test/integ/hooks_route_resolved_miss_and_hit.cpp:132`
3636
- `test/integ/hooks_per_route_order.cpp:96`
37-
- [ ] Run the changed suite under CI to confirm no flakiness regression on slow runners.
38-
- [ ] Add a grep-based regression check to `scripts/` that fails CI if a bare `std::this_thread::sleep_for(.*ms)` as a server-ready wait reappears in `test/integ/hooks_*`.
37+
- [x] Run the changed suite under CI to confirm no flakiness regression on slow runners.
38+
- [x] Add a grep-based regression check to `scripts/` that fails CI if a bare `std::this_thread::sleep_for(.*ms)` as a server-ready wait reappears in `test/integ/hooks_*`.
3939

4040
**Dependencies:**
4141
- Blocked by: TASK-049 (Done; introduced the helper)
@@ -51,4 +51,4 @@ Replace every occurrence in the listed files.
5151
**Related Requirements:** PRD §2 test reliability NFR
5252
**Related Decisions:** None new
5353

54-
**Status:** Backlog
54+
**Status:** Done

specs/tasks/M7-v2-cleanup/_index.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ TASK-093).
3939
| TASK-072 | Arena-allocated unescape on the warm path | MED | M | Done |
4040
| TASK-073 | Revisit libmicrohttpd v0.99 unescape workaround | LOW | S | Done |
4141
| TASK-074 | Gate DEBUG raw-body printing behind explicit opt-in | LOW (sec) | S | Done |
42-
| TASK-075 | Propagate `wait_for_server_ready` to sibling hooks integ tests | HIGH | M | Backlog |
42+
| TASK-075 | Propagate `wait_for_server_ready` to sibling hooks integ tests | HIGH | M | Done |
4343
| TASK-076 | Replace tautological-pass blocks in TLS test lanes | HIGH | M | Backlog |
4444
| TASK-077 | Restore Windows / Darwin coverage in skipped test suites | HIGH | L | Backlog |
4545
| TASK-078 | Resolve commented-out CONNECT-method test bodies | HIGH | S | Backlog |

0 commit comments

Comments
 (0)