Fix BGP test server shutdown for asyncio compatibility#1276
Closed
thomas-mangin wants to merge 6 commits intomainfrom
Closed
Fix BGP test server shutdown for asyncio compatibility#1276thomas-mangin wants to merge 6 commits intomainfrom
thomas-mangin wants to merge 6 commits intomainfrom
Conversation
The __del__ method in the Exec class was not actually calling terminate() due to missing parentheses. This caused subprocesses to never be cleaned up properly, leading to zombie processes that would cause the test runner to hang indefinitely. This bug prevented CI from properly detecting test failures because the functional test script would never exit and return an error code. Fixes: qa/bin/functional:171
When tests timeout, they were marked as failed visually but the success variable was never updated to False. This caused the script to return exit code 0 (success) even when tests timed out, preventing CI from detecting these failures. This fix ensures that any test remaining in the running set after the timeout is properly counted as a failure, causing the script to return exit code 1. Fixes apply to: - EncodingTests.run_selected (line 521) - DecodingTests.run_selected (line 657) - ParsingTests.run_selected (line 731)
When encoding tests timeout, the server processes were terminated but the client processes were not, potentially leaving hanging processes. This fix ensures that both server and client processes are properly terminated when tests timeout. Fixes: EncodingTests.run_selected (line 521)
…est-errors-011CUqMFyWqNsW2Ao3Uq35ZY Fix CI/CD error detection for functional tests
The BGP test server (qa/sbin/bgp) was using loop.stop() which doesn't work correctly with asyncio.run() and server.serve_forever(). Changes: - Added shutdown_event (asyncio.Event) to BGPService class - Modified exit() to set the event and close the server - Changed main() to wait on shutdown_event instead of serve_forever() This allows the test server to properly terminate when tests complete, fixing an issue where encoding tests would hang waiting for the server to shutdown. The fix follows asyncio best practices for graceful server shutdown.
Fixed bug in the Exec class where terminate() and collect() were calling methods on self._process without checking if it was None first, causing AttributeError during cleanup. Changes: - Added None check in collect() before accessing _process - Added None check in terminate() before sending signal This fixes the CI error: AttributeError: 'NoneType' object has no attribute 'send_signal'
thomas-mangin
deleted the
claude/fix-bgp-test-server-shutdown-6.0-011CUqGm1Mh22ZD3dkUZA397
branch
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.
The BGP test server (qa/sbin/bgp) was using loop.stop() which doesn't work correctly with asyncio.run() and server.serve_forever().
Changes:
This allows the test server to properly terminate when tests complete, fixing an issue where encoding tests would hang waiting for the server to shutdown.
The fix follows asyncio best practices for graceful server shutdown.