fix: clean HibernateSequentialNumberCounterStoreTest flaky tests (#16202

) * fix: clean config * fix: remove random string, parameterize tests * fix: missing test dependency
dhis2 · Jan 18, 2024 · db28163 · db28163
1 parent 83ad6f9
commit db28163
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 108 deletions.
diff --git a/...ain/java/org/hisp/dhis/reservedvalue/hibernate/HibernateSequentialNumberCounterStore.java b/...ain/java/org/hisp/dhis/reservedvalue/hibernate/HibernateSequentialNumberCounterStore.java
@@ -28,7 +28,6 @@
 package org.hisp.dhis.reservedvalue.hibernate;
 
 import java.util.List;
-import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 import javax.persistence.EntityManager;
 import lombok.RequiredArgsConstructor;
@@ -54,9 +53,7 @@ public List<Integer> getNextValues(String uid, String key, int length) {
                 .setParameter("length", length)
                 .getSingleResult();
 
-    return IntStream.range(count - length, length + (count - length))
-        .boxed()
-        .collect(Collectors.toList());
+    return IntStream.range(count - length, length + (count - length)).boxed().toList();
   }
 
   @Override

diff --git a/dhis-2/dhis-test-integration/pom.xml b/dhis-2/dhis-test-integration/pom.xml
@@ -189,6 +189,11 @@
       <artifactId>junit-jupiter-api</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.junit.jupiter</groupId>
+      <artifactId>junit-jupiter-params</artifactId>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.skyscreamer</groupId>
       <artifactId>jsonassert</artifactId>

diff --git a/...is-test-integration/src/test/java/org/hisp/dhis/reservedvalue/hibernate/DummyService.java b/...is-test-integration/src/test/java/org/hisp/dhis/reservedvalue/hibernate/DummyService.java
diff --git a/...java/org/hisp/dhis/reservedvalue/hibernate/HibernateSequentialNumberCounterStoreTest.java b/...java/org/hisp/dhis/reservedvalue/hibernate/HibernateSequentialNumberCounterStoreTest.java
@@ -43,36 +43,48 @@
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
-import org.apache.commons.lang3.RandomStringUtils;
+import java.util.stream.Stream;
 import org.hisp.dhis.reservedvalue.SequentialNumberCounterStore;
 import org.hisp.dhis.test.integration.TransactionalIntegrationTest;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.context.annotation.Bean;
-import org.springframework.context.annotation.Configuration;
 
 class HibernateSequentialNumberCounterStoreTest extends TransactionalIntegrationTest {
 
-  @Autowired private DummyService dummyService;
+  @Autowired private SequentialNumberCounterStore sequentialNumberCounterStore;
 
   @Test
   void getNextValues() {
-    List<Integer> result = dummyService.getNextValues("ABC", "ABC-#", 3);
+    List<Integer> result = nextSequentialValues("ABC", "ABC-#", 3);
     assertEquals(3, result.size());
     assertTrue(result.contains(1));
     assertTrue(result.contains(2));
     assertTrue(result.contains(3));
-    result = dummyService.getNextValues("ABC", "ABC-#", 50);
+    result = nextSequentialValues("ABC", "ABC-#", 50);
     assertEquals(50, result.size());
     assertTrue(result.contains(4));
     assertTrue(result.contains(5));
     assertTrue(result.contains(52));
     assertTrue(result.contains(53));
   }
 
-  private void test(final int threadCount) throws InterruptedException, ExecutionException {
-    final String uid = RandomStringUtils.randomAlphabetic(3).toUpperCase();
-    Callable<List<Integer>> task = () -> dummyService.getNextValues(uid, uid + "-#", 50);
+  private static Stream<Arguments> threadCounter() {
+    return Stream.of(
+        Arguments.of(1, "AAA"),
+        Arguments.of(4, "BBB"),
+        Arguments.of(8, "CCC"),
+        Arguments.of(16, "DDD"),
+        Arguments.of(32, "EEE"));
+  }
+
+  @ParameterizedTest
+  @MethodSource("threadCounter")
+  void shouldGenerateSequentialValueGivenThreadCounter(int threadCount, String uid)
+      throws InterruptedException, ExecutionException {
+    Callable<List<Integer>> task = () -> nextSequentialValues(uid, uid + "-#", 50);
     List<Callable<List<Integer>>> tasks = Collections.nCopies(threadCount, task);
     ExecutorService executorService = Executors.newFixedThreadPool(threadCount);
     List<Future<List<Integer>>> futures = executorService.invokeAll(tasks);
@@ -95,52 +107,24 @@ private void test(final int threadCount) throws InterruptedException, ExecutionE
     assertThat(allIdList.get(allIdList.size() - 1), is(50 * threadCount));
   }
 
-  @Test
-  void test1() throws InterruptedException, ExecutionException {
-    test(1);
-  }
-
-  @Test
-  void test4() throws InterruptedException, ExecutionException {
-    test(4);
-  }
-
-  @Test
-  void test8() throws InterruptedException, ExecutionException {
-    test(8);
-  }
-
-  @Test
-  void test16() throws InterruptedException, ExecutionException {
-    test(16);
-  }
-
-  @Test
-  void test32() throws InterruptedException, ExecutionException {
-    test(32);
-  }
-
   @Test
   void deleteCounter() {
-    assertTrue(dummyService.getNextValues("ABC", "ABC-#", 3).contains(1));
-    dummyService.deleteCounter("ABC");
-    assertTrue(dummyService.getNextValues("ABC", "ABC-#", 3).contains(1));
-    assertTrue(dummyService.getNextValues("ABC", "ABC-##", 3).contains(1));
-    assertTrue(dummyService.getNextValues("ABC", "ABC-###", 3).contains(1));
-    dummyService.deleteCounter("ABC");
-    assertTrue(dummyService.getNextValues("ABC", "ABC-#", 3).contains(1));
-    assertTrue(dummyService.getNextValues("ABC", "ABC-##", 3).contains(1));
-    assertTrue(dummyService.getNextValues("ABC", "ABC-###", 3).contains(1));
+    assertTrue(nextSequentialValues("ABC", "ABC-#", 3).contains(1));
+    deleteCounter("ABC");
+    assertTrue(nextSequentialValues("ABC", "ABC-#", 3).contains(1));
+    assertTrue(nextSequentialValues("ABC", "ABC-##", 3).contains(1));
+    assertTrue(nextSequentialValues("ABC", "ABC-###", 3).contains(1));
+    deleteCounter("ABC");
+    assertTrue(nextSequentialValues("ABC", "ABC-#", 3).contains(1));
+    assertTrue(nextSequentialValues("ABC", "ABC-##", 3).contains(1));
+    assertTrue(nextSequentialValues("ABC", "ABC-###", 3).contains(1));
   }
 
-  @Configuration
-  static class TestConfig {
-
-    @Autowired private SequentialNumberCounterStore sequentialNumberCounterStore;
+  public List<Integer> nextSequentialValues(String uid, String key, int length) {
+    return sequentialNumberCounterStore.getNextValues(uid, key, length);
+  }
 
-    @Bean
-    public DummyService dummyService() {
-      return new DummyService(sequentialNumberCounterStore);
-    }
+  public void deleteCounter(String uid) {
+    sequentialNumberCounterStore.deleteCounter(uid);
   }
 }