improve test compatibility with Java 17 and remove deprecated methods (#12341)

* remove use of reflection in EnvironmentVariableDynamicConfigProvider for Java 17 compatibility
* fix mocks mock objects not getting closed properly, causing issues with Java 17
* remove use of deprecated methods and rules in tests

core/src/main/java/org/apache/druid/metadata/EnvironmentVariableDynamicConfigProvider.java

@@ -21,6 +21,7 @@ package org.apache.druid.metadata;
 
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
 
@@ -51,11 +52,17 @@ public class EnvironmentVariableDynamicConfigProvider implements DynamicConfigPr
   {
     HashMap<String, String> map = new HashMap<>();
     for (Map.Entry<String, String> entry : variables.entrySet()) {
-      map.put(entry.getKey(), System.getenv(entry.getValue()));
+      map.put(entry.getKey(), getEnv(entry.getValue()));
     }
     return map;
   }
 
+  @VisibleForTesting
+  protected String getEnv(String var)
+  {
+    return System.getenv(var);
+  }
+
   @Override
   public String toString()
   {

core/src/test/java/org/apache/druid/java/util/common/RetryUtilsTest.java

@@ -24,11 +24,8 @@ import com.google.common.base.Predicates;
 import org.apache.commons.lang3.mutable.MutableInt;
 import org.apache.druid.java.util.RetryableException;
 import org.apache.druid.java.util.common.concurrent.Execs;
-import org.hamcrest.CoreMatchers;
 import org.junit.Assert;
-import org.junit.Rule;
 import org.junit.Test;
-import org.junit.rules.ExpectedException;
 
 import java.io.IOException;
 import java.util.concurrent.ExecutionException;
@@ -38,17 +35,8 @@ import java.util.concurrent.atomic.AtomicInteger;
 
 public class RetryUtilsTest
 {
-  private static final Predicate<Throwable> IS_TRANSIENT = new Predicate<Throwable>()
-  {
-    @Override
-    public boolean apply(Throwable e)
-    {
-      return e instanceof IOException && e.getMessage().equals("what");
-    }
-  };
-
-  @Rule
-  public ExpectedException expectedException = ExpectedException.none();
+  private static final Predicate<Throwable> IS_TRANSIENT =
+      e -> e instanceof IOException && e.getMessage().equals("what");
 
   @Test
   public void testImmediateSuccess() throws Exception
@@ -108,11 +96,10 @@ public class RetryUtilsTest
   }
 
   @Test
-  public void testExceptionPredicateNotMatching() throws Exception
+  public void testExceptionPredicateNotMatching()
   {
     final AtomicInteger count = new AtomicInteger();
-    boolean threwExpectedException = false;
-    try {
+    Assert.assertThrows("uhh", IOException.class, () -> {
       RetryUtils.retry(
           () -> {
             if (count.incrementAndGet() >= 2) {
@@ -124,16 +111,12 @@ public class RetryUtilsTest
           IS_TRANSIENT,
           3
       );
-    }
-    catch (IOException e) {
-      threwExpectedException = e.getMessage().equals("uhh");
-    }
-    Assert.assertTrue("threw expected exception", threwExpectedException);
+    });
     Assert.assertEquals("count", 1, count.get());
   }
 
   @Test(timeout = 5000L)
-  public void testInterruptWhileSleepingBetweenTries() throws ExecutionException, InterruptedException
+  public void testInterruptWhileSleepingBetweenTries()
   {
     ExecutorService exec = Execs.singleThreaded("test-interrupt");
     try {
@@ -150,10 +133,7 @@ public class RetryUtilsTest
           Integer.MAX_VALUE
       ));
 
-      expectedException.expect(ExecutionException.class);
-      expectedException.expectCause(CoreMatchers.instanceOf(InterruptedException.class));
-      expectedException.expectMessage("sleep interrupted");
-      future.get();
+      Assert.assertThrows("sleep interrupted", ExecutionException.class, future::get);
     }
     finally {
       exec.shutdownNow();
@@ -161,7 +141,7 @@ public class RetryUtilsTest
   }
 
   @Test(timeout = 5000L)
-  public void testInterruptRetryLoop() throws ExecutionException, InterruptedException
+  public void testInterruptRetryLoop()
   {
     ExecutorService exec = Execs.singleThreaded("test-interrupt");
     try {
@@ -181,10 +161,7 @@ public class RetryUtilsTest
           true
       ));
 
-      expectedException.expect(ExecutionException.class);
-      expectedException.expectCause(CoreMatchers.instanceOf(RuntimeException.class));
-      expectedException.expectMessage("Current thread is interrupted after [2] tries");
-      future.get();
+      Assert.assertThrows("Current thread is interrupted after [2] tries", ExecutionException.class, future::get);
     }
     finally {
       exec.shutdownNow();

core/src/test/java/org/apache/druid/java/util/emitter/core/HttpEmitterTest.java

@@ -24,8 +24,8 @@ import com.google.common.primitives.Ints;
 import org.asynchttpclient.ListenableFuture;
 import org.asynchttpclient.Request;
 import org.asynchttpclient.Response;
+import org.hamcrest.MatcherAssert;
 import org.hamcrest.Matchers;
-import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -79,7 +79,7 @@ public class HttpEmitterTest
     emitter.emitAndReturnBatch(new IntEvent());
     emitter.flush();
     long fillTimeMs = System.currentTimeMillis() - startMs;
-    Assert.assertThat((double) timeoutUsed.get(), Matchers.lessThan(fillTimeMs * (timeoutAllowanceFactor + 0.5)));
+    MatcherAssert.assertThat((double) timeoutUsed.get(), Matchers.lessThan(fillTimeMs * (timeoutAllowanceFactor + 0.5)));
 
     startMs = System.currentTimeMillis();
     final Batch batch = emitter.emitAndReturnBatch(new IntEvent());
@@ -87,6 +87,6 @@ public class HttpEmitterTest
     batch.seal();
     emitter.flush();
     fillTimeMs = System.currentTimeMillis() - startMs;
-    Assert.assertThat((double) timeoutUsed.get(), Matchers.lessThan(fillTimeMs * (timeoutAllowanceFactor + 0.5)));
+    MatcherAssert.assertThat((double) timeoutUsed.get(), Matchers.lessThan(fillTimeMs * (timeoutAllowanceFactor + 0.5)));
   }
 }

core/src/test/java/org/apache/druid/java/util/metrics/ClockDriftSafeMonitorSchedulerTest.java

@@ -58,18 +58,20 @@ public class ClockDriftSafeMonitorSchedulerTest
   private ExecutorService cronTaskRunner;
   @Mock
   private CronScheduler cronScheduler;
-  
+  private AutoCloseable mocks;
+
   @Before
   public void setUp()
   {
     cronTaskRunner = Execs.singleThreaded("monitor-scheduler-test");
-    MockitoAnnotations.initMocks(this);
+    mocks = MockitoAnnotations.openMocks(this);
   }
 
   @After
-  public void tearDown()
+  public void tearDown() throws Exception
   {
     cronTaskRunner.shutdownNow();
+    mocks.close();
   }
   
   @Test

core/src/test/java/org/apache/druid/metadata/EnvironmentVariableDynamicConfigProviderTest.java

@@ -21,89 +21,48 @@ package org.apache.druid.metadata;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.collect.ImmutableMap;
-import org.junit.AfterClass;
 import org.junit.Assert;
-import org.junit.BeforeClass;
 import org.junit.Test;
 
 import java.io.IOException;
-import java.lang.reflect.Field;
-import java.util.Collections;
-import java.util.HashMap;
 import java.util.Map;
 
 public class EnvironmentVariableDynamicConfigProviderTest
 {
-  private static final ObjectMapper JSON_MAPPER = new ObjectMapper();
-  private static final Map<String, String> CHANGED_ENV_MAP = new HashMap<>();
-
-  @BeforeClass
-  public static void setupTest() throws Exception
-  {
-    Map<String, String> oldEnvMap = getENVMap();
-    Map<String, String> addEnvMap = ImmutableMap.of("DRUID_USER", "druid", "DRUID_PASSWORD", "123");
-    for (Map.Entry<String, String> entry : addEnvMap.entrySet()) {
-      CHANGED_ENV_MAP.put(entry.getKey(), oldEnvMap.get(entry.getKey()));
-      oldEnvMap.put(entry.getKey(), entry.getValue());
-    }
-  }
-
-  @AfterClass
-  public static void tearDownTest() throws Exception
-  {
-    Map<String, String> oldEnvMap = getENVMap();
-    for (Map.Entry<String, String> entry : CHANGED_ENV_MAP.entrySet()) {
-      if (entry.getValue() == null) {
-        oldEnvMap.remove(entry.getKey());
-      } else {
-        oldEnvMap.put(entry.getKey(), entry.getValue());
-      }
-    }
-  }
-
   @Test
   public void testSerde() throws IOException
   {
+    final ObjectMapper objectMapper = new ObjectMapper();
+
     String providerString = "{\"type\": \"environment\", \"variables\" : {\"testKey\":\"testValue\"}}";
-    DynamicConfigProvider provider = JSON_MAPPER.readValue(providerString, DynamicConfigProvider.class);
+    DynamicConfigProvider provider = objectMapper.readValue(providerString, DynamicConfigProvider.class);
     Assert.assertTrue(provider instanceof EnvironmentVariableDynamicConfigProvider);
     Assert.assertEquals("testValue", ((EnvironmentVariableDynamicConfigProvider) provider).getVariables().get("testKey"));
-    DynamicConfigProvider serde = JSON_MAPPER.readValue(JSON_MAPPER.writeValueAsString(provider), DynamicConfigProvider.class);
+    DynamicConfigProvider serde = objectMapper.readValue(objectMapper.writeValueAsString(provider), DynamicConfigProvider.class);
     Assert.assertEquals(provider, serde);
   }
 
   @Test
-  public void testGetConfig() throws Exception
+  public void testGetConfig()
   {
-    String providerString = "{\"type\": \"environment\", \"variables\" : {\"user\":\"DRUID_USER\",\"password\":\"DRUID_PASSWORD\"}}";
-    DynamicConfigProvider provider = JSON_MAPPER.readValue(providerString, DynamicConfigProvider.class);
-    Assert.assertTrue(provider instanceof EnvironmentVariableDynamicConfigProvider);
-    Assert.assertEquals("druid", ((EnvironmentVariableDynamicConfigProvider) provider).getConfig().get("user"));
-    Assert.assertEquals("123", ((EnvironmentVariableDynamicConfigProvider) provider).getConfig().get("password"));
-  }
+    final ImmutableMap<String, String> env = ImmutableMap.of(
+        "DRUID_USER",
+        "druid",
+        "DRUID_PASSWORD",
+        "123"
+    );
 
-  /**
-   * This method use reflection to get system environment variables map in runtime JVM
-   * which can be changed.
-   *
-   * @return system environment variables map.
-   */
-  private static Map<String, String> getENVMap() throws Exception
-  {
-    Map<String, String> envMap = null;
-    Class[] classes = Collections.class.getDeclaredClasses();
-    Map<String, String> systemEnv = System.getenv();
-    for (Class cl : classes) {
-      if ("java.util.Collections$UnmodifiableMap".equals(cl.getName())) {
-        Field field = cl.getDeclaredField("m");
-        field.setAccessible(true);
-        Object object = field.get(systemEnv);
-        envMap = (Map<String, String>) object;
+    Map<String, String> config = ImmutableMap.of("user", "DRUID_USER", "password", "DRUID_PASSWORD");
+    EnvironmentVariableDynamicConfigProvider provider = new EnvironmentVariableDynamicConfigProvider(config)
+    {
+      @Override
+      protected String getEnv(String var)
+      {
+        return env.containsKey(var) ? env.get(var) : super.getEnv(var);
       }
-    }
-    if (envMap == null) {
-      throw new RuntimeException("Failed to get environment map.");
-    }
-    return envMap;
+    };
+
+    Assert.assertEquals("druid", provider.getConfig().get("user"));
+    Assert.assertEquals("123", provider.getConfig().get("password"));
   }
 }