This is an automated email from the ASF dual-hosted git repository. reiern70 pushed a commit to branch fix-async-pagestore-test in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 60a7e1d6d529bfd343d9bd3768533eabea414341 Author: reiern70 <[email protected]> AuthorDate: Wed Sep 23 12:54:56 2026 -0500 Make the asynchronous page store context test observe its own assertions storeAsynchronousContextClosed checks what an IPageStore may do with the IPageContext it is handed once the add has moved to the page saving thread. It queued the add, then called destroy() on the delegate store rather than on the AsynchronousPageStore, so nothing ever interrupted or joined that thread. The test read its failure reference and returned while the saving thread had usually not started, and passed by never looking. That left two problems. Assertion failures raised on the saving thread are Errors, so PageAddingRunnable's catch of Exception does not hold them; they escaped onto a daemon thread that outlived the test, writing into a store the test had already destroyed. Reported at whatever point the runner noticed, this is the intermittent failure seen in CI. And because the assertions were never reached, a wrong expectation went unnoticed: the test required getSessionAttribute("key2", () -> null) to throw asynchronously. It does not, and should not. PendingAdd#getSessionAttribute throws only where a value would be changed - a missing entry whose supplier yields a value. A read of a missing key with a null default changes nothing and returns null, which is what the getSessionData block in the same test already expects. The attribute expectations now mirror it: a cached key reads back, a missing key reads null, and only an attempted set throws. No production behaviour changes. The test now counts down a latch in a finally around the asynchronous body and awaits it, so an add that never happens fails the test instead of passing it. The body records any Throwable for the test thread to rethrow, which lets the sentinel "set a marker exception, then swallow the expected one" idiom give way to assertThrows. destroy() is called on the AsynchronousPageStore, interrupting and joining the saving thread, and still reaches the delegate through DelegatingPageStore. --- .../pageStore/AsynchronousPageStoreTest.java | 75 ++++++++++++---------- 1 file changed, 41 insertions(+), 34 deletions(-) diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/pageStore/AsynchronousPageStoreTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/pageStore/AsynchronousPageStoreTest.java index 5c1577ee10..2b48e4b90f 100644 --- a/wicket-core-tests/src/test/java/org/apache/wicket/pageStore/AsynchronousPageStoreTest.java +++ b/wicket-core-tests/src/test/java/org/apache/wicket/pageStore/AsynchronousPageStoreTest.java @@ -17,6 +17,8 @@ package org.apache.wicket.pageStore; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -362,7 +364,9 @@ public class AsynchronousPageStoreTest public void storeAsynchronousContextClosed() throws Throwable { final AtomicReference<Throwable> asyncFail = new AtomicReference<>(); - + + final CountDownLatch added = new CountDownLatch(1); + IPageStore store = new MockPageStore() { @Override @@ -386,38 +390,39 @@ public class AsynchronousPageStoreTest @Override public synchronized void addPage(IPageContext context, IManageablePage page) { - // can get session id - context.getSessionId(true); - - // cannot access request - try { - context.getRequestData(KEY1, () -> null); - asyncFail.set(new Exception().fillInStackTrace()); - } catch (WicketRuntimeException expected) { - } - try { - context.getRequestData(KEY2, () -> null); - asyncFail.set(new Exception().fillInStackTrace()); - } catch (WicketRuntimeException expected) { + // an assertion failing here would die with the page saving thread + try + { + // can get session id + context.getSessionId(true); + + // cannot access request + assertThrows(WicketRuntimeException.class, + () -> context.getRequestData(KEY1, () -> null)); + assertThrows(WicketRuntimeException.class, + () -> context.getRequestData(KEY2, () -> null)); + + // can read session data + assertEquals("value1", context.getSessionData(KEY1, () -> "value2")); + assertEquals(null, context.getSessionData(KEY2, () -> null)); + // .. but cannot set + assertThrows(WicketRuntimeException.class, + () -> context.getSessionData(KEY2, () -> "value2")); + + // can read session attribute already read + assertEquals("value1", context.getSessionAttribute("key1", () -> null)); + assertNull(context.getSessionAttribute("key2", () -> null)); + // .. but cannot set + assertThrows(WicketRuntimeException.class, + () -> context.getSessionAttribute("key2", () -> "value2")); } - - // can read session data - assertEquals("value1", context.getSessionData(KEY1, () -> "value2")); - assertEquals(null, context.getSessionData(KEY2, () -> null)); - // .. but cannot set - try { - context.getSessionData(KEY2, () -> "value2"); - asyncFail.set(new Exception().fillInStackTrace()); - } catch (WicketRuntimeException expected) { + catch (Throwable failure) + { + asyncFail.set(failure); } - - // can read session attribute already read - assertEquals("value1", context.getSessionAttribute("key1", () -> null)); - // .. but nothing new - try { - context.getSessionAttribute("key2", () -> null); - asyncFail.set(new Exception().fillInStackTrace()); - } catch (WicketRuntimeException expected) { + finally + { + added.countDown(); } } }; @@ -429,9 +434,11 @@ public class AsynchronousPageStoreTest IPageContext context = new MockPageContext(); asyncPageStore.addPage(context , page); - - store.destroy(); - + + assertTrue(added.await(30, TimeUnit.SECONDS), "page was never added asynchronously"); + + asyncPageStore.destroy(); + if (asyncFail.get() != null) { throw asyncFail.get(); }
