From 20a9c12f8631e982dafd6e674e71d94b99a1e227 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Mon, 24 Aug 2026 14:31:21 +0800 Subject: [PATCH] test(e2e): fail closed on runner readiness (#6490) --- scripts/check_test_wiring.py | 47 +++++++++++++++++++++++++++ scripts/run_e2e_tests.sh | 61 +++++------------------------------- 2 files changed, 55 insertions(+), 53 deletions(-) diff --git a/scripts/check_test_wiring.py b/scripts/check_test_wiring.py index 8ca8239dd..4f652a14c 100755 --- a/scripts/check_test_wiring.py +++ b/scripts/check_test_wiring.py @@ -209,6 +209,20 @@ def check_runner_selection(root: Path) -> list[str]: errors.append("scripts/run_e2e_tests.sh: --test is documented as a pattern and must not force exact matching") if 'eval "$test_cmd"' in runner: errors.append("scripts/run_e2e_tests.sh: command construction must not use eval") + start = re.search(r"start_rustfs\(\) \{(?P.*?)\n\}\n\n# Function to run tests", runner, re.DOTALL) + start_body = start.group("body") if start else "" + if '"http://localhost:9000/health/ready"' not in start_body or not re.search( + r"curl [^\n]*(?:--fail|-f(?:\s|$))", start_body + ): + errors.append("scripts/run_e2e_tests.sh: startup must require the ready endpoint to return HTTP success") + if start_body.count("return 0") != 1 or "nc -z" in start_body: + errors.append("scripts/run_e2e_tests.sh: startup readiness must not fall back to process or port liveness") + failed_start = re.search(r"if ! start_rustfs; then(?P.*?)\n\s*fi", runner, re.DOTALL) + failed_start_commands = ( + [line.strip() for line in failed_start.group("body").splitlines() if line.strip()] if failed_start else [] + ) + if failed_start_commands != ['print_error "Failed to start RustFS properly"', "exit 1"]: + errors.append("scripts/run_e2e_tests.sh: failed startup must not continue into tests") return errors @@ -599,6 +613,39 @@ def validate(root: Path) -> list[str]: class SelfTests(unittest.TestCase): + def test_runner_readiness_fails_closed(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + script = root / "scripts/run_e2e_tests.sh" + script.parent.mkdir(parents=True) + valid_runner = ( + "start_rustfs() {\n" + ' curl --fail "http://localhost:9000/health/ready" && return 0\n' + " return 1\n" + "}\n\n# Function to run tests\n" + "--include-ignored --test-threads=1\n" + "if ! start_rustfs; then\n" + ' print_error "Failed to start RustFS properly"\n' + " exit 1\n" + "fi\n" + ) + script.write_text(valid_runner) + self.assertEqual(check_runner_selection(root), []) + + script.write_text(valid_runner.replace("/health/ready", "/health")) + self.assertEqual(len(check_runner_selection(root)), 1) + + script.write_text(valid_runner.replace("return 1", "nc -z localhost 9000 && return 0")) + self.assertEqual(len(check_runner_selection(root)), 1) + + script.write_text( + valid_runner.replace( + ' print_error "Failed to start RustFS properly"\n exit 1', + ' if [ -n "$RUSTFS_PID" ]; then\n echo continuing\n else\n exit 1\n fi', + ) + ) + self.assertEqual(len(check_runner_selection(root)), 1) + def test_e2e_requires_registration(self) -> None: with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) diff --git a/scripts/run_e2e_tests.sh b/scripts/run_e2e_tests.sh index 8e98b202c..7a1a470eb 100755 --- a/scripts/run_e2e_tests.sh +++ b/scripts/run_e2e_tests.sh @@ -148,7 +148,7 @@ start_rustfs() { # Wait for RustFS to be ready print_info "Waiting for RustFS to be ready..." - local max_attempts=15 # Reduced from 30 to 15 seconds + local max_attempts=60 local attempt=0 while [ $attempt -lt $max_attempts ]; do @@ -160,61 +160,22 @@ start_rustfs() { exit 1 fi - # Try simple HTTP connection first (most reliable) - if curl -s --noproxy localhost --connect-timeout 2 --max-time 3 "http://localhost:9000/" >/dev/null 2>&1; then + if curl --silent --show-error --fail --noproxy localhost --connect-timeout 2 --max-time 3 \ + "http://localhost:9000/health/ready" >/dev/null 2>&1; then print_success "RustFS is ready!" return 0 fi - # Try health endpoint if available - if curl -s --noproxy localhost --connect-timeout 2 --max-time 3 "http://localhost:9000/health" >/dev/null 2>&1; then - print_success "RustFS is ready!" - return 0 - fi - - # Try port connectivity check (faster than HTTP) - if nc -z localhost 9000 2>/dev/null; then - print_info "Port 9000 is open, verifying HTTP response..." - if curl -s --noproxy localhost --connect-timeout 1 --max-time 2 "http://localhost:9000/" >/dev/null 2>&1; then - print_success "RustFS is ready!" - return 0 - fi - fi - sleep 1 attempt=$((attempt + 1)) echo -n "." done echo - print_warning "RustFS health check failed within $max_attempts seconds" - print_info "Checking if RustFS process is still running..." - if kill -0 "$RUSTFS_PID" 2>/dev/null; then - print_info "RustFS process is still running (PID: $RUSTFS_PID)" - print_info "Trying final connection attempts..." - - # Quick final attempts with shorter timeouts - for i in 1 2 3; do - if curl -s --noproxy localhost --connect-timeout 1 --max-time 2 "http://localhost:9000/" >/dev/null 2>&1; then - print_success "RustFS is now ready!" - return 0 - fi - if nc -z localhost 9000 2>/dev/null; then - print_info "Port 9000 is accessible, continuing with tests..." - return 0 - fi - sleep 1 - done - - print_warning "RustFS may be slow to respond, but process is running" - print_info "Continuing with tests anyway..." - return 0 - else - print_error "RustFS process has died" - print_error "Log output:" - cat "$TARGET_DIR/rustfs.log" || true - return 1 - fi + print_error "RustFS readiness check failed within $max_attempts seconds" + print_error "Log output:" + cat "$TARGET_DIR/rustfs.log" || true + return 1 } # Function to run tests @@ -295,13 +256,7 @@ main() { # Start RustFS if ! start_rustfs; then print_error "Failed to start RustFS properly" - print_info "Checking if we can still run tests..." - if [ ! -z "$RUSTFS_PID" ] && kill -0 "$RUSTFS_PID" 2>/dev/null; then - print_info "RustFS process is still running, attempting to continue..." - else - print_error "RustFS is not running, cannot proceed with tests" - exit 1 - fi + exit 1 fi # Run tests