Google Git
Sign in
gerrit/plugins/healthcheck/0128fec9e64b74c80247c4a9986dc87bb5f89764^1..0128fec9e64b74c80247c4a9986dc87bb5f89764/.
commit
0128fec9e64b74c80247c4a9986dc87bb5f89764
[log]
author
Luca Milanesio <luca.milanesio@gmail.com>
Tue Jul 06 11:07:58 2021 +0100
committer
Luca Milanesio <luca.milanesio@gmail.com>
Tue Jul 06 11:07:58 2021 +0100
tree
10c6b199f78df10ae439b02fd72d0aab192545f1
parent
771904bf39226698717796d2a3625db72ea61d88 [diff]
parent
6b0a1f60ab27faecaf9f58b65ca42f12703217e2 [diff]
Merge branch 'stable-3.1' into stable-3.2

* stable-3.1:
  Fix BlockedThreadsCheck failing with un-blocked threads
  Use Guice injector for BlockedThreadsCheck in tests

Change-Id: I78e1622bb79c6191802e5622b6185b3cdb94fa50

diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/BlockedThreadsCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/BlockedThreadsCheck.java
index f2cbb70..4e21771 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/BlockedThreadsCheck.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/BlockedThreadsCheck.java

@@ -23,7 +23,6 @@
 import com.google.inject.Inject;
 import com.google.inject.Module;
 import com.google.inject.Provider;
-import com.google.inject.Singleton;
 import com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig;
 import com.googlesource.gerrit.plugins.healthcheck.HealthCheckMetrics;
 import java.lang.management.ManagementFactory;
@@ -35,7 +34,6 @@
 import java.util.function.Supplier;
 import java.util.stream.Stream;
 
-@Singleton
 public class BlockedThreadsCheck extends AbstractHealthCheck {
   public static Module SUB_CHECKS =
       new FactoryModule() {

diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/BlockedThreadsCheckTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/BlockedThreadsCheckTest.java
index d50d12a..c52d646 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/BlockedThreadsCheckTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/BlockedThreadsCheckTest.java

@@ -20,13 +20,11 @@
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
-import com.google.common.util.concurrent.ListeningExecutorService;
 import com.google.inject.AbstractModule;
 import com.google.inject.Guice;
 import com.google.inject.Injector;
-import com.google.inject.util.Providers;
 import com.googlesource.gerrit.plugins.healthcheck.check.BlockedThreadsCheck;
-import com.googlesource.gerrit.plugins.healthcheck.check.BlockedThreadsConfigurator;
+import com.googlesource.gerrit.plugins.healthcheck.check.BlockedThreadsCheck.ThreadBeanProvider;
 import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck.Result;
 import java.lang.management.ThreadInfo;
 import java.lang.management.ThreadMXBean;
@@ -40,18 +38,28 @@
 
 @RunWith(MockitoJUnitRunner.class)
 public class BlockedThreadsCheckTest {
+  private static final String HEALTHCHECK_CONFIG_BODY_THRESHOLD =
+      "[healthcheck \"" + BLOCKEDTHREADS + "\"]\n" + "  threshold = ";
+  private static final HealthCheckConfig CONFIG_THRESHOLD_25 =
+      new HealthCheckConfig(HEALTHCHECK_CONFIG_BODY_THRESHOLD + "25");
+  private static final HealthCheckConfig CONFIG_THRESHOLD_33 =
+      new HealthCheckConfig(HEALTHCHECK_CONFIG_BODY_THRESHOLD + "33");
+
   @Mock BlockedThreadsCheck.ThreadBeanProvider threadBeanProviderMock;
 
   @Mock ThreadMXBean beanMock;
 
+  private Injector testInjector;
+
   @Before
   public void setUp() {
     when(threadBeanProviderMock.get()).thenReturn(beanMock);
+    testInjector = createTestInjector(HealthCheckConfig.DEFAULT_CONFIG);
   }
 
   @Test
   public void shouldPassCheckWhenNoThreadsAreReturned() {
-    BlockedThreadsCheck objectUnderTest = createCheck(HealthCheckConfig.DEFAULT_CONFIG);
+    BlockedThreadsCheck objectUnderTest = createCheck();
     when(beanMock.getThreadInfo(null, 0)).thenReturn(new ThreadInfo[0]);
     assertThat(objectUnderTest.run().result).isEqualTo(Result.PASSED);
   }
@@ -85,21 +93,42 @@
   }
 
   @Test
+  public void shouldFailThenPassCheck() {
+    String prefix = "some-prefix";
+
+    List<ThreadInfo> failingThreadInfo = new ArrayList<>();
+    failingThreadInfo.addAll(nCopies(1, mockInfo(Thread.State.RUNNABLE, prefix)));
+    failingThreadInfo.addAll(nCopies(3, mockInfo(Thread.State.BLOCKED, prefix)));
+
+    List<ThreadInfo> successThreadInfo = new ArrayList<>();
+    successThreadInfo.addAll(nCopies(1, mockInfo(Thread.State.RUNNABLE, prefix)));
+    successThreadInfo.addAll(nCopies(1, mockInfo(Thread.State.BLOCKED, prefix)));
+
+    when(beanMock.getThreadInfo(null, 0))
+        .thenReturn(
+            failingThreadInfo.toArray(new ThreadInfo[0]),
+            successThreadInfo.toArray(new ThreadInfo[0]));
+
+    assertThat(createCheck().run().result).isEqualTo(Result.FAILED);
+    assertThat(createCheck().run().result).isEqualTo(Result.PASSED);
+  }
+
+  @Test
   public void shouldPassCheckWhenBlockedThreadsAreLessThenThreshold() {
     int running = 3;
     int blocked = 1;
-    HealthCheckConfig config =
-        new HealthCheckConfig("[healthcheck \"" + BLOCKEDTHREADS + "\"]\n" + "  threshold = 25");
-    mockThreadsAndCheckResult(running, blocked, Result.PASSED, config);
+    testInjector = createTestInjector(CONFIG_THRESHOLD_25);
+
+    mockThreadsAndCheckResult(running, blocked, Result.PASSED);
   }
 
   @Test
   public void shouldFailCheckWhenBlockedThreadsAreAboveTheThreshold() {
     int running = 1;
     int blocked = 1;
-    HealthCheckConfig config =
-        new HealthCheckConfig("[healthcheck \"" + BLOCKEDTHREADS + "\"]\n" + "  threshold = 33");
-    mockThreadsAndCheckResult(running, blocked, Result.FAILED, config);
+    testInjector = createTestInjector(CONFIG_THRESHOLD_33);
+
+    mockThreadsAndCheckResult(running, blocked, Result.FAILED);
   }
 
   @Test
@@ -107,10 +136,9 @@
     int running = 3;
     int blocked = 1;
     String prefix = "blocked-threads-prefix";
-    HealthCheckConfig config =
-        new HealthCheckConfig(
-            "[healthcheck \"" + BLOCKEDTHREADS + "\"]\n" + "  threshold = " + prefix + " = 25");
-    mockThreadsAndCheckResult(running, blocked, Result.PASSED, prefix, config);
+    testInjector = createTestInjector(CONFIG_THRESHOLD_25);
+
+    mockThreadsAndCheckResult(running, blocked, Result.PASSED, prefix);
   }
 
   @Test
@@ -118,10 +146,9 @@
     int running = 1;
     int blocked = 1;
     String prefix = "blocked-threads-prefix";
-    HealthCheckConfig config =
-        new HealthCheckConfig(
-            "[healthcheck \"" + BLOCKEDTHREADS + "\"]\n" + "  threshold = " + prefix + " = 33");
-    mockThreadsAndCheckResult(running, blocked, Result.FAILED, prefix, config);
+    testInjector = createTestInjector(CONFIG_THRESHOLD_33);
+
+    mockThreadsAndCheckResult(running, blocked, Result.FAILED, prefix);
   }
 
   @Test
@@ -141,31 +168,27 @@
                 + "\nthreshold = "
                 + notBlockedPrefix
                 + "=33");
+    testInjector = createTestInjector(config);
+
     List<ThreadInfo> infos = new ArrayList<>(running + blocked + running);
     infos.addAll(nCopies(running, mockInfo(Thread.State.RUNNABLE, blockedPrefix)));
     infos.addAll(nCopies(blocked, mockInfo(Thread.State.BLOCKED, blockedPrefix)));
     infos.addAll(nCopies(running, mockInfo(Thread.State.RUNNABLE, notBlockedPrefix)));
     when(beanMock.getThreadInfo(null, 0)).thenReturn(infos.toArray(new ThreadInfo[infos.size()]));
-    checkResult(Result.FAILED, config);
+    checkResult(Result.FAILED);
   }
 
   private void mockThreadsAndCheckResult(int running, int blocked, Result expected) {
-    mockThreadsAndCheckResult(running, blocked, expected, HealthCheckConfig.DEFAULT_CONFIG);
+    mockThreadsAndCheckResult(running, blocked, expected, "some-prefix");
   }
 
-  private void mockThreadsAndCheckResult(
-      int running, int blocked, Result expected, HealthCheckConfig config) {
-    mockThreadsAndCheckResult(running, blocked, expected, "some-prefix", config);
-  }
-
-  private void mockThreadsAndCheckResult(
-      int running, int blocked, Result expected, String prefix, HealthCheckConfig config) {
+  private void mockThreadsAndCheckResult(int running, int blocked, Result expected, String prefix) {
     mockThreads(running, blocked, prefix);
-    checkResult(expected, config);
+    checkResult(expected);
   }
 
-  private void checkResult(Result expected, HealthCheckConfig config) {
-    BlockedThreadsCheck objectUnderTest = createCheck(config);
+  private void checkResult(Result expected) {
+    BlockedThreadsCheck objectUnderTest = createCheck();
     assertThat(objectUnderTest.run().result).isEqualTo(expected);
   }
 
@@ -183,8 +206,11 @@
     return infoMock;
   }
 
-  private BlockedThreadsCheck createCheck(HealthCheckConfig config) {
-    DummyHealthCheckMetricsFactory checkMetricsFactory = new DummyHealthCheckMetricsFactory();
+  private BlockedThreadsCheck createCheck() {
+    return testInjector.getInstance(BlockedThreadsCheck.class);
+  }
+
+  private Injector createTestInjector(HealthCheckConfig config) {
     Injector injector =
         Guice.createInjector(
             new HealthCheckModule(),
@@ -192,15 +218,11 @@
               @Override
               protected void configure() {
                 bind(HealthCheckConfig.class).toInstance(config);
-                bind(HealthCheckMetrics.Factory.class).toInstance(checkMetricsFactory);
+                bind(HealthCheckMetrics.Factory.class).to(DummyHealthCheckMetricsFactory.class);
+                bind(ThreadBeanProvider.class).toInstance(threadBeanProviderMock);
               }
             },
             BlockedThreadsCheck.SUB_CHECKS);
-    return new BlockedThreadsCheck(
-        injector.getInstance(ListeningExecutorService.class),
-        config,
-        checkMetricsFactory,
-        threadBeanProviderMock,
-        Providers.of(injector.getInstance(BlockedThreadsConfigurator.class)));
+    return injector;
   }
 }