Skip to content

Commit 458325c

Browse files
committed
Log only once per implementation type for CloseableResource types
To avoid flooding console output with warnings. Resolves #4776. (cherry picked from commit 8e97a8b)
1 parent 976a110 commit 458325c

File tree

3 files changed

+33
-24
lines changed

3 files changed

+33
-24
lines changed

documentation/src/docs/asciidoc/release-notes/release-notes-5.13.4.adoc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ repository on GitHub.
5858
[[release-notes-5.13.4-junit-jupiter-new-features-and-improvements]]
5959
==== New Features and Improvements
6060

61-
* ❓
61+
* Log only once per implementation type for `CloseableResource` implementations that do
62+
not implement `AutoCloseable` to avoid flooding console output with this warning.
6263

6364

6465
[[release-notes-5.13.4-junit-vintage]]

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/AbstractExtensionContext.java

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@
5353
abstract class AbstractExtensionContext<T extends TestDescriptor> implements ExtensionContextInternal, AutoCloseable {
5454

5555
private static final Logger LOGGER = LoggerFactory.getLogger(AbstractExtensionContext.class);
56+
private static final Namespace CLOSEABLE_RESOURCE_LOGGING_NAMESPACE = Namespace.create(
57+
AbstractExtensionContext.class, "CloseableResourceLogging");
5658

5759
private final ExtensionContext parent;
5860
private final EngineExecutionListener engineExecutionListener;
@@ -85,42 +87,39 @@ abstract class AbstractExtensionContext<T extends TestDescriptor> implements Ext
8587
.collect(collectingAndThen(toCollection(LinkedHashSet::new), Collections::unmodifiableSet));
8688
// @formatter:on
8789

88-
this.valuesStore = createStore(parent, launcherStoreFacade, createCloseAction());
90+
this.valuesStore = new NamespacedHierarchicalStore<>(getParentStore(parent), createCloseAction());
91+
}
92+
93+
private NamespacedHierarchicalStore<org.junit.platform.engine.support.store.Namespace> getParentStore(
94+
ExtensionContext parent) {
95+
return parent == null //
96+
? this.launcherStoreFacade.getRequestLevelStore() //
97+
: ((AbstractExtensionContext<?>) parent).valuesStore;
8998
}
9099

91100
@SuppressWarnings("deprecation")
92-
private NamespacedHierarchicalStore.CloseAction<org.junit.platform.engine.support.store.Namespace> createCloseAction() {
101+
private <N> NamespacedHierarchicalStore.CloseAction<N> createCloseAction() {
102+
Store store = this.launcherStoreFacade.getSessionLevelStore(CLOSEABLE_RESOURCE_LOGGING_NAMESPACE);
93103
return (__, ___, value) -> {
94104
boolean isAutoCloseEnabled = this.configuration.isClosingStoredAutoCloseablesEnabled();
95105

96-
if (value instanceof AutoCloseable && isAutoCloseEnabled) {
106+
if (isAutoCloseEnabled && value instanceof AutoCloseable) {
97107
((AutoCloseable) value).close();
98108
return;
99109
}
100110

101111
if (value instanceof Store.CloseableResource) {
102112
if (isAutoCloseEnabled) {
103-
LOGGER.warn(
104-
() -> "Type implements CloseableResource but not AutoCloseable: " + value.getClass().getName());
113+
store.getOrComputeIfAbsent(value.getClass(), type -> {
114+
LOGGER.warn(() -> "Type implements CloseableResource but not AutoCloseable: " + type.getName());
115+
return true;
116+
});
105117
}
106118
((Store.CloseableResource) value).close();
107119
}
108120
};
109121
}
110122

111-
private static NamespacedHierarchicalStore<org.junit.platform.engine.support.store.Namespace> createStore(
112-
ExtensionContext parent, LauncherStoreFacade launcherStoreFacade,
113-
NamespacedHierarchicalStore.CloseAction<org.junit.platform.engine.support.store.Namespace> closeAction) {
114-
NamespacedHierarchicalStore<org.junit.platform.engine.support.store.Namespace> parentStore;
115-
if (parent == null) {
116-
parentStore = launcherStoreFacade.getRequestLevelStore();
117-
}
118-
else {
119-
parentStore = ((AbstractExtensionContext<?>) parent).valuesStore;
120-
}
121-
return new NamespacedHierarchicalStore<>(parentStore, closeAction);
122-
}
123-
124123
@Override
125124
public void close() {
126125
this.valuesStore.close();

jupiter-tests/src/test/java/org/junit/jupiter/engine/descriptor/ResourceAutoClosingTests.java

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,19 +68,28 @@ void shouldNotCloseAutoCloseableWhenIsClosingStoredAutoCloseablesEnabledIsFalse(
6868
void shouldLogWarningWhenResourceImplementsCloseableResourceButNotAutoCloseableAndConfigIsTrue(
6969
@TrackLogRecords LogRecordListener listener) throws Exception {
7070
ExecutionRecorder executionRecorder = new ExecutionRecorder();
71-
CloseableResource resource = new CloseableResource();
72-
String msg = "Type implements CloseableResource but not AutoCloseable: " + resource.getClass().getName();
71+
CloseableResource resource1 = new CloseableResource();
72+
CloseableResource resource2 = new CloseableResource();
73+
CloseableResource resource3 = new CloseableResource() {
74+
};
7375
when(configuration.isClosingStoredAutoCloseablesEnabled()).thenReturn(true);
7476

7577
ExtensionContext extensionContext = new JupiterEngineExtensionContext(executionRecorder, testDescriptor,
7678
configuration, extensionRegistry, launcherStoreFacade);
7779
ExtensionContext.Store store = extensionContext.getStore(ExtensionContext.Namespace.GLOBAL);
78-
store.put("resource", resource);
80+
store.put("resource1", resource1);
81+
store.put("resource2", resource2);
82+
store.put("resource3", resource3);
7983

8084
((AutoCloseable) extensionContext).close();
8185

82-
assertThat(listener.stream(Level.WARNING)).map(LogRecord::getMessage).contains(msg);
83-
assertThat(resource.closed).isTrue();
86+
assertThat(listener.stream(Level.WARNING)).map(LogRecord::getMessage) //
87+
.containsExactlyInAnyOrder(
88+
"Type implements CloseableResource but not AutoCloseable: " + resource1.getClass().getName(),
89+
"Type implements CloseableResource but not AutoCloseable: " + resource3.getClass().getName());
90+
assertThat(resource1.closed).isTrue();
91+
assertThat(resource2.closed).isTrue();
92+
assertThat(resource3.closed).isTrue();
8493
}
8594

8695
@Test

0 commit comments

Comments
 (0)