This is an automated email from the ASF dual-hosted git repository.
jonnybot pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy-geb.git
The following commit(s) were added to refs/heads/master by this push:
new 42d5ad9a Do not persist a driver instance in the configuration object
but in the browser
42d5ad9a is described below
commit 42d5ad9a7e697e18e1d65a3f1a2b7b27486f0f90
Author: Björn Kautler <[email protected]>
AuthorDate: Sat Apr 19 04:44:02 2025 +0200
Do not persist a driver instance in the configuration object but in the
browser
---
.../groovy/configuration/DriverConfigSpec.groovy | 10 ++---
gradle/codenarc/rulesets.groovy | 4 +-
module/geb-core/src/main/groovy/geb/Browser.groovy | 52 +++++++++++++---------
.../src/main/groovy/geb/Configuration.groovy | 35 ++++++++-------
.../geb-core/src/test/groovy/geb/PauseSpec.groovy | 2 +-
.../conf/ConfigurationDriverCreationSpec.groovy | 4 +-
.../test/groovy/geb/conf/DriverCachingSpec.groovy | 38 +++++++++++-----
.../geb/test/GebTestManagerBrowserResetSpec.groovy | 4 +-
8 files changed, 90 insertions(+), 59 deletions(-)
diff --git
a/doc/manual-snippets/src/test/groovy/configuration/DriverConfigSpec.groovy
b/doc/manual-snippets/src/test/groovy/configuration/DriverConfigSpec.groovy
index 698e9fa0..6dfb3da9 100644
--- a/doc/manual-snippets/src/test/groovy/configuration/DriverConfigSpec.groovy
+++ b/doc/manual-snippets/src/test/groovy/configuration/DriverConfigSpec.groovy
@@ -54,7 +54,7 @@ class DriverConfigSpec extends Specification implements
InlineConfigurationLoade
"""
then:
- config.driver instanceof FirefoxDriver
+ config.createDriver() instanceof FirefoxDriver
cleanup:
CachingDriverFactory.clearCacheAndQuitDriver()
@@ -72,7 +72,7 @@ class DriverConfigSpec extends Specification implements
InlineConfigurationLoade
"""
then:
- config.driver instanceof FirefoxDriver
+ config.createDriver() instanceof FirefoxDriver
cleanup:
CachingDriverFactory.clearCacheAndQuitDriver()
@@ -90,7 +90,7 @@ class DriverConfigSpec extends Specification implements
InlineConfigurationLoade
"""
then:
- config.driver instanceof FirefoxDriver
+ config.createDriver() instanceof FirefoxDriver
cleanup:
CachingDriverFactory.clearCacheAndQuitDriver()
@@ -130,7 +130,7 @@ class DriverConfigSpec extends Specification implements
InlineConfigurationLoade
""")
then:
- config.driver in driverClass
+ config.createDriver() in driverClass
cleanup:
CachingDriverFactory.clearCacheAndQuitDriver()
@@ -140,4 +140,4 @@ class DriverConfigSpec extends Specification implements
InlineConfigurationLoade
null | HtmlUnitDriver
"remote" | RemoteWebDriver
}
-}
\ No newline at end of file
+}
diff --git a/gradle/codenarc/rulesets.groovy b/gradle/codenarc/rulesets.groovy
index 8a5bbe2f..527a157b 100644
--- a/gradle/codenarc/rulesets.groovy
+++ b/gradle/codenarc/rulesets.groovy
@@ -28,7 +28,7 @@ ruleset {
doNotApplyToFileNames = 'GebTest.groovy, GebReportingTest.groovy'
}
EmptyMethod {
- doNotApplyToClassNames = 'UsingRuleWithoutTestManagerTest,
UsingReportingRuleWithoutTestManagerTest'
+ doNotApplyToClassNames = 'UsingRuleWithoutTestManagerTest,
UsingReportingRuleWithoutTestManagerTest, Configuration'
}
}
ruleset('rulesets/braces.xml')
@@ -190,4 +190,4 @@ ruleset {
}
}
ruleset('rulesets/unused.xml')
-}
\ No newline at end of file
+}
diff --git a/module/geb-core/src/main/groovy/geb/Browser.groovy
b/module/geb-core/src/main/groovy/geb/Browser.groovy
index 625e0d5d..3b614463 100644
--- a/module/geb-core/src/main/groovy/geb/Browser.groovy
+++ b/module/geb-core/src/main/groovy/geb/Browser.groovy
@@ -60,6 +60,7 @@ class Browser {
private Page page
private final Configuration config
+ private WebDriver driver
private final RemoteDriverOperations remoteDriverOperations = new
RemoteDriverOperations(this.class.classLoader)
private String reportGroup = null
private NavigatorFactory navigatorFactory = null
@@ -68,7 +69,7 @@ class Browser {
* If the driver is remote, this object allows access to its capabilities
(users of Geb should not access this object, it is used internally).
*/
@Lazy
- WebDriver augmentedDriver =
remoteDriverOperations.getAugmentedDriver(driver)
+ WebDriver augmentedDriver =
remoteDriverOperations.getAugmentedDriver(getDriver())
/**
* Create a new browser with a default configuration loader, loading the
default configuration file.
@@ -164,10 +165,14 @@ class Browser {
* <p>
* The driver implementation to use is determined by the configuration.
*
- * @see geb.Configuration#getDriver()
+ * @see geb.Configuration#createDriver()
*/
WebDriver getDriver() {
- config.driver
+ if (driver == null) {
+ driver = createDriver()
+ }
+
+ driver
}
/**
@@ -189,11 +194,9 @@ class Browser {
* This should only be called before making any requests as a means to
override the driver instance
* that would be created from the configuration. Where possible, prefer
using the configuration mechanism
* to specify the driver implementation to use.
- * <p>
- * This method delegates to {@link
geb.Configuration#setDriver(org.openqa.selenium.WebDriver)}.
*/
void setDriver(WebDriver driver) {
- config.driver = driver
+ this.driver = driver
}
/**
@@ -223,7 +226,7 @@ class Browser {
* @see org.openqa.selenium.WebDriver#getCurrentUrl()
*/
String getCurrentUrl() {
- driver.currentUrl
+ getDriver().currentUrl
}
/**
@@ -509,9 +512,9 @@ class Browser {
void go(Map params = [:], String url = null, UrlFragment fragment = null) {
def newUri = calculateUri(url, params, fragment)
def currentUri = retrieveCurrentUri()
- driver.get(newUri.toString())
+ getDriver().get(newUri.toString())
if (sameUrlWithDifferentFragment(currentUri, newUri)) {
- driver.navigate().refresh()
+ getDriver().navigate().refresh()
}
if (!page) {
page(Page)
@@ -630,7 +633,7 @@ class Browser {
* @see org.openqa.selenium.WebDriver.Options#deleteAllCookies()
*/
void clearCookies() {
- driver?.manage()?.deleteAllCookies()
+ getDriver()?.manage()?.deleteAllCookies()
}
/**
@@ -669,7 +672,7 @@ class Browser {
* @see org.openqa.selenium.WebDriver#quit()
*/
void quit() {
- driver.quit()
+ getDriver().quit()
}
/**
@@ -678,7 +681,7 @@ class Browser {
* @see org.openqa.selenium.WebDriver#close()
*/
void close() {
- driver.close()
+ getDriver().close()
}
/**
@@ -687,7 +690,7 @@ class Browser {
* @see org.openqa.selenium.WebDriver#getWindowHandle()
*/
String getCurrentWindow() {
- driver.windowHandle
+ getDriver().windowHandle
}
/**
@@ -696,7 +699,7 @@ class Browser {
* @see org.openqa.selenium.WebDriver#getWindowHandles()
*/
Set<String> getAvailableWindows() {
- driver.windowHandles
+ getDriver().windowHandles
}
/**
@@ -812,7 +815,7 @@ class Browser {
cloned.call()
} finally {
if ((!options.containsKey(CLOSE_OPTION) &&
config.withNewWindowConfig.close.orElse(true)) || options.close) {
- driver.close()
+ getDriver().close()
}
switchToWindow(originalWindow)
page originalPage
@@ -996,8 +999,8 @@ class Browser {
}
public <T> Optional<T> driverAs(Class<T> castType) {
- if (castType.isInstance(driver)) {
- Optional.of(driver as T)
+ if (castType.isInstance(getDriver())) {
+ Optional.of(getDriver() as T)
} else if (castType.isInstance(augmentedDriver)) {
Optional.of(augmentedDriver as T)
} else {
@@ -1006,7 +1009,7 @@ class Browser {
}
protected switchToWindow(String window) {
- driver.switchTo().window(window)
+ getDriver().switchTo().window(window)
}
protected <T> T doWithWindow(Map options, Closure<T> block) {
@@ -1021,7 +1024,7 @@ class Browser {
cloned.call()
} finally {
if (options.close || (!options.containsKey(CLOSE_OPTION) &&
config.withWindowConfig.close)) {
- driver.close()
+ getDriver().close()
}
}
}
@@ -1035,6 +1038,15 @@ class Browser {
config.createNavigatorFactory(this)
}
+ /**
+ * Called to create the driver, the first time it is requested.
+ *
+ * @return The driver
+ */
+ protected WebDriver createDriver() {
+ config.createDriver()
+ }
+
private WebDriver getDriverWithNetworkConditions() {
optionalHasNetworkConditionsClass.map { hasNetworkConditionsClass ->
driverAs(hasNetworkConditionsClass).orElse(null)
@@ -1230,7 +1242,7 @@ class Browser {
private URI retrieveCurrentUri() {
def currentUri = null
try {
- def currentUrl = driver.currentUrl
+ def currentUrl = getDriver().currentUrl
currentUri = currentUrl ? new URI(currentUrl) : null
} catch (NullPointerException npe) {
} catch (URISyntaxException use) {
diff --git a/module/geb-core/src/main/groovy/geb/Configuration.groovy
b/module/geb-core/src/main/groovy/geb/Configuration.groovy
index 40c948dc..73652141 100644
--- a/module/geb-core/src/main/groovy/geb/Configuration.groovy
+++ b/module/geb-core/src/main/groovy/geb/Configuration.groovy
@@ -39,8 +39,6 @@ class Configuration {
final Properties properties
final BuildAdapter buildAdapter
- private WebDriver driver
-
Configuration(Map rawConfig) {
this(toConfigObject(rawConfig), null, null, null)
}
@@ -256,7 +254,7 @@ class Configuration {
* This may be the class name of a driver implementation, a driver short
name or a closure
* that when invoked with no arguments returns a driver implementation.
*
- * @see #getDriver()
+ * @see #createDriver()
*/
void setDriverConf(value) {
rawConfig.driver = value
@@ -268,7 +266,7 @@ class Configuration {
* This may be the class name of a driver implementation, a short name, or
a closure
* that when invoked returns an actual driver.
*
- * @see #getDriver()
+ * @see #createDriver()
*/
def getDriverConf() {
def value = properties.getProperty("geb.driver") ?:
readValue("driver", null)
@@ -376,16 +374,27 @@ class Configuration {
rawConfig.pageEventListener = pageEventListener
}
- WebDriver getDriver() {
- if (driver == null) {
- driver = createDriver()
- }
+ WebDriver createDriver() {
+
wrapDriverFactoryInCachingIfNeeded(getDriverFactory(getDriverConf())).driver
+ }
- driver
+ /**
+ * @deprecated As of 8.0, replaced by {@link #createDriver()}, the
configuration does
+ * no longer carry a driver instance.
+ */
+ @Deprecated
+ WebDriver getDriver() {
+ createDriver()
}
- void setDriver(WebDriver driver) {
- this.driver = driver
+ /**
+ * @deprecated As of 8.0, the configuration does no longer carry a driver
+ * instance, but only create new driver instances, the driver
+ * instance used is stored in the browser instance.
+ */
+ @Deprecated
+ void setDriver(WebDriver ignored) {
+ // does nothing anymore
}
/**
@@ -700,10 +709,6 @@ class Configuration {
throw new InvalidGebConfiguration("Configuration for bounds and
'required' template options is conflicting")
}
- protected WebDriver createDriver() {
-
wrapDriverFactoryInCachingIfNeeded(getDriverFactory(getDriverConf())).driver
- }
-
protected DriverFactory getDriverFactory(driverValue) {
switch (driverValue) {
case CharSequence:
diff --git a/module/geb-core/src/test/groovy/geb/PauseSpec.groovy
b/module/geb-core/src/test/groovy/geb/PauseSpec.groovy
index b06453fa..ef0f0539 100644
--- a/module/geb-core/src/test/groovy/geb/PauseSpec.groovy
+++ b/module/geb-core/src/test/groovy/geb/PauseSpec.groovy
@@ -56,7 +56,7 @@ class PauseSpec extends GebSpecWithCallbackServer {
@InheritConstructors
private static class ThreadSafeScriptExecutionDriverConfiguration extends
Configuration {
@Override
- protected WebDriver createDriver() {
+ WebDriver createDriver() {
def driver = super.createDriver()
driver.javascriptEnabled = true
ThreadSafeScriptExecutionDriver.of(driver)
diff --git
a/module/geb-core/src/test/groovy/geb/conf/ConfigurationDriverCreationSpec.groovy
b/module/geb-core/src/test/groovy/geb/conf/ConfigurationDriverCreationSpec.groovy
index 0840b910..7da3399a 100644
---
a/module/geb-core/src/test/groovy/geb/conf/ConfigurationDriverCreationSpec.groovy
+++
b/module/geb-core/src/test/groovy/geb/conf/ConfigurationDriverCreationSpec.groovy
@@ -168,7 +168,7 @@ class ConfigurationDriverCreationSpec extends Specification
{
def config = new ConfigObject()
config.cacheDriver = false
config.driver = { new HtmlUnitDriver() }
- d = new Configuration(config, p()).driver
+ d = new Configuration(config, p()).createDriver()
then:
d instanceof HtmlUnitDriver
@@ -182,7 +182,7 @@ class ConfigurationDriverCreationSpec extends Specification
{
config.driver = { 'not a driver' }
when:
- new Configuration(config).driver
+ new Configuration(config).createDriver()
then:
thrown(DriverCreationException)
diff --git a/module/geb-core/src/test/groovy/geb/conf/DriverCachingSpec.groovy
b/module/geb-core/src/test/groovy/geb/conf/DriverCachingSpec.groovy
index 3d3a6903..6b57a409 100644
--- a/module/geb-core/src/test/groovy/geb/conf/DriverCachingSpec.groovy
+++ b/module/geb-core/src/test/groovy/geb/conf/DriverCachingSpec.groovy
@@ -35,13 +35,11 @@ class DriverCachingSpec extends Specification {
File getReportsDir() { null }
}
- def conf(cacheDriverPerThread = false) {
+ def conf(cacheDriverPerThread = false, cacheDriver = true) {
def conf = new Configuration(new ConfigObject(), new Properties(), new
NullBuildAdapter())
+ conf.cacheDriver = cacheDriver
conf.cacheDriverPerThread = cacheDriverPerThread
conf.driverConf = { new HtmlUnitDriver() }
-
- assert conf.cacheDriver
-
conf
}
@@ -58,23 +56,23 @@ class DriverCachingSpec extends Specification {
def holder = new BlockingVariable()
when:
- Thread.start { holder.set(conf2.driver) }
+ Thread.start { holder.set(conf2.createDriver()) }
and:
- def driver1 = conf1.driver
+ def driver1 = conf1.createDriver()
def driver2 = holder.get()
then:
!driver1.is(driver2)
and:
- driver1.is conf(true).driver
+ driver1.is conf(true).createDriver()
when:
CachingDriverFactory.clearCacheAndQuitDriver()
then:
- !driver1.is(conf(true).driver)
+ !driver1.is(conf(true).createDriver())
cleanup:
driver1.quit()
@@ -90,23 +88,39 @@ class DriverCachingSpec extends Specification {
def holder = new BlockingVariable()
when:
- Thread.start { holder.set(conf2.driver) }
+ Thread.start { holder.set(conf2.createDriver()) }
and:
- def driver1 = conf1.driver
+ def driver1 = conf1.createDriver()
def driver2 = holder.get()
then:
driver1.is(driver2)
and:
- driver1.is conf().driver
+ driver1.is conf().createDriver()
when:
CachingDriverFactory.clearCacheAndQuitDriver()
then:
- !driver1.is(conf(true).driver)
+ !driver1.is(conf(true).createDriver())
+
+ cleanup:
+ driver1.quit()
+ driver2.quit()
+ }
+
+ def "disabled caching yields different drivers on each call"() {
+ given:
+ def conf = conf(false, false)
+
+ when:
+ def driver1 = conf.createDriver()
+ def driver2 = conf.createDriver()
+
+ then:
+ !driver1.is(driver2)
cleanup:
driver1.quit()
diff --git
a/module/geb-core/src/test/groovy/geb/test/GebTestManagerBrowserResetSpec.groovy
b/module/geb-core/src/test/groovy/geb/test/GebTestManagerBrowserResetSpec.groovy
index b5d82ce8..fddd385f 100644
---
a/module/geb-core/src/test/groovy/geb/test/GebTestManagerBrowserResetSpec.groovy
+++
b/module/geb-core/src/test/groovy/geb/test/GebTestManagerBrowserResetSpec.groovy
@@ -48,10 +48,10 @@ class GebTestManagerBrowserResetSpec extends Specification {
driver = new RemoteWebDriverWithExpectations(
webDriverServer.url, DEFAULT_IGNORED_COMMANDS - 'quit'
)
- configuration.driver = driver
+ configuration.driverConf = { null }
gebTestManager = new GebTestManagerBuilder()
.withBrowserCreator {
- new Browser(configuration)
+ new Browser(configuration, driver: driver)
}
.build()
