More comprehensive registration/release of config in some tests

If someone calls ConfigProvider.getConfig() out of the blue in a test that doesn't use any Quarkus*Test extension, this will indirectly call QuarkusConfigFactory and leak config in two ways: 1. In QuarkusConfigFactory#config 2. In SmallRyeConfigProviderResolver, registering config for the TCCL, which in such a case is most likely the system CL. Thus, a well-behaved test should call QuarkusConfigFactory.setConfig(null) to clean up all that, no just SmallRyeConfigProviderResolver.releaseConfig(). Similarly, tests that register configuration explicitly can just call QuarkusConfigFactory.setConfig(config) at the beginning and QuarkusConfigFactory.setConfig(null) at the end, which will properly simulate how a real Quarkus application behaves, and should cover all edge cases involving multiple classloaders, properly cleaning up everything at the end of the test.
quarkusio · Oct 26, 2023 · e76a0e5 · e76a0e5
1 parent 711cb7a
commit e76a0e5
Show file tree

Hide file tree

Showing 7 changed files with 20 additions and 87 deletions.
diff --git a/...config-yaml/runtime/src/test/java/io/quarkus/config/yaml/runtime/ApplicationYamlTest.java b/...config-yaml/runtime/src/test/java/io/quarkus/config/yaml/runtime/ApplicationYamlTest.java
@@ -3,14 +3,10 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-import org.eclipse.microprofile.config.Config;
-import org.eclipse.microprofile.config.ConfigProvider;
-import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 
-import io.quarkus.runtime.configuration.QuarkusConfigFactory;
 import io.smallrye.config.SmallRyeConfig;
 import io.smallrye.config.SmallRyeConfigBuilder;
 
@@ -21,34 +17,26 @@
  */
 public class ApplicationYamlTest {
 
-    static volatile SmallRyeConfig config;
+    private static SmallRyeConfig config;
 
     @BeforeAll
     public static void doBefore() {
         SmallRyeConfigBuilder builder = new SmallRyeConfigBuilder();
         builder.addDefaultSources().addDiscoveredConverters().addDiscoveredSources();
-        QuarkusConfigFactory.setConfig(config = builder.build());
-        Config conf = ConfigProvider.getConfig();
-        if (conf != config) {
-            ConfigProviderResolver cpr = ConfigProviderResolver.instance();
-            cpr.releaseConfig(conf);
-            ConfigProvider.getConfig();
-        }
+        config = builder.build();
         System.out.println(System.getProperty("user.dir"));
     }
 
     @Test
     public void testBasicApplicationYaml() {
-        assertEquals("something", ConfigProvider.getConfig().getValue("foo.bar", String.class));
-        assertEquals("somethingElse", ConfigProvider.getConfig().getValue("foo2.bar", String.class));
-        assertEquals("other", ConfigProvider.getConfig().getValue("foo.baz", String.class));
-        assertTrue(ConfigProvider.getConfig().getValue("file.system", Boolean.class).booleanValue());
+        assertEquals("something", config.getValue("foo.bar", String.class));
+        assertEquals("somethingElse", config.getValue("foo2.bar", String.class));
+        assertEquals("other", config.getValue("foo.baz", String.class));
+        assertTrue(config.getValue("file.system", Boolean.class).booleanValue());
     }
 
     @AfterAll
     public static void doAfter() {
-        ConfigProviderResolver cpr = ConfigProviderResolver.instance();
-        cpr.releaseConfig(config);
         config = null;
     }
 }
diff --git a/...lient-config/runtime/src/test/java/io/quarkus/restclient/config/RestClientConfigTest.java b/...lient-config/runtime/src/test/java/io/quarkus/restclient/config/RestClientConfigTest.java
@@ -6,38 +6,19 @@
 import java.net.URL;
 import java.util.Optional;
 
-import org.eclipse.microprofile.config.Config;
-import org.eclipse.microprofile.config.ConfigProvider;
-import org.eclipse.microprofile.config.spi.ConfigBuilder;
-import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
 import org.eclipse.microprofile.rest.client.ext.QueryParamStyle;
-import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 
 import io.smallrye.config.PropertiesConfigSource;
+import io.smallrye.config.SmallRyeConfig;
+import io.smallrye.config.SmallRyeConfigBuilder;
 
 public class RestClientConfigTest {
 
-    private static ClassLoader classLoader;
-    private static ConfigProviderResolver configProviderResolver;
-
-    @BeforeAll
-    public static void initConfig() {
-        classLoader = Thread.currentThread().getContextClassLoader();
-        configProviderResolver = ConfigProviderResolver.instance();
-    }
-
-    @AfterEach
-    public void releaseConfig() {
-        configProviderResolver.releaseConfig(configProviderResolver.getConfig());
-    }
-
     @Test
     public void testLoadRestClientConfig() throws IOException {
-        setupMPConfig();
+        SmallRyeConfig config = createMPConfig();
 
-        Config config = ConfigProvider.getConfig();
         Optional<String> optionalValue = config.getOptionalValue("quarkus.rest-client.test-client.url", String.class);
         assertThat(optionalValue).isPresent();
 
@@ -75,11 +56,11 @@ private void verifyConfig(RestClientConfig config) {
         assertThat(config.multipart.maxChunkSize.get()).isEqualTo(1024);
     }
 
-    private static void setupMPConfig() throws IOException {
-        ConfigBuilder configBuilder = configProviderResolver.getBuilder();
+    private static SmallRyeConfig createMPConfig() throws IOException {
+        SmallRyeConfigBuilder configBuilder = new SmallRyeConfigBuilder().addDefaultInterceptors();
         URL propertyFile = RestClientConfigTest.class.getClassLoader().getResource("application.properties");
         assertThat(propertyFile).isNotNull();
         configBuilder.withSources(new PropertiesConfigSource(propertyFile));
-        configProviderResolver.registerConfig(configBuilder.build(), classLoader);
+        return configBuilder.build();
     }
 }
diff --git a/...c/rest-client/runtime/src/test/java/io/quarkus/restclient/runtime/RestClientBaseTest.java b/...c/rest-client/runtime/src/test/java/io/quarkus/restclient/runtime/RestClientBaseTest.java
@@ -21,13 +21,10 @@
 import jakarta.ws.rs.client.ClientResponseContext;
 import jakarta.ws.rs.client.ClientResponseFilter;
 
-import org.eclipse.microprofile.config.Config;
-import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
 import org.eclipse.microprofile.rest.client.RestClientBuilder;
 import org.eclipse.microprofile.rest.client.ext.QueryParamStyle;
 import org.eclipse.microprofile.rest.client.inject.RegisterRestClient;
 import org.junit.jupiter.api.AfterAll;
-import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
@@ -42,7 +39,6 @@ public class RestClientBaseTest {
 
     private static Path truststorePath;
     private static Path keystorePath;
-    private static Config createdConfig;
 
     @BeforeAll
     public static void beforeAll() throws IOException, KeyStoreException, CertificateException, NoSuchAlgorithmException {
@@ -65,14 +61,6 @@ public static void beforeAll() throws IOException, KeyStoreException, Certificat
         }
     }
 
-    @AfterEach
-    public void afterEach() {
-        if (createdConfig != null) {
-            ConfigProviderResolver.instance().releaseConfig(createdConfig);
-            createdConfig = null;
-        }
-    }
-
     @AfterAll
     public static void afterAll() {
         if (truststorePath != null) {

diff --git a/...c/test/java/io/quarkus/rest/client/reactive/runtime/RestClientCDIDelegateBuilderTest.java b/...c/test/java/io/quarkus/rest/client/reactive/runtime/RestClientCDIDelegateBuilderTest.java
@@ -16,12 +16,9 @@
 import jakarta.ws.rs.client.ClientResponseContext;
 import jakarta.ws.rs.client.ClientResponseFilter;
 
-import org.eclipse.microprofile.config.Config;
-import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
 import org.eclipse.microprofile.rest.client.ext.QueryParamStyle;
 import org.eclipse.microprofile.rest.client.inject.RegisterRestClient;
 import org.jboss.resteasy.reactive.client.api.QuarkusRestClientProperties;
-import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
@@ -42,7 +39,6 @@ public class RestClientCDIDelegateBuilderTest {
     static File tempDir;
     private static File truststoreFile;
     private static File keystoreFile;
-    private static Config createdConfig;
 
     @BeforeAll
     public static void beforeAll() throws IOException, KeyStoreException, CertificateException, NoSuchAlgorithmException {
@@ -60,14 +56,6 @@ public static void beforeAll() throws IOException, KeyStoreException, Certificat
         keystore.store(new FileOutputStream(keystoreFile), KEYSTORE_PASSWORD.toCharArray());
     }
 
-    @AfterEach
-    public void afterEach() {
-        if (createdConfig != null) {
-            ConfigProviderResolver.instance().releaseConfig(createdConfig);
-            createdConfig = null;
-        }
-    }
-
     @Test
     public void testClientSpecificConfigs() {
         // given

diff --git a/...ve/src/test/java/io/quarkus/it/hibernate/validator/ConfigMappingStartupValidatorTest.java b/...ve/src/test/java/io/quarkus/it/hibernate/validator/ConfigMappingStartupValidatorTest.java
@@ -11,9 +11,6 @@
 import jakarta.inject.Inject;
 import jakarta.validation.constraints.Pattern;
 
-import org.eclipse.microprofile.config.Config;
-import org.eclipse.microprofile.config.ConfigProvider;
-import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
@@ -88,19 +85,12 @@ public void validateMapping(Class<?> mappingClass, String prefix, Object mapping
                     }
                 });
         QuarkusConfigFactory.setConfig(smallryeConfig = builder.build());
-        Config conf = ConfigProvider.getConfig();
-        if (conf != smallryeConfig) {
-            ConfigProviderResolver cpr = ConfigProviderResolver.instance();
-            cpr.releaseConfig(conf);
-            ConfigProvider.getConfig();
-        }
         suppressedConfigValidatorExceptions = new HashMap<>();
     }
 
     @AfterAll
     public static void doAfter() {
-        ConfigProviderResolver cpr = ConfigProviderResolver.instance();
-        cpr.releaseConfig(smallryeConfig);
+        QuarkusConfigFactory.setConfig(null);
         smallryeConfig = null;
         suppressedConfigValidatorExceptions = null;
     }

diff --git a/test-framework/common/src/main/java/io/quarkus/test/common/LauncherUtil.java b/test-framework/common/src/main/java/io/quarkus/test/common/LauncherUtil.java
@@ -22,7 +22,6 @@
 import java.util.regex.Pattern;
 
 import org.eclipse.microprofile.config.Config;
-import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
 
 import io.quarkus.runtime.LaunchMode;
 import io.quarkus.runtime.configuration.ConfigUtils;
@@ -41,14 +40,6 @@ private LauncherUtil() {
     public static Config installAndGetSomeConfig() {
         final SmallRyeConfig config = ConfigUtils.configBuilder(false, LaunchMode.NORMAL).build();
         QuarkusConfigFactory.setConfig(config);
-        final ConfigProviderResolver cpr = ConfigProviderResolver.instance();
-        try {
-            final Config installed = cpr.getConfig();
-            if (installed != config) {
-                cpr.releaseConfig(installed);
-            }
-        } catch (IllegalStateException ignored) {
-        }
         return config;
     }
 

diff --git a/test-framework/common/src/main/java/io/quarkus/test/common/TestResourceManager.java b/test-framework/common/src/main/java/io/quarkus/test/common/TestResourceManager.java
@@ -179,6 +179,13 @@ public void close() {
                 throw new RuntimeException("Unable to stop Quarkus test resource " + entry.getTestResource(), e);
             }
         }
+        // TODO using QuarkusConfigFactory.setConfig(null) here makes continuous testing fail,
+        //   e.g. in io.quarkus.hibernate.orm.HibernateHotReloadTestCase
+        //   or io.quarkus.opentelemetry.deployment.OpenTelemetryContinuousTestingTest;
+        //   maybe this cleanup is not really necessary and just "doesn't hurt" because
+        //   the released config is still cached in QuarkusConfigFactory#config
+        //   and will be restored soon after when QuarkusConfigFactory#getConfigFor is called?
+        //   In that case we should remove this cleanup.
         try {
             ((SmallRyeConfigProviderResolver) SmallRyeConfigProviderResolver.instance())
                     .releaseConfig(Thread.currentThread().getContextClassLoader());