diff --git a/core/src/main/java/org/apache/druid/metadata/EnvironmentVariableDynamicConfigProvider.java b/core/src/main/java/org/apache/druid/metadata/EnvironmentVariableDynamicConfigProvider.java index 629c748cf88..560ebebb46a 100644 --- a/core/src/main/java/org/apache/druid/metadata/EnvironmentVariableDynamicConfigProvider.java +++ b/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 map = new HashMap<>(); for (Map.Entry 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() { diff --git a/core/src/test/java/org/apache/druid/java/util/common/RetryUtilsTest.java b/core/src/test/java/org/apache/druid/java/util/common/RetryUtilsTest.java index e4fc226f3ce..7df0aa0687c 100644 --- a/core/src/test/java/org/apache/druid/java/util/common/RetryUtilsTest.java +++ b/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 IS_TRANSIENT = new Predicate() - { - @Override - public boolean apply(Throwable e) - { - return e instanceof IOException && e.getMessage().equals("what"); - } - }; - - @Rule - public ExpectedException expectedException = ExpectedException.none(); + private static final Predicate 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(); diff --git a/core/src/test/java/org/apache/druid/java/util/emitter/core/HttpEmitterTest.java b/core/src/test/java/org/apache/druid/java/util/emitter/core/HttpEmitterTest.java index 6591849e04d..b589455b4d8 100644 --- a/core/src/test/java/org/apache/druid/java/util/emitter/core/HttpEmitterTest.java +++ b/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))); } } diff --git a/core/src/test/java/org/apache/druid/java/util/metrics/ClockDriftSafeMonitorSchedulerTest.java b/core/src/test/java/org/apache/druid/java/util/metrics/ClockDriftSafeMonitorSchedulerTest.java index b5e80a2ea6c..c270b148b30 100644 --- a/core/src/test/java/org/apache/druid/java/util/metrics/ClockDriftSafeMonitorSchedulerTest.java +++ b/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 diff --git a/core/src/test/java/org/apache/druid/metadata/EnvironmentVariableDynamicConfigProviderTest.java b/core/src/test/java/org/apache/druid/metadata/EnvironmentVariableDynamicConfigProviderTest.java index 94f408d530e..5d3ef158cf7 100644 --- a/core/src/test/java/org/apache/druid/metadata/EnvironmentVariableDynamicConfigProviderTest.java +++ b/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 CHANGED_ENV_MAP = new HashMap<>(); - - @BeforeClass - public static void setupTest() throws Exception - { - Map oldEnvMap = getENVMap(); - Map addEnvMap = ImmutableMap.of("DRUID_USER", "druid", "DRUID_PASSWORD", "123"); - for (Map.Entry 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 oldEnvMap = getENVMap(); - for (Map.Entry 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 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 getENVMap() throws Exception - { - Map envMap = null; - Class[] classes = Collections.class.getDeclaredClasses(); - Map 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) object; + Map 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")); } }