Skip to content

Commit

Permalink
Stronger safeguards against leaked config in internal Quarkus*Test ex…
Browse files Browse the repository at this point in the history
…tensions

If someone calls ConfigProvider.getConfig() out of the blue in a test
that doesn't use any Quarkus*Test extension, this will 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.

A well-behaved test would call QuarkusConfigFactory.setConfig(null) to
clean up all that, but it's easy to miss and there is potential for
ConfigProvider.getConfig() being called indirectly, so there's no way we
can guarantee all tests are well-behaved.

This should at least guarantee that after a badly behaving test
executes, the next test using a Quarkus*Test extension will clean up the
mess.
  • Loading branch information
yrodiere committed Oct 26, 2023
1 parent b1a4ab8 commit 711cb7a
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package io.quarkus.test;

import io.quarkus.runtime.configuration.QuarkusConfigFactory;

public final class ConfigUtil {
private ConfigUtil() {
}

/**
* Clean up config left over from badly-behaving previous tests.
* <p>
* Tests may call ConfigProvider.getConfig() directly or indirectly,
* triggering lazy initialization in QuarkusConfigFactory#getConfigFor,
* and if they don't call QuarkusConfigFactory.setConfig(null) afterward,
* they may leak that config.
* As it's quite hard to guarantee that all tests clean up after them,
* especially if they don't use any Quarkus*Test extensions,
* we call this cleanup here in Quarkus*Test extensions as an additional safeguard.
* <p>
* Note this must be called quite early in extensions in order to be effective:
* things like TestHTTPResourceManager#getUri make use of config and may be called very early,
* upon test instantiation, so cleaning up in beforeEach() won't cut it for example.
*/
public static void cleanUp() {
try {
QuarkusConfigFactory.setConfig(null);
} catch (Throwable ignored) {
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ public Object createTestInstance(TestInstanceFactoryContext factoryContext, Exte

@Override
public void beforeAll(ExtensionContext context) throws Exception {
ConfigUtil.cleanUp();
GroovyClassValue.disable();
//set the right launch mode in the outer CL, used by the HTTP host config source
ProfileManager.setLaunchMode(LaunchMode.DEVELOPMENT);
Expand Down Expand Up @@ -324,6 +325,7 @@ public void afterAll(ExtensionContext context) throws Exception {
inMemoryLogHandler.clearRecords();
inMemoryLogHandler.setFilter(null);
ClearCache.clearAnnotationCache();
ConfigUtil.cleanUp();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,7 @@ private JavaArchive getArchiveProducerOrDefault() {

@Override
public void beforeAll(ExtensionContext extensionContext) throws Exception {
ConfigUtil.cleanUp();
ensureNoInjectAnnotationIsUsed(extensionContext.getRequiredTestClass());
ExclusivityChecker.checkTestType(extensionContext, QuarkusProdModeTest.class);

Expand Down Expand Up @@ -757,6 +758,8 @@ public void afterAll(ExtensionContext extensionContext) throws Exception {
if ((outputDir != null) && !preventOutputDirCleanup) {
FileUtil.deleteDirectory(outputDir);
}

ConfigUtil.cleanUp();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,7 @@ private void runExtensionMethod(ReflectiveInvocationContext<Method> invocationCo

@Override
public void beforeAll(ExtensionContext extensionContext) throws Exception {
ConfigUtil.cleanUp();
GroovyClassValue.disable();
//set the right launch mode in the outer CL, used by the HTTP host config source
ProfileManager.setLaunchMode(LaunchMode.TEST);
Expand Down Expand Up @@ -766,6 +767,7 @@ public void afterAll(ExtensionContext extensionContext) throws Exception {
afterAllCustomizer.run();
}
ClearCache.clearAnnotationCache();
ConfigUtil.cleanUp();
}
if (records != null) {
assertLogRecords.accept(records);
Expand Down

0 comments on commit 711cb7a

Please sign in to comment.