Skip to content

Stop threads started by test when each test completes#73

Closed
esnible wants to merge 4 commits intopraxis-proxy:mainfrom
esnible:stop-test-threads-part2
Closed

Stop threads started by test when each test completes#73
esnible wants to merge 4 commits intopraxis-proxy:mainfrom
esnible:stop-test-threads-part2

Conversation

@esnible
Copy link
Copy Markdown
Member

@esnible esnible commented Apr 24, 2026

This PR completes the work that CoPilot requested at #64 (comment)

The rationale is that new tests will follow the example of old tests -- thus we don't want any tests that leak resources.

I expected this code to run perfectly on the Mac. It did earlier in the week. Today three throughput tests fail

  • throughput_filter_chain::bench_pipeline_4_filters
    • throughput 390 req/s below minimum 500 req/s
  • throughput_simple::bench_simple_proxy_concurrent
    • throughput 392 req/s below minimum 500 req/s
  • throughput_tcp::bench_tcp_proxy_concurrent
    • throughput 412 req/s below minimum 500 req/s

These are not regressions caused by this PR. Probably the problem is related to other things running today on my Mac.

@shaneutt We may wish to make the throughput threshold configurable in a follow-up.

Regarding the size, this code is mostly the same refactor. I also ran cargo +nightly fmt --all.

esnible added 3 commits April 24, 2026 11:01
Signed-off-by: Ed Snible <snible@us.ibm.com>
Signed-off-by: Ed Snible <snible@us.ibm.com>
Signed-off-by: Ed Snible <snible@us.ibm.com>
@esnible esnible marked this pull request as ready for review April 24, 2026 15:29
Signed-off-by: Ed Snible <snible@us.ibm.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors the Rust test suite to avoid leaking threads/resources by switching tests from “fire-and-forget” backend helpers to RAII shutdown guards (BackendGuard) so backends stop when each test completes.

Changes:

  • Removes the leaky start_backend helper from praxis_test_utils and updates re-exports accordingly.
  • Updates many smoke/security/resilience/conformance/configuration tests to use start_backend_with_shutdown(...).port() (and similar guarded helpers).
  • Updates the stream buffer example to return both backend and proxy guards so both are scoped to the test.

Reviewed changes

Copilot reviewed 42 out of 42 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/utils/src/net/mod.rs Updates test-utils re-exports to prefer shutdown-capable backend helpers.
tests/utils/src/net/backend/simple.rs Removes start_backend and keeps start_backend_with_shutdown returning BackendGuard.
tests/utils/src/net/backend/mod.rs Stops re-exporting the removed start_backend helper.
tests/smoke/tests/suite/smoke.rs Uses guarded backend startup so smoke tests don’t leak backend threads.
tests/security/tests/suite/request_smuggling.rs Migrates fixed backend usage to start_backend_with_shutdown.
tests/security/tests/suite/ip_acl.rs Migrates backends (including header echo) to shutdown-guard variants.
tests/security/tests/suite/info_leakage.rs Migrates fixed backend usage to start_backend_with_shutdown.
tests/security/tests/suite/host_header.rs Migrates fixed backend usage to start_backend_with_shutdown, including attacker/victim backends.
tests/security/tests/suite/filter_leakage.rs Migrates fixed backend usage to start_backend_with_shutdown.
tests/resilience/tests/suite/throughput_simple.rs Uses guarded backend startup for throughput benchmarks.
tests/resilience/tests/suite/throughput_filter_chain.rs Uses guarded backend startup for filter-chain throughput benchmarks.
tests/resilience/tests/suite/rate_limit_burst.rs Uses guarded backend startup for burst/rate-limit tests.
tests/resilience/tests/suite/multi_listener_isolation.rs Uses guarded backend startup for multi-listener isolation scenarios.
tests/resilience/tests/suite/large_payload.rs Uses guarded backend startup for response-size handling scenario.
tests/resilience/tests/suite/concurrent_load.rs Uses guarded backend startup for concurrent load scenarios.
tests/resilience/tests/suite/backend_recovery.rs Uses guarded backend startup for recovery scenario.
tests/resilience/tests/suite/backend_failure.rs Uses guarded backend startup for failure scenarios that need a live backend.
tests/conformance/tests/suite/tls.rs Uses guarded backend startup for TLS conformance tests.
tests/conformance/tests/suite/rfcs/rfc9112.rs Uses guarded backend startup for RFC 9112 conformance tests.
tests/conformance/tests/suite/rfcs/rfc9110.rs Uses guarded backend startup for RFC 9110 conformance tests.
tests/conformance/tests/suite/ipv6.rs Uses guarded backend startup in IPv6-related conformance tests.
tests/conformance/tests/suite/http.rs Uses guarded backend startup across HTTP robustness tests.
tests/conformance/tests/suite/cors.rs Uses guarded backend startup across CORS conformance tests.
tests/conformance/tests/suite/chunked.rs Uses guarded backend startup across chunked-transfer tests.
tests/configuration/tests/suite/examples/transformation/header_manipulation.rs Updates example test to use shutdown-guard backend when wiring example config.
tests/configuration/tests/suite/examples/traffic_management/weighted_load_balancing.rs Uses guarded backend startup for weighted LB example test.
tests/configuration/tests/suite/examples/traffic_management/virtual_hosts.rs Uses guarded backend startup for virtual hosts example test.
tests/configuration/tests/suite/examples/traffic_management/timeout.rs Uses guarded backend startup for timeout example test.
tests/configuration/tests/suite/examples/traffic_management/session_affinity.rs Uses guarded backend startup for session affinity example test.
tests/configuration/tests/suite/examples/traffic_management/round_robin.rs Uses guarded backend startup for round-robin example test.
tests/configuration/tests/suite/examples/traffic_management/path_based_routing.rs Uses guarded backend startup for path-based routing example test.
tests/configuration/tests/suite/examples/traffic_management/least_connections.rs Uses guarded backend startup for least-connections example test.
tests/configuration/tests/suite/examples/traffic_management/canary_routing.rs Uses guarded backend startup for canary routing example test.
tests/configuration/tests/suite/examples/traffic_management/basic_reverse_proxy.rs Uses guarded backend startup for basic reverse proxy example test.
tests/configuration/tests/suite/examples/pipeline/conditional_filters.rs Uses guarded backend startup for conditional filters example test.
tests/configuration/tests/suite/examples/payload_processing/stream_buffer.rs Returns (BackendGuard, ProxyGuard) so both backend and proxy are scoped to each test.
tests/configuration/tests/suite/examples/operations/multi_listener.rs Uses guarded backend startup for multi-listener example test.
tests/configuration/tests/suite/examples/observability/logging.rs Uses guarded backend startup for logging example test.
tests/configuration/tests/suite/examples/observability/access_logging.rs Uses guarded backend startup for access logging example test.
tests/configuration/tests/suite/examples/max_body_guard.rs Uses guarded backend startup for max body guard example test.
tests/configuration/tests/suite/examples/api_key_filter.rs Uses guarded backend startup for API key filter example test.
tests/configuration/tests/suite/examples/ai/model_to_header.rs Uses guarded backend startup for model-to-header routing example tests.

Comment on lines +14 to +17
Backend, BackendGuard, RoutedBackend, start_backend_v6, start_backend_with_shutdown, start_echo_backend,
start_echo_backend_with_shutdown, start_header_echo_backend, start_header_echo_backend_with_shutdown,
start_hop_by_hop_response_backend, start_slow_backend, start_uri_echo_backend,
start_uri_echo_backend_with_shutdown,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea; PR is already over size limits. Let's discuss at the next meeting if we need further follow-ups PRs to polish the existing tests.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made an exception and flagged this with conventions/bypass 👍

Comment on lines +16 to +21
let backend = start_backend_with_shutdown("ok");
let proxy_port = free_port();
let config = crate::example_utils::load_example_config(
"transformation/header-manipulation.yaml",
proxy_port,
HashMap::from([("127.0.0.1:3000", backend_port)]),
HashMap::from([("127.0.0.1:3000", backend.port())]),
Comment on lines +33 to +34
let _backend = start_backend_with_shutdown("ok");
let backend_port = _backend.port();
Comment on lines +17 to +18
let _backend = start_backend_with_shutdown("ok");
let backend_port = _backend.port();
@shaneutt shaneutt self-assigned this Apr 24, 2026
@github-project-automation github-project-automation Bot moved this to Backlog in AI Gateway Apr 24, 2026
@shaneutt shaneutt moved this from Backlog to Review in AI Gateway Apr 24, 2026
@shaneutt shaneutt added this to the v0.5.0 milestone Apr 24, 2026
@esnible
Copy link
Copy Markdown
Member Author

esnible commented Apr 25, 2026

@shaneutt I am having difficulty rebasing -- I think it would be faster to regenerate the PR on top of the current HEAD.

Before I do that, do we really need this PR? I only did implemented this because of a comment on a previous PR that seemed to request that I should refactor all of the tests, not just enough to get things working on the Mac.

@shaneutt
Copy link
Copy Markdown
Member

Sure, let's consider this resolved then 👍

@shaneutt shaneutt closed this Apr 26, 2026
@github-project-automation github-project-automation Bot moved this from Review to Done in AI Gateway Apr 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants