Skip to content

Commit d020371

Browse files
authored
Merge pull request #11 from Query-farm/multi-transport-sql-ci
ci: multi-transport SQL E2E (subprocess, http, unix)
2 parents 3701318 + 28c6ac0 commit d020371

4 files changed

Lines changed: 231 additions & 20 deletions

File tree

.github/workflows/ci.yml

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,19 @@ jobs:
6969
GH_TOKEN: ${{ github.token }}
7070

7171
integration:
72-
name: SQL end-to-end (DuckDB sqllogictest)
72+
# SQL end-to-end across every transport the vgi extension supports. The SAME
73+
# test/sql/*.test suite runs three ways — the only difference is the LOCATION
74+
# the .test files ATTACH (ci/run-integration.sh sets VGI_MASK_WORKER per
75+
# transport): subprocess = the stdio worker binary DuckDB spawns; http =
76+
# `mask-worker --http` (auto port, advertises PORT:<n>); unix = `mask-worker
77+
# --unix <sock>` (advertises UNIX:<sock>).
78+
name: SQL E2E (${{ matrix.transport }}) · ${{ matrix.os }}
7379
needs: resolve-haybarn
7480
strategy:
7581
fail-fast: false
7682
matrix:
83+
os: [ubuntu-latest, macos-latest]
84+
transport: [subprocess, http, unix]
7785
include:
7886
- { os: ubuntu-latest, asset: haybarn_unittest-linux-amd64.zip }
7987
- { os: macos-latest, asset: haybarn_unittest-osx-arm64.zip }
@@ -90,6 +98,10 @@ jobs:
9098
uses: mozilla-actions/sccache-action@v0.0.9
9199

92100
- name: Build worker (release)
101+
# The `http` transport needs the `vgi-rpc` `http` feature; the workspace
102+
# already pins `vgi-rpc = { features = ["macros", "http"] }` (and the SDK
103+
# `vgi` crate enables HTTP via that), so a plain release build serves all
104+
# three transports — no extra cargo features required here.
93105
run: cargo build --release --bin mask-worker
94106

95107
- name: Download haybarn-unittest
@@ -106,11 +118,15 @@ jobs:
106118
- name: Resolve runner + worker paths
107119
run: |
108120
# Absolute paths: run-integration.sh cd's into a staging dir before
109-
# invoking the runner, so relative paths would not resolve.
121+
# invoking the runner, so relative paths would not resolve. WORKER_BIN
122+
# is the compiled binary — used directly as the subprocess LOCATION and
123+
# launched in --http / --unix mode for those transports.
110124
UNITTEST="$PWD/$(find hb -name 'haybarn-unittest' -type f | head -1)"
111125
chmod +x "$UNITTEST"
112126
echo "HAYBARN_UNITTEST=$UNITTEST" >> "$GITHUB_ENV"
113-
echo "VGI_MASK_WORKER=$PWD/target/release/mask-worker" >> "$GITHUB_ENV"
127+
echo "WORKER_BIN=$PWD/target/release/mask-worker" >> "$GITHUB_ENV"
114128
115-
- name: Run extension integration suite
129+
- name: Run extension integration suite (${{ matrix.transport }})
116130
run: ci/run-integration.sh
131+
env:
132+
TRANSPORT: ${{ matrix.transport }}

ci/README.md

Lines changed: 63 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,46 @@ the Rust unit + integration tests, and this repo's sqllogictest suite
55
(`test/sql/*.test`) against the vgi-mask VGI worker through the **real DuckDB
66
`vgi` extension** on every push / PR.
77

8+
## Transport matrix
9+
10+
The integration suite runs over **every transport the vgi extension supports**.
11+
The exact same `test/sql/*.test` files run three ways; the only thing that
12+
changes is what LOCATION the `.test` files `ATTACH` (set by
13+
[`run-integration.sh`](run-integration.sh) from the `TRANSPORT` env var):
14+
15+
| `TRANSPORT` | `VGI_MASK_WORKER` (the ATTACH LOCATION) | how the worker is launched |
16+
|--------------|------------------------------------------|----------------------------|
17+
| `subprocess` | `…/target/release/mask-worker` | DuckDB spawns the stdio binary (default) |
18+
| `http` | `http://127.0.0.1:<port>` | `mask-worker --http` (auto port; prints `PORT:<n>` on stdout, which the script polls for) |
19+
| `unix` | `unix:///tmp/mask.<pid>.sock` | `mask-worker --unix <sock>` (prints `UNIX:<sock>` on stdout + creates the socket; the script waits for both) |
20+
21+
CI runs `transport: [subprocess, http, unix]` × `os: [ubuntu, macos]` as a
22+
matrix. Build the worker once with a plain `cargo build --release` — the
23+
workspace already pins `vgi-rpc = { features = ["macros", "http"] }`, so the one
24+
binary serves all three transports; **no extra cargo feature is needed**.
25+
26+
### The `http` leg needs DuckDB's `httpfs` extension
27+
28+
The vgi extension's **HTTP client** is built on DuckDB's `httpfs`. Over `http://`,
29+
`ATTACH` fails without it:
30+
31+
> `Binder Error: VGI HTTP transport requires the httpfs extension. Install it with: INSTALL httpfs; LOAD httpfs;`
32+
33+
Crucially, that message contains the substring **`HTTP`**, and DuckDB's
34+
sqllogictest runner ships a default `ignore_error_messages = {"HTTP", "Unable to
35+
connect"}` that **silently SKIPs** any test whose error matches — so a missing
36+
`httpfs` looks like a (deceptive) pass-by-skip, not a failure. We handle this in
37+
two places:
38+
39+
1. [`preprocess-require.awk`](preprocess-require.awk), invoked with
40+
`-v transport=http`, injects a signed `INSTALL httpfs FROM core; LOAD httpfs;`
41+
right after each `LOAD vgi;` so the http leg actually loads the client and runs.
42+
2. [`run-integration.sh`](run-integration.sh) fails the job if the runner reports
43+
*any* skipped tests (a skip is never a pass) — guarding against this and any
44+
future silent-skip masking.
45+
46+
The `unix` (AF_UNIX launcher) leg needs no extra extension.
47+
848
## How it works (no C++ build)
949

1050
Rather than building the vgi DuckDB extension from source, the integration job
@@ -21,23 +61,37 @@ sqllogictest runner, published in Haybarn's releases) and installs the
2161
tests gate on, so [`preprocess-require.awk`](preprocess-require.awk) rewrites
2262
each `require <ext>` into an explicit signed `INSTALL <ext> FROM
2363
{community,core}; LOAD <ext>;`. `require-env` and everything else pass
24-
through untouched.
25-
4. **Run**[`run-integration.sh`](run-integration.sh) stages the preprocessed
26-
tree, points `VGI_MASK_WORKER` at the release binary, warms the extension
27-
cache once (`INSTALL vgi FROM community;` — this is what makes the tests'
28-
explicit `LOAD vgi;` succeed), then runs the suite in a single
29-
`haybarn-unittest` invocation. Any failed assertion exits non-zero and fails
30-
the job.
64+
through untouched. (The vgi-mask `.test` files already `LOAD vgi;`
65+
explicitly and use `require-env VGI_MASK_WORKER`, so the `require`-rewrite is
66+
a no-op here.) When run with `-v transport=http`, the awk also injects
67+
`INSTALL httpfs FROM core; LOAD httpfs;` after each `LOAD vgi;` (see the
68+
transport-matrix section above for why).
69+
4. **Run**[`run-integration.sh`](run-integration.sh) brings up the worker for
70+
the selected `TRANSPORT` and sets `VGI_MASK_WORKER` accordingly, stages the
71+
preprocessed tree, warms the extension cache once (`INSTALL vgi FROM
72+
community;` — this is what makes the tests' explicit `LOAD vgi;` succeed),
73+
then runs the suite in a single `haybarn-unittest` invocation. Any failed
74+
assertion — or any skipped test — exits non-zero and fails the job.
3175

3276
## Run it locally
3377

3478
```bash
3579
cargo build --release --bin mask-worker
3680
# point HAYBARN_UNITTEST at a haybarn-unittest binary (or a local DuckDB
37-
# `unittest` built with the vgi extension), and the worker at the release binary:
81+
# `unittest` built with the vgi extension), and WORKER_BIN at the release binary.
82+
# TRANSPORT defaults to subprocess; set it to http or unix for the other legs.
3883
HAYBARN_UNITTEST=/path/to/haybarn-unittest \
39-
VGI_MASK_WORKER="$PWD/target/release/mask-worker" \
84+
WORKER_BIN="$PWD/target/release/mask-worker" \
85+
TRANSPORT=subprocess \
4086
ci/run-integration.sh
87+
88+
# HTTP leg: launches `mask-worker --http`, reads PORT:<n>, attaches http://…
89+
HAYBARN_UNITTEST=/path/to/haybarn-unittest WORKER_BIN="$PWD/target/release/mask-worker" \
90+
TRANSPORT=http ci/run-integration.sh
91+
92+
# Unix leg: launches `mask-worker --unix /tmp/mask.<pid>.sock`, attaches unix://…
93+
HAYBARN_UNITTEST=/path/to/haybarn-unittest WORKER_BIN="$PWD/target/release/mask-worker" \
94+
TRANSPORT=unix ci/run-integration.sh
4195
```
4296

4397
Or use the Makefile target `make test-sql`, which builds the release worker and

ci/preprocess-require.awk

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,36 @@
66
# comes from the signed community channel; httpfs/json/parquet/spatial from the
77
# signed core channel. `require-env` and every other directive pass through
88
# untouched. See ci/README.md.
9+
#
10+
# When invoked with `-v transport=http`, the http:// LOCATION exercised by that
11+
# transport requires DuckDB's `httpfs` extension (the vgi extension's HTTP
12+
# client is built on it; without it ATTACH fails with a Binder Error whose text
13+
# contains "HTTP" — which DuckDB's sqllogictest runner silently *auto-skips*
14+
# via its default `ignore_error_messages = {"HTTP", ...}`, so a missing httpfs
15+
# looks like a pass-by-skip rather than a failure). We therefore inject a signed
16+
# `INSTALL httpfs FROM core; LOAD httpfs;` right after the test's own
17+
# `LOAD vgi;` so the http leg actually runs (and is not silently skipped).
18+
# Harmless/absent for subprocess and unix (transport != http).
919
/^require[ \t]+vgi[ \t]*$/ {
1020
print "statement ok"; print "INSTALL vgi FROM community;"; print "";
11-
print "statement ok"; print "LOAD vgi;"; next
21+
print "statement ok"; print "LOAD vgi;";
22+
if (transport == "http") {
23+
print "";
24+
print "statement ok"; print "INSTALL httpfs FROM core;"; print "";
25+
print "statement ok"; print "LOAD httpfs;";
26+
}
27+
next
28+
}
29+
# The .test files LOAD vgi explicitly (statement ok + LOAD vgi;) rather than via
30+
# `require vgi`; hook that line too so the http leg gets httpfs.
31+
/^LOAD[ \t]+vgi[ \t]*;[ \t]*$/ {
32+
print
33+
if (transport == "http") {
34+
print "";
35+
print "statement ok"; print "INSTALL httpfs FROM core;"; print "";
36+
print "statement ok"; print "LOAD httpfs;";
37+
}
38+
next
1239
}
1340
/^require[ \t]+(httpfs|json|parquet|spatial)[ \t]*$/ {
1441
ext = $2

ci/run-integration.sh

Lines changed: 120 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,25 +5,122 @@
55
# VGI worker, using a prebuilt standalone `haybarn-unittest` and the signed
66
# community `vgi` extension — no C++ build from source. See ci/README.md.
77
#
8+
# Parameterized by TRANSPORT (default: subprocess), exercising the SAME suite
9+
# over each transport the vgi extension supports — the only thing that changes
10+
# is what LOCATION (VGI_MASK_WORKER) the .test files ATTACH:
11+
#
12+
# subprocess VGI_MASK_WORKER = the stdio worker command (DuckDB spawns it).
13+
# http start `mask-worker --http` (auto port; advertises `PORT:<n>`
14+
# on stdout), VGI_MASK_WORKER = http://127.0.0.1:<port>.
15+
# unix start `mask-worker --unix <sock>` (advertises `UNIX:<sock>`
16+
# on stdout), VGI_MASK_WORKER = unix://<sock>.
17+
#
818
# Required environment:
919
# HAYBARN_UNITTEST path to the haybarn-unittest binary
10-
# VGI_MASK_WORKER worker LOCATION the .test files attach (a stdio command
11-
# such as the compiled mask-worker binary, or an http:// URL)
20+
# WORKER_BIN path to the compiled mask-worker binary (used to launch
21+
# the http/unix servers, and the stdio LOCATION). Falls back
22+
# to VGI_MASK_WORKER when that is a bare command (subprocess).
1223
# Optional:
24+
# TRANSPORT subprocess | http | unix (default: subprocess)
1325
# STAGE scratch dir for the preprocessed test tree (default: mktemp)
1426
set -euo pipefail
1527

28+
TRANSPORT="${TRANSPORT:-subprocess}"
29+
1630
: "${HAYBARN_UNITTEST:?path to the haybarn-unittest binary}"
17-
: "${VGI_MASK_WORKER:?worker LOCATION (stdio command or http:// URL)}"
1831

1932
HERE="$(cd "$(dirname "$0")" && pwd)"
2033
REPO="$(cd "$HERE/.." && pwd)"
2134
STAGE="${STAGE:-$(mktemp -d)}"
2235

36+
# For http/unix we must launch the worker binary ourselves; for subprocess the
37+
# binary IS the LOCATION. WORKER_BIN names the compiled binary; default to the
38+
# release build in this repo.
39+
WORKER_BIN="${WORKER_BIN:-$REPO/target/release/mask-worker}"
40+
41+
SERVER_PID=""
42+
SOCK_PATH=""
43+
cleanup() {
44+
local rc=$?
45+
if [[ -n "$SERVER_PID" ]]; then
46+
kill "$SERVER_PID" 2>/dev/null || true
47+
wait "$SERVER_PID" 2>/dev/null || true
48+
fi
49+
[[ -n "$SOCK_PATH" ]] && rm -f "$SOCK_PATH" 2>/dev/null || true
50+
return "$rc"
51+
}
52+
trap cleanup EXIT
53+
54+
# Bring up the out-of-band server for http/unix and resolve VGI_MASK_WORKER.
55+
# Both transports announce their endpoint on stdout (`PORT:<n>` / `UNIX:<path>`),
56+
# which we poll for in the log before running the suite (readiness gate).
57+
start_server_and_set_location() {
58+
local kind="$1"
59+
: "${WORKER_BIN:?path to the mask-worker binary (WORKER_BIN)}"
60+
[[ -x "$WORKER_BIN" ]] || { echo "ERROR: worker binary not executable: $WORKER_BIN" >&2; exit 1; }
61+
62+
local log="$STAGE/.worker-$kind.log"
63+
case "$kind" in
64+
http)
65+
"$WORKER_BIN" --http >"$log" 2>&1 &
66+
SERVER_PID=$!
67+
local port=""
68+
for _ in $(seq 1 60); do
69+
if ! kill -0 "$SERVER_PID" 2>/dev/null; then
70+
echo "ERROR: worker (--http) exited during startup. Log:" >&2; cat "$log" >&2; exit 1
71+
fi
72+
port=$(sed -n 's/.*PORT:\([0-9][0-9]*\).*/\1/p' "$log" 2>/dev/null | head -1)
73+
[[ -n "$port" ]] && break
74+
sleep 0.5
75+
done
76+
[[ -n "$port" ]] || { echo "ERROR: timed out waiting for PORT:<n>. Log:" >&2; cat "$log" >&2; exit 1; }
77+
export VGI_MASK_WORKER="http://127.0.0.1:$port"
78+
echo "HTTP worker ready on 127.0.0.1:$port (pid $SERVER_PID)"
79+
;;
80+
unix)
81+
SOCK_PATH="${VGI_MASK_SOCK:-/tmp/mask.$$.sock}"
82+
rm -f "$SOCK_PATH" 2>/dev/null || true
83+
"$WORKER_BIN" --unix "$SOCK_PATH" >"$log" 2>&1 &
84+
SERVER_PID=$!
85+
local ready=""
86+
for _ in $(seq 1 60); do
87+
if ! kill -0 "$SERVER_PID" 2>/dev/null; then
88+
echo "ERROR: worker (--unix) exited during startup. Log:" >&2; cat "$log" >&2; exit 1
89+
fi
90+
if grep -q "UNIX:$SOCK_PATH" "$log" 2>/dev/null && [[ -S "$SOCK_PATH" ]]; then
91+
ready=1; break
92+
fi
93+
sleep 0.5
94+
done
95+
[[ -n "$ready" ]] || { echo "ERROR: timed out waiting for UNIX socket. Log:" >&2; cat "$log" >&2; exit 1; }
96+
export VGI_MASK_WORKER="unix://$SOCK_PATH"
97+
echo "Unix worker ready on $SOCK_PATH (pid $SERVER_PID)"
98+
;;
99+
esac
100+
}
101+
102+
case "$TRANSPORT" in
103+
subprocess)
104+
# The binary itself is the stdio LOCATION DuckDB spawns. Honor an explicit
105+
# VGI_MASK_WORKER (e.g. a bare command) if the caller set one.
106+
export VGI_MASK_WORKER="${VGI_MASK_WORKER:-$WORKER_BIN}"
107+
;;
108+
http) start_server_and_set_location http ;;
109+
unix) start_server_and_set_location unix ;;
110+
*) echo "ERROR: unknown TRANSPORT '$TRANSPORT' (want subprocess|http|unix)" >&2; exit 1 ;;
111+
esac
112+
113+
: "${VGI_MASK_WORKER:?worker LOCATION (stdio command, http:// URL, or unix:// socket)}"
114+
23115
echo "Staging preprocessed tests into $STAGE ..."
24116
mkdir -p "$STAGE/test/sql"
117+
# Pass the transport to the preprocessor: the http leg additionally needs DuckDB's
118+
# `httpfs` extension loaded (the vgi extension's HTTP client is built on it), so
119+
# the awk injects a signed INSTALL/LOAD httpfs after each `LOAD vgi;`. Without it
120+
# the http ATTACH fails with a "HTTP"-containing error that the sqllogictest
121+
# runner *silently auto-skips* (default ignore_error_messages), masking the gap.
25122
for f in "$REPO"/test/sql/*.test; do
26-
awk -f "$HERE/preprocess-require.awk" "$f" > "$STAGE/test/sql/$(basename "$f")"
123+
awk -v transport="$TRANSPORT" -f "$HERE/preprocess-require.awk" "$f" > "$STAGE/test/sql/$(basename "$f")"
27124
done
28125

29126
cd "$STAGE"
@@ -45,5 +142,22 @@ rm -f "$STAGE/test/_warm.test"
45142

46143
# Run the whole suite in one invocation, streaming the runner's native
47144
# sqllogictest report. Any failed assertion exits non-zero and fails the job.
48-
echo "Running suite (worker: $VGI_MASK_WORKER) ..."
49-
"$HAYBARN_UNITTEST" "test/sql/*"
145+
#
146+
# Guard against the silent-skip trap: DuckDB's sqllogictest runner auto-skips
147+
# any test whose error message contains "HTTP" (default ignore_error_messages),
148+
# so a broken http leg can report "All tests were skipped" with exit 0 and look
149+
# green. Tee the report and fail if NOTHING actually ran. (For subprocess/unix
150+
# there is no skip path, so this only ever bites a genuinely broken http leg.)
151+
echo "Running suite (transport: $TRANSPORT, worker: $VGI_MASK_WORKER) ..."
152+
REPORT="$STAGE/.report.txt"
153+
set +e
154+
"$HAYBARN_UNITTEST" "test/sql/*" 2>&1 | tee "$REPORT"
155+
status="${PIPESTATUS[0]}"
156+
set -e
157+
if grep -qiE "All tests were skipped|total skipped [1-9]" "$REPORT"; then
158+
echo "ERROR: tests were SKIPPED — almost certainly an ATTACH/transport error whose" >&2
159+
echo " message matched the runner's default ignore list (e.g. \"HTTP\"). A skip" >&2
160+
echo " is NOT a pass. Transport=$TRANSPORT worker=$VGI_MASK_WORKER." >&2
161+
exit 1
162+
fi
163+
exit "$status"

0 commit comments

Comments
 (0)