This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-6336-181e38b4a0b8bfa49fdef5b4fa2d43eed2d79e1d in repository https://gitbox.apache.org/repos/asf/texera.git
commit fc07d1e4fc5b310e81c652b1532165e67190063c Author: Meng Wang <[email protected]> AuthorDate: Wed Jul 15 01:19:35 2026 -0700 fix(ci): stop smoke-boot.sh false-positive on jOOQ "tip of the day" (#6336) ### What changes were proposed in this PR? The boot smoke check (`.github/scripts/smoke-boot.sh`, from #6220) decided a service had crashed on boot by grepping its whole log for exception class names (`NoClassDefFoundError`, `ClassNotFoundException`, …). jOOQ prints a random **"tip of the day"** banner naming those exceptions *in prose*, so whenever it drew that tip the check reported a crash **even though the service booted fine** — a random failure on any jOOQ-using service, on any PR (e.g. `config-service` on #6331). Closes #6332. - **Decide from the process, not the log.** `smoke-boot.sh` now bases its verdict on process behaviour: reaching LISTEN is a healthy boot; **exiting before LISTEN is a crash** (it reports the process's exit code); neither within the timeout is a hang. This is what actually distinguishes a boot crash and can't be fooled by log text — no more grepping for exception-name strings. - **Rework the regression test end-to-end.** `.github/scripts/test_smoke_boot.sh` drives `smoke-boot.sh` with fake launchers: a healthy service that logs the jOOQ tip must pass, a crasher must fail, a hang must time out. The `infra` job's shell-test discovery is broadened to `.github/scripts/` so it runs. ### Any related issues, documentation, discussions? Closes #6332. Boot check from #6220; the false positive surfaced through the `platform-integration` boot checks (#6274). Per review feedback, the fragile log scan is replaced by a process-exit check. ### How was this PR tested? - `.github/scripts/test_smoke_boot.sh` passes locally and is deterministic across repeated runs: a healthy service that prints the jOOQ tip → OK; a service that exits on boot → FAIL (with its exit code); a service that never listens → FAIL (timeout). - Confirmed `smoke-boot.sh` reports the crash's exit code, e.g. `exited on boot (exit code 3) before listening on :<port>`. - The `infra` job discovers the test via `find bin .github/scripts -name 'test_*.sh'`; `smoke-boot.sh` keeps its executable bit. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (claude-opus-4-8) --------- Signed-off-by: Meng Wang <[email protected]> Co-authored-by: Copilot Autofix powered by AI <[email protected]> --- .github/scripts/smoke-boot.sh | 116 ++++++++++++++++++++++++------------- .github/scripts/test_smoke_boot.sh | 114 ++++++++++++++++++++++++++++++++++++ .github/workflows/build.yml | 8 +-- 3 files changed, 194 insertions(+), 44 deletions(-) diff --git a/.github/scripts/smoke-boot.sh b/.github/scripts/smoke-boot.sh index 1866a38483..9b98c67a79 100755 --- a/.github/scripts/smoke-boot.sh +++ b/.github/scripts/smoke-boot.sh @@ -17,11 +17,20 @@ # under the License. # smoke-boot.sh -- launch a packaged Texera service from its unpacked dist and -# assert it boots (reaches a listening port) without a runtime classpath/linkage -# failure (NoClassDefFoundError, LinkageError, Jackson/Scala-module version -# conflict). That class of bug compiles and unit-tests clean but crashes the real -# `main`, so it slips through CI, which otherwise only builds + unit-tests each -# service and never starts it. See https://github.com/apache/texera/issues/6220. +# assert it boots (reaches a listening port) instead of crashing on startup with +# a runtime classpath/linkage failure (NoClassDefFoundError, LinkageError, +# Jackson/Scala-module version conflict, ...). That class of bug compiles and +# unit-tests clean but crashes the real `main`, so it slips through CI, which +# otherwise only builds + unit-tests each service and never starts it. See +# https://github.com/apache/texera/issues/6220. +# +# The verdict is based on the process's own behaviour, not on scanning its logs: +# * reaches LISTEN -> booted OK +# * exits before LISTEN -> crashed on boot (report its exit code) +# * neither, within timeout -> hung / failed to come up +# Scanning stdout/stderr for exception names was fragile -- any library that +# merely prints an exception name in prose (e.g. jOOQ's random "tip of the day") +# tripped it. See https://github.com/apache/texera/issues/6332. # # Usage: # smoke-boot.sh <launcher-glob> <port> [timeout_secs] @@ -43,30 +52,55 @@ launcher_glob="${1:?usage: smoke-boot.sh <launcher-glob> <port> [timeout]}" port="${2:?port required}" timeout="${3:-60}" -# Resolve the (possibly globbed) launcher to a concrete executable. -launcher="$(ls $launcher_glob 2>/dev/null | head -n1 || true)" -if [[ -z "$launcher" || ! -x "$launcher" ]]; then - echo "::error::smoke-boot: launcher not found or not executable: $launcher_glob" +# Resolve the (possibly globbed) launcher to exactly one concrete executable. +# Erroring on multiple matches avoids silently smoke-testing the wrong (e.g. +# lexicographically-first) binary if a dist dir ever accumulates versions. +matches="$(ls -d $launcher_glob 2>/dev/null || true)" +count="$(printf '%s' "$matches" | grep -c . || true)" +if [[ "$count" -eq 0 ]]; then + echo "::error::smoke-boot: launcher not found: $launcher_glob" + exit 1 +fi +if [[ "$count" -gt 1 ]]; then + echo "::error::smoke-boot: launcher glob matched $count files, expected exactly 1: $launcher_glob" + exit 1 +fi +launcher="$matches" +if [[ ! -x "$launcher" ]]; then + echo "::error::smoke-boot: launcher not executable: $launcher" exit 1 fi - -log="$(mktemp)" -echo "smoke-boot: launching '$launcher' (port=$port timeout=${timeout}s)" -"$launcher" >"$log" 2>&1 & -pid=$! - -# Runtime classpath / linkage / module failures -- the class of regression this -# check exists to catch. -crash_re='NoClassDefFoundError|ClassNotFoundException|LinkageError|NoSuchMethodError|AbstractMethodError|ExceptionInInitializerError|IncompatibleClassChangeError|requires Jackson Databind' port_open() { + # Probe 127.0.0.1 explicitly (not "localhost", which can resolve to ::1 first + # while the JVM binds IPv4 0.0.0.0, giving a false "not listening"). if command -v nc >/dev/null 2>&1; then - nc -z localhost "$port" >/dev/null 2>&1 + nc -z 127.0.0.1 "$port" >/dev/null 2>&1 else - (exec 3<>"/dev/tcp/localhost/$port") 2>/dev/null + (exec 3<>"/dev/tcp/127.0.0.1/$port") 2>/dev/null fi } +# Fail fast if the port is already taken. The wait loop below treats "something +# is LISTENing on :port" as a healthy boot, so a leftover/unrelated listener +# could otherwise mask a crashed service. Requiring the port to be free at launch +# means a listener detected afterwards is the service we started. +if port_open; then + echo "::error::smoke-boot: port $port is already in use before launching '$launcher'" + exit 1 +fi + +log="$(mktemp)" +echo "smoke-boot: launching '$launcher' (port=$port timeout=${timeout}s)" +"$launcher" >"$log" 2>&1 & +pid=$! + +# On any exit -- including an unexpected abort under `set -e` -- stop the service +# and remove the log, so a failing script can't orphan the JVM or leak temp files. +trap 'kill "$pid" 2>/dev/null || true; rm -f "$log"' EXIT + +# Wait for the service to reach one of three terminal states: it opens its port +# (booted), it exits on its own (crashed), or neither happens in time (hung). outcome="timeout" for ((i = 0; i < timeout; i++)); do if port_open; then outcome="listen"; break; fi @@ -74,21 +108,6 @@ for ((i = 0; i < timeout; i++)); do sleep 1 done -# Stop the service (it may already be gone). SIGTERM, then a bounded grace -# period, then SIGKILL -- so a service that ignores SIGTERM or hangs in shutdown -# can't leave the CI step running indefinitely. -kill "$pid" 2>/dev/null || true -for _ in $(seq 1 10); do - if ! kill -0 "$pid" 2>/dev/null; then - break - fi - sleep 1 -done -if kill -0 "$pid" 2>/dev/null; then - kill -9 "$pid" 2>/dev/null || true -fi -wait "$pid" 2>/dev/null || true - fail() { echo "::error::smoke-boot: $*" echo "----- last 80 log lines from '$launcher' -----" @@ -96,19 +115,36 @@ fail() { exit 1 } -# A linkage/module error on boot fails regardless of whether the port came up. -if grep -qE "$crash_re" "$log"; then - fail "'$launcher' hit a runtime classpath/linkage error on boot" -fi +# Stop a still-running service. SIGTERM, then a bounded grace period, then +# SIGKILL -- so a service that ignores SIGTERM or hangs in shutdown can't leave +# the CI step running indefinitely. +stop_service() { + kill "$pid" 2>/dev/null || true + for _ in $(seq 1 10); do + if ! kill -0 "$pid" 2>/dev/null; then + wait "$pid" 2>/dev/null || true + return 0 + fi + sleep 1 + done + kill -9 "$pid" 2>/dev/null || true + wait "$pid" 2>/dev/null || true +} case "$outcome" in listen) + stop_service echo "smoke-boot: OK -- '$launcher' reached LISTEN on :$port" ;; exited) - fail "'$launcher' exited before listening on :$port" + # The service died before it ever listened -- a boot crash. Its own exit + # code is the signal; reap it (the process has already exited) and report it. + code=0 + wait "$pid" 2>/dev/null || code=$? + fail "'$launcher' exited on boot (exit code $code) before listening on :$port" ;; *) + stop_service fail "'$launcher' did not listen on :$port within ${timeout}s" ;; esac diff --git a/.github/scripts/test_smoke_boot.sh b/.github/scripts/test_smoke_boot.sh new file mode 100755 index 0000000000..265a26e04c --- /dev/null +++ b/.github/scripts/test_smoke_boot.sh @@ -0,0 +1,114 @@ +#!/usr/bin/env bash +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# End-to-end regression tests for smoke-boot.sh. Each case launches a fake +# "service" through smoke-boot.sh and checks its verdict from the exit code -- +# smoke-boot no longer scans logs, so its decision is driven purely by whether +# the process reaches LISTEN, exits, or hangs. +# +# Guards issue #6332: a service that boots fine but prints exception-name prose +# in its log (e.g. jOOQ's random "tip of the day") must PASS; and a service that +# crashes on boot must still FAIL. + +set -uo pipefail + +command -v python3 >/dev/null || { echo "python3 is required to run these tests" >&2; exit 1; } + +script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +smoke="$script_dir/smoke-boot.sh" +work="$(mktemp -d 2>/dev/null || mktemp -d -t smoke-boot)" +trap 'rm -rf "$work"' EXIT +rc=0 + +# Print a likely-free localhost TCP port. +free_port() { + python3 -c 'import socket; s=socket.socket(); s.bind(("127.0.0.1",0)); print(s.getsockname()[1]); s.close()' +} + +pass() { echo "ok: $1"; } +failed() { echo "FAIL: $1"; rc=1; } + +# --- #6332: a healthy service that logs the jOOQ tip must PASS --- +# It prints prose naming NoClassDefFoundError / ClassNotFoundException, then +# opens its port and stays up. The old log-scanning check flagged this as a +# crash; the process-based check must not. +port="$(free_port)" +cat >"$work/healthy" <<EOF +#!/usr/bin/env bash +echo "jOOQ tip of the day: A NoClassDefFoundError or ClassNotFoundException is often a sign of a version mismatch" +exec python3 -c "import socket, time; s = socket.socket(); s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1); s.bind(('127.0.0.1', $port)); s.listen(); time.sleep(30)" +EOF +chmod +x "$work/healthy" +if "$smoke" "$work/healthy" "$port" 15 >/dev/null 2>&1; then + pass "healthy boot that logs the jOOQ tip -> OK (#6332)" +else + failed "healthy boot that logs the jOOQ tip should be OK" +fi + +# --- a service that crashes on boot (exits before listening) must FAIL --- +port="$(free_port)" +cat >"$work/crasher" <<EOF +#!/usr/bin/env bash +echo "startup failed" >&2 +exit 1 +EOF +chmod +x "$work/crasher" +if "$smoke" "$work/crasher" "$port" 15 >/dev/null 2>&1; then + failed "crash on boot should FAIL" +else + pass "crash on boot (exits before listening) -> FAIL" +fi + +# --- a service that hangs without ever listening must FAIL (timeout) --- +port="$(free_port)" +cat >"$work/hang" <<EOF +#!/usr/bin/env bash +exec sleep 300 +EOF +chmod +x "$work/hang" +if "$smoke" "$work/hang" "$port" 3 >/dev/null 2>&1; then + failed "hang without listening should FAIL" +else + pass "hang without listening -> FAIL (timeout)" +fi + +# --- #6336: a crash must not be masked by a pre-existing listener on the port. +# Hold the port with an unrelated listener, then boot a crasher on it: the port +# is busy at launch, so smoke-boot must FAIL fast rather than mistake the +# squatter's LISTEN for a healthy boot. --- +port="$(free_port)" +python3 -c "import socket, time; s = socket.socket(); s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1); s.bind(('127.0.0.1', $port)); s.listen(); time.sleep(60)" & +squatter=$! +# Wait until the squatter is actually accepting connections before testing. +for _ in $(seq 1 50); do + python3 -c "import socket,sys; s=socket.socket(); s.settimeout(0.2); sys.exit(0 if s.connect_ex(('127.0.0.1',$port))==0 else 1)" && break + sleep 0.1 +done +if "$smoke" "$work/crasher" "$port" 5 >/dev/null 2>&1; then + failed "a crash masked by a pre-existing listener should FAIL (#6336)" +else + pass "port already in use -> FAIL fast, crash not masked (#6336)" +fi +kill "$squatter" 2>/dev/null || true +wait "$squatter" 2>/dev/null || true + +if [[ "$rc" -ne 0 ]]; then + echo "smoke-boot regression tests FAILED" + exit 1 +fi +echo "smoke-boot regression tests passed" diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 67943b907e..2e58d5f2ff 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -1131,8 +1131,8 @@ jobs: python -m pip install --upgrade pip python -m pip install -r amber/dev-requirements.txt - name: Run shell smoke tests - # Discover + run every `test_*.sh` under bin/. New shell test - # suites picked up automatically — no edits to this workflow. + # Discover + run every `test_*.sh` under bin/ and .github/scripts/. New + # shell test suites picked up automatically — no edits to this workflow. # `find -print0` instead of bash's `**` glob so this works on # macOS's pinned /bin/bash 3.2 (no `shopt -s globstar`). run: | @@ -1142,9 +1142,9 @@ jobs: found=1 echo "==> $t" bash "$t" - done < <(find bin -type f -name "test_*.sh" -print0) + done < <(find bin .github/scripts -type f -name "test_*.sh" -print0) if [ "$found" = 0 ]; then - echo "no shell tests found under bin/" >&2 + echo "no shell tests found under bin/ or .github/scripts/" >&2 exit 1 fi - name: Run Python unit tests
