From ed335b8221c079de7019510e8280b64e9acda532 Mon Sep 17 00:00:00 2001
From: Nisarg Thakkar <nisargtha@gmail.com>
Date: Fri, 13 Dec 2024 23:04:11 -0800
Subject: [PATCH] [test] Speed up
 TestControllerClient#testControllerClientWithInvalidUrls (#1395)

This test is validating that as long as at least one of the hosts in the Controller urls is healthy, the "ControllerClient" won't see a "ConnectException". However, the "ControllerClient" will randomize the controller order in every request, it is not deterministic that a single run will ever attempt to communicate with the unavailable controller. Our tests are also configured to retry failing tests multiple times, and if any of them passes, it assumes the test is good. So, regressions in this logic could get missed by our team. To prevent this, the test runs the test for 100 iterations.

However, this leads to a case where the test is running for 13 minutes on CI hosts. To speed up the test, we do two things:
1. Reduce the number of iterations to 50
2. Parallelize the loop to concurrently attempt multiple times
---
 .../venice/controllerapi/TestControllerClient.java         | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/internal/venice-test-common/src/integrationTest/java/com/linkedin/venice/controllerapi/TestControllerClient.java b/internal/venice-test-common/src/integrationTest/java/com/linkedin/venice/controllerapi/TestControllerClient.java
index 466346f4bd..2a8cacd8f6 100644
--- a/internal/venice-test-common/src/integrationTest/java/com/linkedin/venice/controllerapi/TestControllerClient.java
+++ b/internal/venice-test-common/src/integrationTest/java/com/linkedin/venice/controllerapi/TestControllerClient.java
@@ -22,6 +22,7 @@
 import java.io.IOException;
 import java.net.ConnectException;
 import java.util.Optional;
+import java.util.stream.IntStream;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
@@ -170,9 +171,9 @@ public void testControllerClientWithInvalidUrls() throws IOException {
           discoResponseInvalidControllers.getError());
 
       // When only some controllers are missing, the ConnectException should never be bubbled up. Since this behavior is
-      // triggered from Java libs, and we randomise the controller list to do some load balancing, the best way to
+      // triggered from Java libs, and we randomize the controller list to do some load balancing, the best way to
       // validate is to try multiple invocations
-      for (int i = 0; i < 100; i++) {
+      IntStream.rangeClosed(1, 50).parallel().forEach(i -> {
         D2ServiceDiscoveryResponse discoResponsePartialValidController = ControllerClient
             .discoverCluster(nonExistentControllerUrl1 + "," + validControllerUrl, storeName, Optional.empty(), 1);
         Assert.assertFalse(discoResponsePartialValidController.isError());
@@ -217,7 +218,7 @@ public void testControllerClientWithInvalidUrls() throws IOException {
             1);
         Assert.assertTrue(errorDiscoResponseInvalidAndLegacy.isError());
         Assert.assertEquals(errorDiscoResponseInvalidAndLegacy.getErrorType(), ErrorType.BAD_REQUEST);
-      }
+      });
 
       try (ControllerClient controllerClient =
           ControllerClientFactory.getControllerClient(clusterName, validControllerUrl, Optional.empty())) {