fix: ensure proper cleanup of publisher in tests #310
Conversation
Adding delivery attempt count to PubsubMessages as a message attribute, and creating helper function to allow users to get the count without knowing implementation details.
lettering is not enabled
google-cla
bot
added
the
cla: yes
hannahrogers-google
added
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
google-cla
bot
added
the
cla: yes
Codecov Report
@@ Coverage Diff @@
## master #310 +/- ##
============================================
- Coverage 79.28% 79.21% -0.07%
+ Complexity 319 318 -1
============================================
Files 21 21
Lines 2892 2892
Branches 155 155
============================================
- Hits 2293 2291 -2
- Misses 535 536 +1
- Partials 64 65 +1
Continue to review full report at Codecov.
|
hannahrogers-google
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
|
|
||
| private void shutdownTestPublisher(Publisher publisher) throws InterruptedException { | ||
| publisher.shutdown(); | ||
| fakeExecutor.advanceTime(Duration.ofSeconds(10)); |
There was a problem hiding this comment.
How was 10 seconds chosen? Specifically, how long does this test need to execute on average?
There was a problem hiding this comment.
The gax layer implements a Watchdog (https://github.com/googleapis/gax-java/blob/master/gax/src/main/java/com/google/api/gax/rpc/Watchdog.java) that periodically garbage collects idle streams. The check is scheduled at a fixed rate every 10 seconds. So, I chose 10 seconds to ensure that the watchdog check completes, otherwise we have to wait the full timeout.
product-auto-label
bot
added
the
api: pubsub
Our PublisherImplTest is taking over 5 minutes to run because we are not properly shutting down the publisher/testChannel. This PR ensures everything is shutdown properly, and decreases the test time to 10 seconds. Additional details provided in #309.
Fixes #309 ☕️