This is an automated email from the ASF dual-hosted git repository. jonnybot pushed a commit to branch fix-parallel-test in repository https://gitbox.apache.org/repos/asf/groovy-geb.git
commit 164c64a287553f34f7687b7fb59b875b199762e2 Author: Jonny Carter <[email protected]> AuthorDate: Wed Dec 31 11:15:12 2025 -0600 Remove cache clear in BeforeAll/AfterAll methods It is possible for multiple BeforeAll and AfterAll lifecycle methods in JUnit to get called on the same thread as a running test, causing cached drivers to get quit while still in use. This change adds a documented warning to that API and prevents our test of JUnit 5 parallelism from falling into that trap. For context, see related issues and PRs: * https://github.com/apache/groovy-geb/pull/181 * https://github.com/apache/groovy-geb/issues/188 * https://github.com/apache/groovy-geb/issues/288 * https://github.com/apache/groovy-geb/pull/299 * https://github.com/apache/groovy-geb/pull/301 --- doc/manual/src/docs/asciidoc/021-driver.adoc | 15 +++++++++++++++ .../main/groovy/geb/driver/CachingDriverFactory.groovy | 12 ++++++++++++ .../test/groovy/geb/junit5/ParallelExecutionTest.groovy | 6 ------ 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/doc/manual/src/docs/asciidoc/021-driver.adoc b/doc/manual/src/docs/asciidoc/021-driver.adoc index c4352b44..c988a2f7 100644 --- a/doc/manual/src/docs/asciidoc/021-driver.adoc +++ b/doc/manual/src/docs/asciidoc/021-driver.adoc @@ -57,6 +57,21 @@ After calling any of these methods, the next request for a default driver will r This caching behavior is <<driver-caching-configuration,configurable>>. +[WARNING] +==== +Clearing the webdriver cache in this way is generally safe, though there is a known gotcha for JUnit 5 tests running in parallel. +If you have multiple test classes running in parallel and call `{clear-browser-cache-api}` or `{clear-browser-cache-and-quit-api}` in methods annotated with link:https://docs.junit.org/5.0.1/api/org/junit/jupiter/api/BeforeAll.html[@BeforeAll] or link:https://docs.junit.org/5.0.1/api/org/junit/jupiter/api/AfterAll.html[@AfterAll] the tests may fail intermittently with errors such as: + +* `org.openqa.selenium.NoSuchSessionException: Session is closed` +* `org.openqa.selenium.WebDriverException: org.openqa.selenium.NoSuchWindowException: Window is closed` +* `java.util.concurrent.RejectedExecutionException: Task org.openqa.selenium.htmlunit.HtmlUnitDriver$$Lambda$972/0x0000000100572c40@39b402c9 rejected from java.util.concurrent.ThreadPoolExecutor@4fd3fc52[Terminated, pool size = 0, active threads = 0, queued tasks = 0, completed tasks = 4]` + +This is because the `cacheDriverPerThread` setting keeps the webdriver in a ThreadLocal. Since BeforeAll and AfterAll methods on different test classes annotated with `@Execution(CONCURRENT)` may execute on the same thread, it's possible to inadvertently quit the driver being used by another test currently executing on the same thread, leading to errors like those above. + +If you're relying on the implicit driver management to clean up the driver objects for you, you should be fine. +Likewise, clearing the driver cache in methods annotated with link:https://docs.junit.org/5.0.1/api/org/junit/jupiter/api/BeforeEach.html[`@BeforeEach`] and link:https://docs.junit.org/5.0.1/api/org/junit/jupiter/api/AfterEach.html[`@AfterEach`] should be fine, since parallel test methods (and test-specific lifecycle methods) should be executed in the same thread. It's only in the `@BeforeAll`/`@AfterAll` case that you need to be careful. +==== + == Driver quirks This section details various quirks or issues that have been encountered with different driver implementations. diff --git a/module/geb-core/src/main/groovy/geb/driver/CachingDriverFactory.groovy b/module/geb-core/src/main/groovy/geb/driver/CachingDriverFactory.groovy index 6911fc1b..716341db 100644 --- a/module/geb-core/src/main/groovy/geb/driver/CachingDriverFactory.groovy +++ b/module/geb-core/src/main/groovy/geb/driver/CachingDriverFactory.groovy @@ -46,6 +46,18 @@ class CachingDriverFactory implements DriverFactory { CACHE.get { null }?.clear() } + /** + * Note: When using a perThread cache and executing multiple tests in parallel, it is possible to inadvertently + * quit a driver in one thread that is being used by another, depending on how your test runner handles parallel + * execution. The only known case of this is in JUnit's `@BeforeAll` and `@AfterAll` lifecycle extension methods + * on multiple classes annotated with `@Execution(CONCURRENT)`, but there may be others. + * + * Implicit driver lifecycle management is your friend. + * + * @param innerFactory + * @param quitOnShutdown + * @return + */ static WebDriver clearCacheAndQuitDriver() { def driver = clearCache() driver?.quit() diff --git a/module/geb-junit5/src/test/groovy/geb/junit5/ParallelExecutionTest.groovy b/module/geb-junit5/src/test/groovy/geb/junit5/ParallelExecutionTest.groovy index aff066c3..24120071 100644 --- a/module/geb-junit5/src/test/groovy/geb/junit5/ParallelExecutionTest.groovy +++ b/module/geb-junit5/src/test/groovy/geb/junit5/ParallelExecutionTest.groovy @@ -53,8 +53,6 @@ class ParallelExecutionTest extends ConfigModifyingGebTest { static void setupClass() { classExecutionExclusivityLock.acquire() classOrderLatch.countDown() - CachingDriverFactory.clearCacheAndQuitDriver() - CachingDriverFactory.clearCacheCache() callbackServerExtension.server.html { HttpServletRequest request -> body { div "${request.requestURI.toURI().path}" @@ -64,7 +62,6 @@ class ParallelExecutionTest extends ConfigModifyingGebTest { @AfterAll static void tearDownClass() { - CachingDriverFactory.clearCacheCache() classExecutionExclusivityLock.release(2) } @@ -102,8 +99,6 @@ abstract class AbstractParallelExecutionWithReportingTest extends ConfigModifyin static void setupClass() { ParallelExecutionTest.classOrderLatch.await() ParallelExecutionTest.classExecutionExclusivityLock.acquire() - CachingDriverFactory.clearCacheAndQuitDriver() - CachingDriverFactory.clearCacheCache() clazz.callbackServerExtension.server.html { HttpServletRequest request -> body { div "${request.requestURI.toURI().path}" @@ -121,7 +116,6 @@ abstract class AbstractParallelExecutionWithReportingTest extends ConfigModifyin // still used by the other class by synchronizing // again using a count down latch executionOfTestsCompleteLatch.await() - CachingDriverFactory.clearCacheCache() assert reportFileTestCounterPrefixes(clazz) == '001'..'004' }
