rohangarg commented on code in PR #13475:
URL: https://github.com/apache/druid/pull/13475#discussion_r1039901091
##########
extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/exec/WorkerSketchFetcherTest.java:
##########
@@ -113,20 +117,30 @@ public void tearDown() throws Exception
public void
test_submitFetcherTask_parallelFetch_workerThrowsException_shouldCancelOtherTasks()
throws Exception
{
// Store futures in a queue
- final Queue<ListenableFuture<ClusterByStatisticsSnapshot>> futureQueue =
new ConcurrentLinkedQueue<>();
+ final Queue<Future<?>> futureQueue = new ConcurrentLinkedQueue<>();
final List<String> workerIds = ImmutableList.of("0", "1", "2", "3");
final CountDownLatch latch = new CountDownLatch(workerIds.size());
- target = spy(new WorkerSketchFetcher(workerClient,
ClusterStatisticsMergeMode.PARALLEL, 300_000_000));
+ target = spy(
+ new WorkerSketchFetcher(
+ workerClient,
+ ClusterStatisticsMergeMode.PARALLEL,
+ 300_000_000,
+ executorService
+ )
+ );
+
+ // When submitting futures from the executor, add it to the list first.
+ doAnswer(invocation -> {
+ Future<?> future = spy((Future<?>) invocation.callRealMethod());
+ futureQueue.add(future);
+ return future;
+ }).when(executorService).submit(any(Runnable.class));
- // When fetching snapshots, return a mock and add future to queue
doAnswer(invocation -> {
- ListenableFuture<ClusterByStatisticsSnapshot> snapshotListenableFuture =
-
spy(Futures.immediateFuture(mock(ClusterByStatisticsSnapshot.class)));
- futureQueue.add(snapshotListenableFuture);
latch.countDown();
latch.await();
- return snapshotListenableFuture;
+ return Futures.immediateFuture(mock(ClusterByStatisticsSnapshot.class));
}).when(workerClient).fetchClusterByStatisticsSnapshot(any(), any(),
anyInt());
Review Comment:
Ah, yes I missed that this method is for a part of the computation and not
the whole computation itself.
Although the same concept can be still applied and we can make the
computation in this mocking long timed and wait for an interruption arisen from
the cancel. I tried the following approach locally :
https://github.com/apache/druid/commit/c536a3f414c20dec35c2a47ef47e4d1049ed1091
The goal of the above is to minimize mocking/spying from the attributes of
the class being UT-ed - but I'm ok with the any of the approaches for now since
this change is planned for release 25.0
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
