fix: stop router before NATS in integration test cleanup#557
Closed
fix: stop router before NATS in integration test cleanup#557
Conversation
This prevents the subscriber's ClosedCB from firing log.Fatal when NATS is stopped first, which was causing the test process to exit prematurely and leading to port binding conflicts in parallel test runs. The cleanup order is now: 1. Terminate gorouter session 2. Stop NATS server 3. Clean up test files This matches the fix from upstream PR #555 (commit b2bf830) which resolved similar issues in router/router_test.go.
cf-foundation-community-automation
Bot
moved this to Inbox
in Application Runtime Platform Working Group
hoffmaen
approved these changes
Apr 20, 2026
github-project-automation
Bot
moved this from Inbox
to Pending Merge | Prioritized
in Application Runtime Platform Working Group
hoffmaen
added a commit
to sap-contributions/routing-release
that referenced
this pull request
Apr 20, 2026
Fixes cleanup order in StopAndCleanup(), main_test.go AfterEach, and nats_test.go AfterEach. Stopping NATS first causes the subscriber's ClosedCB to fire log.Fatal → os.Exit(1), killing the test proc. Adopts the same fix as PR cloudfoundry#557 and extends it to all integration test files.
Contributor
github-project-automation
Bot
moved this from Pending Merge | Prioritized
to Done
in Application Runtime Platform Working Group
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes port binding conflicts in parallel integration test runs by correcting the cleanup order in
StopAndCleanup().Problem
When running integration tests in parallel (
--nodes=7), tests were intermittently failing with port binding errors:The root cause was the cleanup order in
common_integration_test.go:When NATS stops before the router, the subscriber's
ClosedCBcallback fireslog.Fatal→os.Exit(1), killing the test process prematurely before ports are fully released. In parallel runs, this causes subsequent tests to fail when attempting to bind to the same ports.Solution
Reversed the cleanup order:
This allows the router and its subscriber to disconnect gracefully before NATS shuts down.
Related
This follows the same pattern as the fix in commit b2bf830 (from PR #555) which resolved identical cleanup ordering issues in
router/router_test.go, but that fix was not applied to the integration test suite.Testing
This change affects all integration tests using
testState.StopAndCleanup(). The fix should reduce or eliminate flaky port conflict failures in CI when tests run in parallel.