diff --git a/documentation/src/docs/asciidoc/release-notes/release-notes-5.11.1.adoc b/documentation/src/docs/asciidoc/release-notes/release-notes-5.11.1.adoc index 80f58d56d747..41114a7b0230 100644 --- a/documentation/src/docs/asciidoc/release-notes/release-notes-5.11.1.adoc +++ b/documentation/src/docs/asciidoc/release-notes/release-notes-5.11.1.adoc @@ -18,6 +18,10 @@ on GitHub. * Fixed potential locking issue with `ExclusiveResource` in the `HierarchicalTestExecutorService`, which could lead to deadlocks in certain scenarios. +* `NamespacedHierarchicalStore` no longer throws an exception after it has been closed if + the store is queried via one of the `get(...)` or `getOrComputeIfAbsent(...)` methods; + however, if a `getOrComputeIfAbsent(...)` invocation results in the computation of a new + value, an exception will still be thrown. [[release-notes-5.11.1-junit-platform-deprecations-and-breaking-changes]] ==== Deprecations and Breaking Changes @@ -37,7 +41,8 @@ on GitHub. [[release-notes-5.11.1-junit-jupiter-bug-fixes]] ==== Bug Fixes -* ❓ +* `TestWatcher` callback methods can once again access data in the + `ExtensionContext.Store`. [[release-notes-5.11.1-junit-jupiter-deprecations-and-breaking-changes]] ==== Deprecations and Breaking Changes diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/store/NamespacedHierarchicalStore.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/store/NamespacedHierarchicalStore.java index 965eb9b24439..79836ac6f501 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/store/NamespacedHierarchicalStore.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/store/NamespacedHierarchicalStore.java @@ -139,7 +139,6 @@ public void close() { * closed */ public Object get(N namespace, Object key) { - rejectIfClosed(); StoredValue storedValue = getStoredValue(new CompositeKey<>(namespace, key)); return StoredValue.evaluateIfNotNull(storedValue); } @@ -156,7 +155,6 @@ public Object get(N namespace, Object key) { * be cast to the required type, or if this store has already been closed */ public T get(N namespace, Object key, Class requiredType) throws NamespacedHierarchicalStoreException { - rejectIfClosed(); Object value = get(namespace, key); return castToRequiredType(key, value, requiredType); } @@ -174,13 +172,15 @@ public T get(N namespace, Object key, Class requiredType) throws Namespac * closed */ public Object getOrComputeIfAbsent(N namespace, K key, Function defaultCreator) { - rejectIfClosed(); Preconditions.notNull(defaultCreator, "defaultCreator must not be null"); CompositeKey compositeKey = new CompositeKey<>(namespace, key); StoredValue storedValue = getStoredValue(compositeKey); if (storedValue == null) { storedValue = this.storedValues.computeIfAbsent(compositeKey, - __ -> storedValue(new MemoizingSupplier(() -> defaultCreator.apply(key)))); + __ -> storedValue(new MemoizingSupplier(() -> { + rejectIfClosed(); + return defaultCreator.apply(key); + }))); } return storedValue.evaluate(); } @@ -202,7 +202,6 @@ public Object getOrComputeIfAbsent(N namespace, K key, Function def public V getOrComputeIfAbsent(N namespace, K key, Function defaultCreator, Class requiredType) throws NamespacedHierarchicalStoreException { - rejectIfClosed(); Object value = getOrComputeIfAbsent(namespace, key, defaultCreator); return castToRequiredType(key, value, requiredType); } diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/TestWatcherTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/TestWatcherTests.java index 27ca3d2d77c9..a8400aa63ba3 100644 --- a/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/TestWatcherTests.java +++ b/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/TestWatcherTests.java @@ -12,6 +12,7 @@ import static java.util.function.Predicate.not; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.entry; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -41,8 +42,11 @@ import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.TestInstance.Lifecycle; import org.junit.jupiter.api.TestMethodOrder; +import org.junit.jupiter.api.extension.BeforeTestExecutionCallback; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.api.extension.ExtensionContext.Namespace; +import org.junit.jupiter.api.extension.ExtensionContext.Store; import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.api.extension.TestWatcher; import org.junit.jupiter.api.fixtures.TrackLogRecords; @@ -67,6 +71,7 @@ class TestWatcherTests extends AbstractJupiterTestEngineTests { @BeforeEach void clearResults() { TrackingTestWatcher.results.clear(); + DataRetrievingTestWatcher.results.clear(); } @Test @@ -167,6 +172,16 @@ void testWatcherSemanticsWhenRegisteredAtMethodLevel() { assertThat(TrackingTestWatcher.results.get("testDisabled")).containsExactly("test", "repeatedTest"); } + @Test + void testWatcherCanRetrieveDataFromTheExtensionContextStore() { + Class testClass = DataRetrievingTestWatcherTestCase.class; + EngineExecutionResults results = executeTestsForClass(testClass); + + results.testEvents().assertStatistics(stats -> stats.started(1).succeeded(1).failed(0)); + + assertThat(DataRetrievingTestWatcher.results).containsExactly(entry("key", "enigma")); + } + private void assertCommonStatistics(EngineExecutionResults results) { results.containerEvents().assertStatistics(stats -> stats.started(3).succeeded(3).failed(0)); results.testEvents().assertStatistics(stats -> stats.skipped(2).started(6).succeeded(2).aborted(2).failed(2)); @@ -414,4 +429,40 @@ public void testFailed(ExtensionContext context, Throwable cause) { } + /** + * {@link TestWatcher} that retrieves data from the {@link ExtensionContext.Store}. + * @see #3944 + */ + static class DataRetrievingTestWatcher implements BeforeTestExecutionCallback, TestWatcher { + + private static final Namespace NAMESPACE = Namespace.create(DataRetrievingTestWatcher.class); + + private static final String KEY = "key"; + + private static final Map results = new HashMap<>(); + + @Override + public void beforeTestExecution(ExtensionContext context) throws Exception { + getStore(context).put(KEY, "enigma"); + } + + @Override + public void testSuccessful(ExtensionContext context) { + results.put(KEY, getStore(context).get(KEY, String.class)); + } + + private static Store getStore(ExtensionContext context) { + return context.getStore(NAMESPACE); + } + } + + @ExtendWith(DataRetrievingTestWatcher.class) + static class DataRetrievingTestWatcherTestCase { + + @Test + public void test() { + //no-op + } + } + } diff --git a/platform-tests/src/test/java/org/junit/platform/engine/support/store/NamespacedHierarchicalStoreTests.java b/platform-tests/src/test/java/org/junit/platform/engine/support/store/NamespacedHierarchicalStoreTests.java index 9f18083a924c..f234ff11dbe3 100644 --- a/platform-tests/src/test/java/org/junit/platform/engine/support/store/NamespacedHierarchicalStoreTests.java +++ b/platform-tests/src/test/java/org/junit/platform/engine/support/store/NamespacedHierarchicalStoreTests.java @@ -519,28 +519,35 @@ void closeIsIdempotent() throws Throwable { verifyNoMoreInteractions(closeAction); } + /** + * @see #3944 + */ @Test - void rejectsModificationAfterClose() { + void acceptsQueryAfterClose() { + store.put(namespace, key, value); store.close(); assertClosed(); - assertThrows(NamespacedHierarchicalStoreException.class, () -> store.put(namespace, "key1", "value1")); - assertThrows(NamespacedHierarchicalStoreException.class, () -> store.remove(namespace, "key1")); - assertThrows(NamespacedHierarchicalStoreException.class, - () -> store.remove(namespace, "key1", Number.class)); + assertThat(store.get(namespace, key)).isEqualTo(value); + assertThat(store.get(namespace, key, String.class)).isEqualTo(value); + assertThat(store.getOrComputeIfAbsent(namespace, key, k -> "new")).isEqualTo(value); + assertThat(store.getOrComputeIfAbsent(namespace, key, k -> "new", String.class)).isEqualTo(value); } @Test - void rejectsQueryAfterClose() { + void rejectsModificationAfterClose() { store.close(); assertClosed(); - assertThrows(NamespacedHierarchicalStoreException.class, () -> store.get(namespace, "key1")); - assertThrows(NamespacedHierarchicalStoreException.class, () -> store.get(namespace, "key1", Integer.class)); + assertThrows(NamespacedHierarchicalStoreException.class, () -> store.put(namespace, key, value)); + assertThrows(NamespacedHierarchicalStoreException.class, () -> store.remove(namespace, key)); + assertThrows(NamespacedHierarchicalStoreException.class, () -> store.remove(namespace, key, int.class)); + + // Since key does not exist, an invocation of getOrComputeIfAbsent(...) will attempt to compute a new value. assertThrows(NamespacedHierarchicalStoreException.class, - () -> store.getOrComputeIfAbsent(namespace, "key1", k -> "value")); + () -> store.getOrComputeIfAbsent(namespace, key, k -> "new")); assertThrows(NamespacedHierarchicalStoreException.class, - () -> store.getOrComputeIfAbsent(namespace, "key1", k -> 1337, Integer.class)); + () -> store.getOrComputeIfAbsent(namespace, key, k -> "new", String.class)); } private void assertNotClosed() {