Skip to content

Commit 9c86057

Browse files
committed
Address PR review: use debug logging and reduce test duplication
- Change LOGGER.info to LOGGER.debug in StoreRequestHandler.getClusterHealthStores() - Consolidate test methods using @dataProvider to reduce duplication in StoreRequestHandlerTest
1 parent d62252c commit 9c86057

2 files changed

Lines changed: 20 additions & 28 deletions

File tree

services/venice-controller/src/main/java/com/linkedin/venice/controller/server/StoreRequestHandler.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -270,9 +270,9 @@ public MultiStoreStatusResponse getClusterHealthStores(String clusterName) {
270270
throw new IllegalArgumentException("Cluster name is required");
271271
}
272272

273-
LOGGER.info("Getting health status for all stores in cluster: {}", clusterName);
273+
LOGGER.debug("Getting health status for all stores in cluster: {}", clusterName);
274274
Map<String, String> storeStatusMap = admin.getAllStoreStatuses(clusterName);
275-
LOGGER.info("Found {} stores with health status in cluster: {}", storeStatusMap.size(), clusterName);
275+
LOGGER.debug("Found {} stores with health status in cluster: {}", storeStatusMap.size(), clusterName);
276276

277277
MultiStoreStatusResponse response = new MultiStoreStatusResponse();
278278
response.setCluster(clusterName);

services/venice-controller/src/test/java/com/linkedin/venice/controller/server/StoreRequestHandlerTest.java

Lines changed: 18 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import java.util.Map;
3737
import java.util.Optional;
3838
import org.testng.annotations.BeforeMethod;
39+
import org.testng.annotations.DataProvider;
3940
import org.testng.annotations.Test;
4041

4142

@@ -364,43 +365,34 @@ public void testListStoresWithDataReplicationPolicyFilterNullPolicy() {
364365
assertEquals(response.getStoreNamesCount(), 0);
365366
}
366367

367-
@Test
368-
public void testGetClusterHealthStoresSuccess() {
369-
Map<String, String> storeStatusMap = new HashMap<>();
370-
storeStatusMap.put("store1", "ONLINE");
371-
storeStatusMap.put("store2", "DEGRADED");
372-
storeStatusMap.put("store3", "UNAVAILABLE");
373-
when(admin.getAllStoreStatuses("testCluster")).thenReturn(storeStatusMap);
374-
375-
MultiStoreStatusResponse response = storeRequestHandler.getClusterHealthStores("testCluster");
376-
377-
verify(admin, times(1)).getAllStoreStatuses("testCluster");
378-
assertEquals(response.getCluster(), "testCluster");
379-
assertEquals(response.getStoreStatusMap().size(), 3);
380-
assertEquals(response.getStoreStatusMap().get("store1"), "ONLINE");
381-
assertEquals(response.getStoreStatusMap().get("store2"), "DEGRADED");
382-
assertEquals(response.getStoreStatusMap().get("store3"), "UNAVAILABLE");
368+
@DataProvider(name = "storeStatusMaps")
369+
public Object[][] storeStatusMaps() {
370+
Map<String, String> populatedMap = new HashMap<>();
371+
populatedMap.put("store1", "ONLINE");
372+
populatedMap.put("store2", "DEGRADED");
373+
populatedMap.put("store3", "UNAVAILABLE");
374+
return new Object[][] { { populatedMap }, { Collections.emptyMap() } };
383375
}
384376

385-
@Test
386-
public void testGetClusterHealthStoresEmptyMap() {
387-
when(admin.getAllStoreStatuses("testCluster")).thenReturn(Collections.emptyMap());
377+
@Test(dataProvider = "storeStatusMaps")
378+
public void testGetClusterHealthStores(Map<String, String> expectedStatusMap) {
379+
when(admin.getAllStoreStatuses("testCluster")).thenReturn(expectedStatusMap);
388380

389381
MultiStoreStatusResponse response = storeRequestHandler.getClusterHealthStores("testCluster");
390382

391383
verify(admin, times(1)).getAllStoreStatuses("testCluster");
392384
assertEquals(response.getCluster(), "testCluster");
393-
assertEquals(response.getStoreStatusMap().size(), 0);
385+
assertEquals(response.getStoreStatusMap(), expectedStatusMap);
394386
}
395387

396-
@Test(expectedExceptions = IllegalArgumentException.class, expectedExceptionsMessageRegExp = "Cluster name is required")
397-
public void testGetClusterHealthStoresMissingClusterName() {
398-
storeRequestHandler.getClusterHealthStores(null);
388+
@DataProvider(name = "blankClusterNames")
389+
public Object[][] blankClusterNames() {
390+
return new Object[][] { { null }, { "" } };
399391
}
400392

401-
@Test(expectedExceptions = IllegalArgumentException.class, expectedExceptionsMessageRegExp = "Cluster name is required")
402-
public void testGetClusterHealthStoresEmptyClusterName() {
403-
storeRequestHandler.getClusterHealthStores("");
393+
@Test(dataProvider = "blankClusterNames", expectedExceptions = IllegalArgumentException.class, expectedExceptionsMessageRegExp = "Cluster name is required")
394+
public void testGetClusterHealthStoresWithBlankClusterName(String clusterName) {
395+
storeRequestHandler.getClusterHealthStores(clusterName);
404396
}
405397

406398
@Test

0 commit comments

Comments
 (0)