commit 29e874a7719ce4e835e64092348c1fd6a08025cb
author Luca Milanesio <luca.milanesio@gmail.com> Tue Oct 10 20:10:27 2023 +0100
committer Luca Milanesio <luca.milanesio@gmail.com> Wed Oct 11 18:17:42 2023 +0000
tree c404b6bca01876e40480c687b68fc73ca8c4c33f
parent 469f6a12d5a30b597ae6217c859db44d380db0a8

Remove white-box unit tests on synchronous FetchCommand

The purpose of unit testing should be verifying that a
component respects its contract with its user through an
API. Some of the tests of the synchronous fetch replication
did instead verify the internal implementation detail of
the replication happening through a zero-delayed execution
of a scheduled task.

Remove all white-box tests which would be misleading in
giving a false perception of confidence that the component
was working.

Change-Id: I5c0b3ee4ceb0456a0b5d12463054afcfe8a4ee0f

src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchCommandTest.java

diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchCommandTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchCommandTest.java
index 7bd723b..156481b 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchCommandTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchCommandTest.java
@@ -16,9 +16,7 @@
 
 import static com.google.gerrit.testing.GerritJUnit.assertThrows;
 import static com.googlesource.gerrit.plugins.replication.pull.ReplicationType.ASYNC;
-import static com.googlesource.gerrit.plugins.replication.pull.ReplicationType.SYNC;
 import static org.mockito.Mockito.any;
-import static org.mockito.Mockito.anyLong;
 import static org.mockito.Mockito.anyString;
 import static org.mockito.Mockito.eq;
 import static org.mockito.Mockito.times;
@@ -36,10 +34,7 @@
 import com.googlesource.gerrit.plugins.replication.pull.api.exception.RemoteConfigurationMissingException;
 import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.TimeoutException;
 import org.eclipse.jgit.transport.URIish;
 import org.junit.Before;
 import org.junit.Test;
@@ -84,14 +79,6 @@
   }
 
   @Test
-  public void shouldScheduleRefFetch() throws Exception {
-    objectUnderTest.fetchSync(projectName, label, REF_NAME_TO_FETCH);
-
-    verify(source, times(1))
-        .schedule(projectName, REF_NAME_TO_FETCH, state, SYNC, Optional.empty());
-  }
-
-  @Test
   public void shouldScheduleRefFetchWithDelay() throws Exception {
     objectUnderTest.fetchAsync(projectName, label, REF_NAME_TO_FETCH, apiRequestMetrics);
 
@@ -100,62 +87,10 @@
   }
 
   @Test
-  public void shouldMarkAllFetchTasksScheduled() throws Exception {
-    objectUnderTest.fetchSync(projectName, label, REF_NAME_TO_FETCH);
-
-    verify(source, times(1))
-        .schedule(projectName, REF_NAME_TO_FETCH, state, SYNC, Optional.empty());
-    verify(state, times(1)).markAllFetchTasksScheduled();
-  }
-
-  @Test
   public void shouldUpdateStateWhenRemoteConfigNameIsMissing() {
     assertThrows(
         RemoteConfigurationMissingException.class,
         () -> objectUnderTest.fetchSync(projectName, "unknownLabel", REF_NAME_TO_FETCH));
     verify(fetchStateLog, times(1)).error(anyString(), eq(state));
   }
-
-  @SuppressWarnings("unchecked")
-  @Test
-  public void shouldUpdateStateWhenInterruptedException() throws Exception {
-    when(future.get(anyLong(), eq(TimeUnit.SECONDS))).thenThrow(new InterruptedException());
-    when(source.schedule(projectName, REF_NAME_TO_FETCH, state, SYNC, Optional.empty()))
-        .thenReturn(future);
-
-    InterruptedException e =
-        assertThrows(
-            InterruptedException.class,
-            () -> objectUnderTest.fetchSync(projectName, label, REF_NAME_TO_FETCH));
-    verify(fetchStateLog, times(1)).error(anyString(), eq(e), eq(state));
-  }
-
-  @SuppressWarnings("unchecked")
-  @Test
-  public void shouldUpdateStateWhenExecutionException() throws Exception {
-    when(future.get(anyLong(), eq(TimeUnit.SECONDS)))
-        .thenThrow(new ExecutionException(new Exception()));
-    when(source.schedule(projectName, REF_NAME_TO_FETCH, state, SYNC, Optional.empty()))
-        .thenReturn(future);
-
-    ExecutionException e =
-        assertThrows(
-            ExecutionException.class,
-            () -> objectUnderTest.fetchSync(projectName, label, REF_NAME_TO_FETCH));
-    verify(fetchStateLog, times(1)).error(anyString(), eq(e), eq(state));
-  }
-
-  @SuppressWarnings("unchecked")
-  @Test
-  public void shouldUpdateStateWhenTimeoutException() throws Exception {
-    when(future.get(anyLong(), eq(TimeUnit.SECONDS))).thenThrow(new TimeoutException());
-    when(source.schedule(projectName, REF_NAME_TO_FETCH, state, SYNC, Optional.empty()))
-        .thenReturn(future);
-
-    TimeoutException e =
-        assertThrows(
-            TimeoutException.class,
-            () -> objectUnderTest.fetchSync(projectName, label, REF_NAME_TO_FETCH));
-    verify(fetchStateLog, times(1)).error(anyString(), eq(e), eq(state));
-  }
 }