test: increase spawn test child timeout to 30 seconds#266
Open
Sjors wants to merge 1 commit intobitcoin-core:masterfrom
Open
test: increase spawn test child timeout to 30 seconds#266Sjors wants to merge 1 commit intobitcoin-core:masterfrom
Sjors wants to merge 1 commit intobitcoin-core:masterfrom
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process. |
maflcko
reviewed
Apr 1, 2026
test/mp/test/spawn_tests.cpp
Outdated
Comment on lines
+97
to
+99
| // Give the child up to 30 seconds to exit. If it does not, terminate it and | ||
| // reap it to avoid leaving a zombie behind. | ||
| const bool exited{WaitPidWithTimeout(pid, std::chrono::milliseconds{1000}, status)}; | ||
| const bool exited{WaitPidWithTimeout(pid, std::chrono::milliseconds{30000}, status)}; |
Contributor
There was a problem hiding this comment.
Suggested change
| // Give the child up to 30 seconds to exit. If it does not, terminate it and | |
| // reap it to avoid leaving a zombie behind. | |
| const bool exited{WaitPidWithTimeout(pid, std::chrono::milliseconds{1000}, status)}; | |
| const bool exited{WaitPidWithTimeout(pid, std::chrono::milliseconds{30000}, status)}; | |
| // Give the child some time to exit. If it does not, terminate it and | |
| // reap it to avoid leaving a zombie behind. | |
| const bool exited{WaitPidWithTimeout(pid, std::chrono::seconds{30}, status)}; |
nit: The chrono header can do the needed multiplication for you, no need to do it manually in the source code
Member
Author
There was a problem hiding this comment.
Nice, I'll let CI finish running and then push this.
Example CI failure: 3/157 Test bitcoin-core#3: mptest ...............................***Failed 1.06 sec [ TEST ] spawn_tests.cpp:44: SpawnProcess does not run callback in child ./ipc/libmultiprocess/test/mp/test/spawn_tests.cpp:108: failed: expected exited; Timeout waiting for child process to exit stack: bf2ed1 cc4091 cc4545 d817f9 d82ba3 bf220b ee33f4b9 ee33f555 ./ipc/libmultiprocess/test/mp/test/spawn_tests.cpp:109: failed: expected WIFEXITED(status) && WEXITSTATUS(status) == 0 stack: bf2e03 cc4091 cc4545 d817f9 d82ba3 bf220b ee33f4b9 ee33f555 [ FAIL ] spawn_tests.cpp:44: SpawnProcess does not run callback in child (1051610 μs) [ TEST ] test.cpp:124: Call FooInterface methods [ PASS ] test.cpp:124: Call FooInterface methods (3744 μs) [ TEST ] test.cpp:216: Call IPC method after client connection is closed [ PASS ] test.cpp:216: Call IPC method after client connection is closed (363 μs) [ TEST ] test.cpp:233: Calling IPC method after server connection is closed [ PASS ] test.cpp:233: Calling IPC method after server connection is closed (346 μs) [ TEST ] test.cpp:250: Calling IPC method and disconnecting during the call [ PASS ] test.cpp:250: Calling IPC method and disconnecting during the call (354 μs) [ TEST ] test.cpp:270: Calling IPC method, disconnecting and blocking during the call [ PASS ] test.cpp:270: Calling IPC method, disconnecting and blocking during the call (554 μs) [ TEST ] test.cpp:319: Make simultaneous IPC calls on single remote thread [ PASS ] test.cpp:319: Make simultaneous IPC calls on single remote thread (741 μs) 6 test(s) passed 1 test(s) failed Fixes bitcoin/bitcoin#34975
Sjors
force-pushed
the
2026/04/spwan-test-timeout
branch
from
ad6171d to
dba3485
Compare
Member
Author
|
No timeouts on the 3 jobs x 2 pushes worth of Bitcoin Core CI runs here. However, I don't remember seeing those here before anyway. I also opened a subtree update PR bitcoin/bitcoin#34977 so that should give us another run. |
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
3 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.
Bump timeout from 1 second to 30 to prevent spurious timeouts.
Fixes bitcoin/bitcoin#34975