m-trieu commented on code in PR #34283: URL: https://github.com/apache/beam/pull/34283#discussion_r2023487998
########## runners/google-cloud-dataflow-java/worker/src/test/java/org/apache/beam/runners/dataflow/worker/windmill/client/AbstractWindmillStreamTest.java: ########## @@ -84,68 +85,60 @@ public void testShutdown_notBlockedBySend() throws InterruptedException, Executi } @Test - public void testMaybeSendHealthCheck() throws InterruptedException, ExecutionException { - TestCallStreamObserver callStreamObserver = new TestCallStreamObserver(); + public void testMaybeScheduleHealthCheck() { + TestCallStreamObserver callStreamObserver = + new TestCallStreamObserver(/* waitForSend= */ false); Function<StreamObserver<Integer>, StreamObserver<Integer>> clientFactory = ignored -> callStreamObserver; TestStream testStream = newStream(clientFactory); testStream.start(); - ExecutorService sendExecutor = Executors.newSingleThreadExecutor(); Instant reportingThreshold = Instant.now().minus(Duration.millis(1)); - Future<?> sendFuture = - sendExecutor.submit(() -> testStream.maybeSendHealthCheck(reportingThreshold)); - - // Sleep a bit to give sendExecutor time to execute the send(). - Uninterruptibles.sleepUninterruptibly(100, TimeUnit.MILLISECONDS); - - callStreamObserver.unblockSend(); - - // Make sure the future completes. - sendFuture.get(); + testStream.maybeScheduleHealthCheck(reportingThreshold); + testStream.waitForHealthChecks(1); assertThat(testStream.numHealthChecks.get()).isEqualTo(1); testStream.shutdown(); } @Test - public void testMaybeSendHealthCheck_doesNotSendIfLastSendLessThanThreshold() - throws ExecutionException, InterruptedException { - TestCallStreamObserver callStreamObserver = new TestCallStreamObserver(); + public void testMaybeSendHealthCheck_doesNotSendIfLastScheduleLessThanThreshold() { + TestCallStreamObserver callStreamObserver = + new TestCallStreamObserver(/* waitForSend= */ false); Function<StreamObserver<Integer>, StreamObserver<Integer>> clientFactory = ignored -> callStreamObserver; TestStream testStream = newStream(clientFactory); testStream.start(); - ExecutorService sendExecutor = Executors.newSingleThreadExecutor(); - Future<?> sendFuture = - sendExecutor.submit( - () -> { - try { - testStream.trySend(1); - } catch (WindmillStreamShutdownException e) { - throw new RuntimeException(e); - } - - // Sleep a bit to give sendExecutor time to execute the send(). - Uninterruptibles.sleepUninterruptibly(100, TimeUnit.MILLISECONDS); - - // Set a really long reporting threshold. - Instant reportingThreshold = Instant.now().minus(Duration.standardHours(1)); - // Should not send health checks since we just sent the above message. - testStream.maybeSendHealthCheck(reportingThreshold); - testStream.maybeSendHealthCheck(reportingThreshold); - }); - callStreamObserver.unblockSend(); + try { + testStream.trySend(1); + } catch (WindmillStreamShutdownException e) { + throw new RuntimeException(e); + } + + // Sleep a bit to give sendExecutor time to execute the send(). + Uninterruptibles.sleepUninterruptibly(100, TimeUnit.MILLISECONDS); + + // Set a really long reporting threshold. + Instant reportingThreshold = Instant.now().minus(Duration.standardHours(1)); + + // Should not send health checks since we just sent the above message. + testStream.maybeScheduleHealthCheck(reportingThreshold); + Uninterruptibles.sleepUninterruptibly(100, TimeUnit.MILLISECONDS); + testStream.maybeScheduleHealthCheck(reportingThreshold); + Uninterruptibles.sleepUninterruptibly(100, TimeUnit.MILLISECONDS); + + callStreamObserver.waitForSend(); + Uninterruptibles.sleepUninterruptibly(100, TimeUnit.MILLISECONDS); Review Comment: done -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@beam.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org