From 4b5afaff5341c110fa5cfe4f853f7ff71f15e30e Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Mon, 31 Aug 2026 13:03:05 +0200 Subject: [PATCH] Revise "Close contexts when clearing test context cache" This commit picks up where 97067f9c9c left off by removing the four trailing collection-clearing calls in clear(), which are now redundant since the removal loop already empties contextMap, hierarchyMap, contextUsageMap, and unusedContexts as an invariant. This commit also adds regression tests in LruContextCacheTests for closing a context hierarchy via clear() and reset(), asserting bottom-up close order with Mockito.inOrder(), and confirming that getParentContextCount() and getContextUsageCount() return to zero. See gh-36825 --- .../context/cache/DefaultContextCache.java | 4 - .../context/cache/LruContextCacheTests.java | 74 ++++++++++++++++++- 2 files changed, 70 insertions(+), 8 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/context/cache/DefaultContextCache.java b/spring-test/src/main/java/org/springframework/test/context/cache/DefaultContextCache.java index e15347189ce..f30798dbad9 100644 --- a/spring-test/src/main/java/org/springframework/test/context/cache/DefaultContextCache.java +++ b/spring-test/src/main/java/org/springframework/test/context/cache/DefaultContextCache.java @@ -429,10 +429,6 @@ public class DefaultContextCache implements ContextCache { for (MergedContextConfiguration key : new ArrayList<>(this.contextMap.keySet())) { remove(key, HierarchyMode.CURRENT_LEVEL); } - this.contextMap.clear(); - this.hierarchyMap.clear(); - this.contextUsageMap.clear(); - this.unusedContexts.clear(); } } diff --git a/spring-test/src/test/java/org/springframework/test/context/cache/LruContextCacheTests.java b/spring-test/src/test/java/org/springframework/test/context/cache/LruContextCacheTests.java index c45d4b4f299..6cd99516da0 100644 --- a/spring-test/src/test/java/org/springframework/test/context/cache/LruContextCacheTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/cache/LruContextCacheTests.java @@ -27,6 +27,7 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.mockito.InOrder; import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.InitializingBean; @@ -44,6 +45,7 @@ import static org.assertj.core.api.Assertions.as; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.InstanceOfAssertFactories.map; +import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -81,17 +83,23 @@ class LruContextCacheTests { assertThatIllegalArgumentException().isThrownBy(() -> new DefaultContextCache(0)); } - @Test + @Test // gh-36825 void clearClosesContexts() { DefaultContextCache cache = new DefaultContextCache(4); cache.put(fooConfig, key -> fooContext); cache.put(barConfig, key -> barContext); cache.put(bazConfig, key -> bazContext); + cache.registerContextUsage(fooConfig, getClass()); + cache.registerContextUsage(barConfig, getClass()); + cache.registerContextUsage(bazConfig, getClass()); assertCacheContents(cache, "Foo", "Bar", "Baz"); + assertThat(cache.getContextUsageCount()).isEqualTo(3); cache.clear(); - assertCacheContents(cache); + assertThat(cache.size()).isZero(); + assertThat(cache.getParentContextCount()).isZero(); + assertThat(cache.getContextUsageCount()).isZero(); verify(fooContext, times(1)).close(); verify(barContext, times(1)).close(); @@ -99,24 +107,78 @@ class LruContextCacheTests { verify(abcContext, never()).close(); } - @Test + @Test // gh-36825 void resetClosesContexts() { DefaultContextCache cache = new DefaultContextCache(4); cache.put(fooConfig, key -> fooContext); cache.put(barConfig, key -> barContext); cache.get(fooConfig); + cache.get(abcConfig); assertThat(cache.getHitCount()).isEqualTo(1); + assertThat(cache.getMissCount()).isEqualTo(1); assertCacheContents(cache, "Bar", "Foo"); cache.reset(); - assertCacheContents(cache); + assertThat(cache.size()).isZero(); assertThat(cache.getHitCount()).isZero(); + assertThat(cache.getMissCount()).isZero(); + assertThat(cache.getParentContextCount()).isZero(); + assertThat(cache.getContextUsageCount()).isZero(); verify(fooContext, times(1)).close(); verify(barContext, times(1)).close(); } + @Test // gh-36825 + void clearClosesContextHierarchyBottomUp() { + DefaultContextCache cache = new DefaultContextCache(4); + + // Foo (parent) <- Bar (child) <- Baz (grandchild) + MergedContextConfiguration parentConfig = config(Foo.class); + MergedContextConfiguration childConfig = config(Bar.class, parentConfig); + MergedContextConfiguration grandchildConfig = config(Baz.class, childConfig); + + cache.put(parentConfig, key -> fooContext); + cache.put(childConfig, key -> barContext); + cache.put(grandchildConfig, key -> bazContext); + assertCacheContents(cache, "Foo", "Bar", "Baz"); + assertThat(cache.getParentContextCount()).isEqualTo(2); + + cache.clear(); + assertThat(cache.size()).isZero(); + assertThat(cache.getParentContextCount()).isZero(); + + // Child contexts must be closed before their parents. + InOrder inOrder = inOrder(bazContext, barContext, fooContext); + inOrder.verify(bazContext).close(); + inOrder.verify(barContext).close(); + inOrder.verify(fooContext).close(); + } + + @Test // gh-36825 + void resetClosesContextHierarchyBottomUp() { + DefaultContextCache cache = new DefaultContextCache(4); + + // Foo (parent) <- Bar (child) + MergedContextConfiguration parentConfig = config(Foo.class); + MergedContextConfiguration childConfig = config(Bar.class, parentConfig); + + cache.put(parentConfig, key -> fooContext); + cache.put(childConfig, key -> barContext); + assertCacheContents(cache, "Foo", "Bar"); + assertThat(cache.getParentContextCount()).isEqualTo(1); + + cache.reset(); + assertThat(cache.size()).isZero(); + assertThat(cache.getParentContextCount()).isZero(); + + // Child contexts must be closed before their parents. + InOrder inOrder = inOrder(barContext, fooContext); + inOrder.verify(barContext).close(); + inOrder.verify(fooContext).close(); + } + @Nested @SuppressWarnings("deprecation") @@ -522,6 +584,10 @@ class LruContextCacheTests { return new MergedContextConfiguration(null, null, new Class[] { clazz }, null, null); } + private static MergedContextConfiguration config(Class clazz, MergedContextConfiguration parent) { + return new MergedContextConfiguration(null, null, new Class[] { clazz }, null, null, null, null, parent); + } + private static void assertCacheContents(DefaultContextCache cache, String... expectedNames) { assertThat(cache).extracting("contextMap", as(map(MergedContextConfiguration.class, ApplicationContext.class))) .satisfies(contextMap -> {