test(pubsub): fix test race and re-enable testRunShutdown_TimeoutExceeded - #14339
test(pubsub): fix test race and re-enable testRunShutdown_TimeoutExceeded#14339michaelpri10 wants to merge 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request re-enables and fixes the previously ignored testRunShutdown_TimeoutExceeded test in StreamingSubscriberConnectionTest.java by adjusting timeouts and implementing a polling loop to wait for the stop thread to enter the TIMED_WAITING state. Additionally, it adds connection cleanup (stopAsync and awaitTerminated) to several other tests. The review feedback correctly points out that calling awaitTerminated() without a timeout in these cleanup steps could cause the test suite to hang indefinitely if a connection fails to shut down, and suggests adding a 5-second timeout instead.
| long startPoll = System.currentTimeMillis(); | ||
| while (t.getState() != Thread.State.TIMED_WAITING && t.isAlive()) { | ||
| if (System.currentTimeMillis() - startPoll > 5000) { | ||
| fail("Timed out waiting for stop thread to enter TIMED_WAITING"); | ||
| } | ||
| Thread.sleep(10); | ||
| } |
There was a problem hiding this comment.
fyi, we have introduced awaitility for testing deps in the Java SDK. Might be easier to test for polling cases.
2 participants
Re-enable
StreamingSubscriberConnectionTest.testRunShutdown_TimeoutExceededand resolve the test race condition that caused builds to hang indefinitely.
for
Thread.State.TIMED_WAITINGbefore advancingFakeClock. Thisguarantees
startTimeis recorded before time advances, eliminatingthe infinite wait loop in
Waiter.tryWait().by ~1.5s per run.
monitor tests to prevent background callbacks leaking into other tests.
org.junit.Ignoreimport.Fixes #13706