Skip to content

Commit 4723c63

Browse files
ci(h2spec): harden gate + restore mbedtls header trigger (Codex review)
Builds on the parallel fixes already on this branch (VEXE unset; server*.v / http*.v added to the paths filter), addressing the remaining Codex items: - run_h2spec.sh: a recorded baseline case that now PASSES fails the gate (was a soft warning), forcing its removal from h2spec_expected_failures.txt so the gate cannot silently keep allowing a since-fixed case (Codex P2). - run_h2spec.sh: resolve $VEXE/$H2SPEC_BIN against the caller's cwd before the script cd's into its own dir, so relative values (e.g. VEXE=./vnew) work even if re-enabled, and a relative H2SPEC_BIN resolves correctly (Codex P1/P3). - .github/workflows/h2spec.yml: mbedtls trigger back to `**` (not `*.v`) so the #insert'ed headers (mbedtls_helpers.h / mbedtls_threading.h), which affect TLS/ALPN, also fire the gate (Codex P2). - run_h2spec.sh: run h2spec with --timeout 5 (H2SPEC_TIMEOUT). Several cases (CONTINUATION sequencing, unknown error codes) wait for the server's GOAWAY/close and flake at h2spec's 2s default under load; the 37-case baseline is stable at 5s across repeated runs. Co-Authored-By: WOZCODE <contact@withwoz.com>
1 parent cc247c2 commit 4723c63

4 files changed

Lines changed: 52 additions & 14 deletions

File tree

.github/workflows/h2spec.yml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ on:
1212
- 'vlib/net/http/server*.v'
1313
- 'vlib/net/http/http*.v'
1414
- 'vlib/net/http/h2spec/**'
15-
- 'vlib/net/mbedtls/*.v'
15+
# `**` (not `*.v`): mbedtls.c.v #inserts mbedtls_helpers.h / mbedtls_threading.h,
16+
# so TLS/ALPN behavior can change via the headers too (Codex P2).
17+
- 'vlib/net/mbedtls/**'
1618
- '.github/workflows/h2spec.yml'
1719
pull_request:
1820
paths: *h2paths

vlib/net/http/h2spec/README.md

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,15 @@ runner never installs anything — it expects `h2spec` to be provided.
4343
server driver `test_server_tls_h2_negotiation` exercises end-to-end).
4444
-**The harness was run end-to-end with a pinned h2spec v2.6.0**: 146 tests,
4545
**109 pass / 37 fail** against the server at this branch's base. The 37-failure
46-
set was **identical across repeated runs** (deterministic), and is recorded in
47-
`h2spec_expected_failures.txt` as the gate baseline. The JUnit parser is
48-
validated against that real report.
46+
set was **identical across four runs** (`--timeout 2` and `--timeout 5`, two
47+
each) and is recorded in `h2spec_expected_failures.txt` as the gate baseline.
48+
The JUnit parser is validated against those real reports.
49+
- ⚠️ **Timing flakiness — handled.** A *separate* handful of h2spec cases
50+
(CONTINUATION sequencing, unknown error codes) wait for the server's GOAWAY/close
51+
and flake at h2spec's 2s default under load. The runner therefore uses
52+
`--timeout 5` (`H2SPEC_TIMEOUT`). The lesson: always confirm the failure set is
53+
stable across repeated runs before trusting a per-case h2spec gate — a single
54+
run can mis-attribute a flaky timeout as a real failure (or regression).
4955
- The baseline reflects genuine server-side conformance gaps (see the file's
5056
header). Each is a tracked item; remove a line as the server is hardened.
5157
- The baseline was generated locally; the first CI run on Linux validates it. If

vlib/net/http/h2spec/h2spec_expected_failures.txt

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,13 @@
88
# passing, run_h2spec.sh warns so the line can be removed and the gate tightened.
99
#
1010
# Generated with h2spec v2.6.0 against the net.http server at this branch's base
11-
# (146 tests: 109 pass, 37 fail; the set was identical across repeated runs, i.e.
12-
# deterministic). These are genuine server-side conformance gaps: stricter request
11+
# (146 tests: 109 pass, 37 fail). The set was identical across four runs (h2spec
12+
# --timeout 2 and --timeout 5, two each) — stable. NOTE: a handful of OTHER h2spec
13+
# cases (CONTINUATION sequencing, unknown error codes) are timing-flaky at h2spec's
14+
# 2s default — they wait for the server's GOAWAY/close — so run_h2spec.sh uses
15+
# --timeout 5 (H2SPEC_TIMEOUT) to keep the gate stable. Do not add a flaky case to
16+
# this list; raise the timeout instead. These 37 are genuine server-side conformance
17+
# gaps: stricter request
1318
# validation (uppercase/duplicate/missing pseudo-headers, connection-specific & TE
1419
# headers, content-length vs DATA), HPACK decode error SCOPE (must be a
1520
# connection-level COMPRESSION_ERROR, not RST_STREAM), the stream state machine

vlib/net/http/h2spec/run_h2spec.sh

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,22 +16,43 @@
1616
# `go install` — the version must be pinned and reproducible.
1717
#
1818
# Env:
19-
# VEXE path to the V compiler (default: ./v then ./vnew)
20-
# H2SPEC_BIN path to the h2spec binary (default: h2spec on PATH)
21-
# H2SPEC_PORT port for the target server (default: 18443)
22-
# VFLAGS_CC extra V flags, e.g. '-cc gcc' (mbedtls needs a real C compiler)
19+
# VEXE path to the V compiler (default: ./v then ./vnew)
20+
# H2SPEC_BIN path to the h2spec binary (default: h2spec on PATH)
21+
# H2SPEC_PORT port for the target server (default: 18443)
22+
# H2SPEC_TIMEOUT h2spec per-case timeout, seconds (default: 5). Several h2spec
23+
# cases wait for the server's GOAWAY/close and FLAKE at h2spec's
24+
# 2s default under load; 5s is comfortably stable here. Raise it
25+
# if a timing case still flakes on a slow runner.
26+
# VFLAGS_CC extra V flags, e.g. '-cc gcc' (mbedtls needs a real C compiler)
2327

2428
set -uo pipefail
29+
orig_pwd="$(pwd)" # the caller's cwd, captured BEFORE we cd into the script dir
2530
cd "$(dirname "$0")"
2631
here="$(pwd)"
2732
repo_root="$(cd ../../../.. && pwd)"
2833

34+
# resolve a caller-supplied path argument: an absolute path or a bare command name
35+
# (PATH lookup, e.g. "h2spec") is used as-is; a relative path is resolved against
36+
# the caller's original cwd, since we cd'd into the script dir above. Without this,
37+
# a documented value like `VEXE=./vnew` (relative to the repo root) would wrongly
38+
# resolve to vlib/net/http/h2spec/vnew and the build would fail. (Codex P1/P3.)
39+
resolve_path() {
40+
case "$1" in
41+
/*) printf '%s' "$1" ;; # absolute path
42+
*/*) printf '%s' "$orig_pwd/$1" ;; # relative path containing a slash
43+
*) printf '%s' "$1" ;; # bare name -> PATH lookup
44+
esac
45+
}
46+
2947
VEXE="${VEXE:-}"
3048
if [ -z "$VEXE" ]; then
3149
if [ -x "$repo_root/v" ]; then VEXE="$repo_root/v"; elif [ -x "$repo_root/vnew" ]; then VEXE="$repo_root/vnew"; else VEXE="v"; fi
50+
else
51+
VEXE="$(resolve_path "$VEXE")"
3252
fi
33-
H2SPEC_BIN="${H2SPEC_BIN:-h2spec}"
53+
H2SPEC_BIN="$(resolve_path "${H2SPEC_BIN:-h2spec}")"
3454
PORT="${H2SPEC_PORT:-18443}"
55+
TIMEOUT="${H2SPEC_TIMEOUT:-5}"
3556
SERVER_BIN="$here/h2spec_server.bin"
3657
EXPECTED="$here/h2spec_expected_failures.txt"
3758

@@ -64,7 +85,7 @@ echo "==> running h2spec (TLS, insecure cert OK)"
6485
# contract, unlike the pretty console output. A failing case has a <failure>
6586
# (or <error>) child; we use "classname :: name" as the stable case ID.
6687
report_xml="$here/h2spec_report.xml"
67-
"$H2SPEC_BIN" -t -k -h 127.0.0.1 -p "$PORT" --junit-report "$report_xml" 2>&1 | tail -40 || true
88+
"$H2SPEC_BIN" -t -k -h 127.0.0.1 -p "$PORT" --timeout "$TIMEOUT" --junit-report "$report_xml" 2>&1 | tail -40 || true
6889
if [ ! -s "$report_xml" ]; then
6990
echo "ERROR: h2spec produced no JUnit report ($report_xml); check the flags/version." >&2
7091
exit 2
@@ -92,9 +113,13 @@ if [ -n "$new_failures" ]; then
92113
rc=1
93114
fi
94115
if [ -n "$now_passing" ]; then
95-
echo "::warning:: these recorded failures now PASS — remove them from h2spec_expected_failures.txt:" >&2
116+
# Fail too (not just warn): a recorded failure that now passes must be removed
117+
# from the baseline, otherwise the gate keeps allowing that case and a later
118+
# PR can reintroduce the failure unnoticed. (Codex P2.) The 37 baseline cases
119+
# are deterministic at H2SPEC_TIMEOUT=5, so this does not flap.
120+
echo "::error:: these recorded failures now PASS — remove them from h2spec_expected_failures.txt to keep the gate honest:" >&2
96121
echo "$now_passing" >&2
97-
# Treat as a (soft) failure so the baseline is tightened; flip to rc=1 once green.
122+
rc=1
98123
fi
99124
[ "$rc" -eq 0 ] && echo "==> h2spec: no regressions vs baseline"
100125
exit "$rc"

0 commit comments

Comments
 (0)