Skip to content

Commit 32e84f4

Browse files
VampireVampire
committed
Do not persist a driver instance in the configuration object but in the browser
1 parent e1ff939 commit 32e84f4

File tree

8 files changed

+89
-58
lines changed

8 files changed

+89
-58
lines changed

‎doc/manual-snippets/src/test/groovy/configuration/DriverConfigSpec.groovy

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ class DriverConfigSpec extends Specification implements InlineConfigurationLoade
5454
"""
5555

5656
then:
57-
config.driver instanceof FirefoxDriver
57+
config.createDriver() instanceof FirefoxDriver
5858

5959
cleanup:
6060
CachingDriverFactory.clearCacheAndQuitDriver()
@@ -72,7 +72,7 @@ class DriverConfigSpec extends Specification implements InlineConfigurationLoade
7272
"""
7373

7474
then:
75-
config.driver instanceof FirefoxDriver
75+
config.createDriver() instanceof FirefoxDriver
7676

7777
cleanup:
7878
CachingDriverFactory.clearCacheAndQuitDriver()
@@ -90,7 +90,7 @@ class DriverConfigSpec extends Specification implements InlineConfigurationLoade
9090
"""
9191

9292
then:
93-
config.driver instanceof FirefoxDriver
93+
config.createDriver() instanceof FirefoxDriver
9494

9595
cleanup:
9696
CachingDriverFactory.clearCacheAndQuitDriver()
@@ -130,7 +130,7 @@ class DriverConfigSpec extends Specification implements InlineConfigurationLoade
130130
""")
131131

132132
then:
133-
config.driver in driverClass
133+
config.createDriver() in driverClass
134134

135135
cleanup:
136136
CachingDriverFactory.clearCacheAndQuitDriver()
@@ -140,4 +140,4 @@ class DriverConfigSpec extends Specification implements InlineConfigurationLoade
140140
null | HtmlUnitDriver
141141
"remote" | RemoteWebDriver
142142
}
143-
}
143+
}

‎gradle/codenarc/rulesets.groovy

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ ruleset {
2828
doNotApplyToFileNames = 'GebTest.groovy, GebReportingTest.groovy'
2929
}
3030
EmptyMethod {
31-
doNotApplyToClassNames = 'UsingRuleWithoutTestManagerTest, UsingReportingRuleWithoutTestManagerTest'
31+
doNotApplyToClassNames = 'UsingRuleWithoutTestManagerTest, UsingReportingRuleWithoutTestManagerTest, Configuration'
3232
}
3333
}
3434
ruleset('rulesets/braces.xml')
@@ -190,4 +190,4 @@ ruleset {
190190
}
191191
}
192192
ruleset('rulesets/unused.xml')
193-
}
193+
}

‎module/geb-core/src/main/groovy/geb/Browser.groovy

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ class Browser {
6060

6161
private Page page
6262
private final Configuration config
63+
private WebDriver driver
6364
private final RemoteDriverOperations remoteDriverOperations = new RemoteDriverOperations(this.class.classLoader)
6465
private String reportGroup = null
6566
private NavigatorFactory navigatorFactory = null
@@ -68,7 +69,7 @@ class Browser {
6869
* If the driver is remote, this object allows access to its capabilities (users of Geb should not access this object, it is used internally).
6970
*/
7071
@Lazy
71-
WebDriver augmentedDriver = remoteDriverOperations.getAugmentedDriver(driver)
72+
WebDriver augmentedDriver = remoteDriverOperations.getAugmentedDriver(getDriver())
7273

7374
/**
7475
* Create a new browser with a default configuration loader, loading the default configuration file.
@@ -164,10 +165,14 @@ class Browser {
164165
* <p>
165166
* The driver implementation to use is determined by the configuration.
166167
*
167-
* @see geb.Configuration#getDriver()
168+
* @see geb.Configuration#createDriver()
168169
*/
169170
WebDriver getDriver() {
170-
config.driver
171+
if (driver == null) {
172+
driver = createDriver()
173+
}
174+
175+
driver
171176
}
172177

173178
/**
@@ -189,11 +194,9 @@ class Browser {
189194
* This should only be called before making any requests as a means to override the driver instance
190195
* that would be created from the configuration. Where possible, prefer using the configuration mechanism
191196
* to specify the driver implementation to use.
192-
* <p>
193-
* This method delegates to {@link geb.Configuration#setDriver(org.openqa.selenium.WebDriver)}.
194197
*/
195198
void setDriver(WebDriver driver) {
196-
config.driver = driver
199+
this.driver = driver
197200
}
198201

199202
/**
@@ -223,7 +226,7 @@ class Browser {
223226
* @see org.openqa.selenium.WebDriver#getCurrentUrl()
224227
*/
225228
String getCurrentUrl() {
226-
driver.currentUrl
229+
getDriver().currentUrl
227230
}
228231

229232
/**
@@ -509,9 +512,9 @@ class Browser {
509512
void go(Map params = [:], String url = null, UrlFragment fragment = null) {
510513
def newUri = calculateUri(url, params, fragment)
511514
def currentUri = retrieveCurrentUri()
512-
driver.get(newUri.toString())
515+
getDriver().get(newUri.toString())
513516
if (sameUrlWithDifferentFragment(currentUri, newUri)) {
514-
driver.navigate().refresh()
517+
getDriver().navigate().refresh()
515518
}
516519
if (!page) {
517520
page(Page)
@@ -630,7 +633,7 @@ class Browser {
630633
* @see org.openqa.selenium.WebDriver.Options#deleteAllCookies()
631634
*/
632635
void clearCookies() {
633-
driver?.manage()?.deleteAllCookies()
636+
getDriver()?.manage()?.deleteAllCookies()
634637
}
635638

636639
/**
@@ -669,7 +672,7 @@ class Browser {
669672
* @see org.openqa.selenium.WebDriver#quit()
670673
*/
671674
void quit() {
672-
driver.quit()
675+
getDriver().quit()
673676
}
674677

675678
/**
@@ -678,7 +681,7 @@ class Browser {
678681
* @see org.openqa.selenium.WebDriver#close()
679682
*/
680683
void close() {
681-
driver.close()
684+
getDriver().close()
682685
}
683686

684687
/**
@@ -687,7 +690,7 @@ class Browser {
687690
* @see org.openqa.selenium.WebDriver#getWindowHandle()
688691
*/
689692
String getCurrentWindow() {
690-
driver.windowHandle
693+
getDriver().windowHandle
691694
}
692695

693696
/**
@@ -696,7 +699,7 @@ class Browser {
696699
* @see org.openqa.selenium.WebDriver#getWindowHandles()
697700
*/
698701
Set<String> getAvailableWindows() {
699-
driver.windowHandles
702+
getDriver().windowHandles
700703
}
701704

702705
/**
@@ -812,7 +815,7 @@ class Browser {
812815
cloned.call()
813816
} finally {
814817
if ((!options.containsKey(CLOSE_OPTION) && config.withNewWindowConfig.close.orElse(true)) || options.close) {
815-
driver.close()
818+
getDriver().close()
816819
}
817820
switchToWindow(originalWindow)
818821
page originalPage
@@ -996,8 +999,8 @@ class Browser {
996999
}
9971000

9981001
public <T> Optional<T> driverAs(Class<T> castType) {
999-
if (castType.isInstance(driver)) {
1000-
Optional.of(driver as T)
1002+
if (castType.isInstance(getDriver())) {
1003+
Optional.of(getDriver() as T)
10011004
} else if (castType.isInstance(augmentedDriver)) {
10021005
Optional.of(augmentedDriver as T)
10031006
} else {
@@ -1006,7 +1009,7 @@ class Browser {
10061009
}
10071010

10081011
protected switchToWindow(String window) {
1009-
driver.switchTo().window(window)
1012+
getDriver().switchTo().window(window)
10101013
}
10111014

10121015
protected <T> T doWithWindow(Map options, Closure<T> block) {
@@ -1021,7 +1024,7 @@ class Browser {
10211024
cloned.call()
10221025
} finally {
10231026
if (options.close || (!options.containsKey(CLOSE_OPTION) && config.withWindowConfig.close)) {
1024-
driver.close()
1027+
getDriver().close()
10251028
}
10261029
}
10271030
}
@@ -1035,6 +1038,15 @@ class Browser {
10351038
config.createNavigatorFactory(this)
10361039
}
10371040

1041+
/**
1042+
* Called to create the driver, the first time it is requested.
1043+
*
1044+
* @return The driver
1045+
*/
1046+
protected WebDriver createDriver() {
1047+
config.createDriver()
1048+
}
1049+
10381050
private WebDriver getDriverWithNetworkConditions() {
10391051
optionalHasNetworkConditionsClass.map { hasNetworkConditionsClass ->
10401052
driverAs(hasNetworkConditionsClass).orElse(null)
@@ -1230,7 +1242,7 @@ class Browser {
12301242
private URI retrieveCurrentUri() {
12311243
def currentUri = null
12321244
try {
1233-
def currentUrl = driver.currentUrl
1245+
def currentUrl = getDriver().currentUrl
12341246
currentUri = currentUrl ? new URI(currentUrl) : null
12351247
} catch (NullPointerException npe) {
12361248
} catch (URISyntaxException use) {

‎module/geb-core/src/main/groovy/geb/Configuration.groovy

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,6 @@ class Configuration {
3939
final Properties properties
4040
final BuildAdapter buildAdapter
4141

42-
private WebDriver driver
43-
4442
Configuration(Map rawConfig) {
4543
this(toConfigObject(rawConfig), null, null, null)
4644
}
@@ -256,7 +254,7 @@ class Configuration {
256254
* This may be the class name of a driver implementation, a driver short name or a closure
257255
* that when invoked with no arguments returns a driver implementation.
258256
*
259-
* @see #getDriver()
257+
* @see #createDriver()
260258
*/
261259
void setDriverConf(value) {
262260
rawConfig.driver = value
@@ -268,7 +266,7 @@ class Configuration {
268266
* This may be the class name of a driver implementation, a short name, or a closure
269267
* that when invoked returns an actual driver.
270268
*
271-
* @see #getDriver()
269+
* @see #createDriver()
272270
*/
273271
def getDriverConf() {
274272
def value = properties.getProperty("geb.driver") ?: readValue("driver", null)
@@ -376,16 +374,27 @@ class Configuration {
376374
rawConfig.pageEventListener = pageEventListener
377375
}
378376

379-
WebDriver getDriver() {
380-
if (driver == null) {
381-
driver = createDriver()
382-
}
377+
WebDriver createDriver() {
378+
wrapDriverFactoryInCachingIfNeeded(getDriverFactory(getDriverConf())).driver
379+
}
383380

384-
driver
381+
/**
382+
* @deprecated As of 8.0, replaced by {@link #createDriver()}, the configuration does
383+
* no longer carry a driver instance.
384+
*/
385+
@Deprecated
386+
WebDriver getDriver() {
387+
createDriver()
385388
}
386389

387-
void setDriver(WebDriver driver) {
388-
this.driver = driver
390+
/**
391+
* @deprecated As of 8.0, the configuration does no longer carry a driver
392+
* instance, but only create new driver instances, the driver
393+
* instance used is stored in the browser instance.
394+
*/
395+
@Deprecated
396+
void setDriver(WebDriver ignored) {
397+
// does nothing anymore
389398
}
390399

391400
/**
@@ -700,10 +709,6 @@ class Configuration {
700709
throw new InvalidGebConfiguration("Configuration for bounds and 'required' template options is conflicting")
701710
}
702711

703-
protected WebDriver createDriver() {
704-
wrapDriverFactoryInCachingIfNeeded(getDriverFactory(getDriverConf())).driver
705-
}
706-
707712
protected DriverFactory getDriverFactory(driverValue) {
708713
switch (driverValue) {
709714
case CharSequence:

‎module/geb-core/src/test/groovy/geb/PauseSpec.groovy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ class PauseSpec extends GebSpecWithCallbackServer {
5656
@InheritConstructors
5757
private static class ThreadSafeScriptExecutionDriverConfiguration extends Configuration {
5858
@Override
59-
protected WebDriver createDriver() {
59+
WebDriver createDriver() {
6060
def driver = super.createDriver()
6161
driver.javascriptEnabled = true
6262
ThreadSafeScriptExecutionDriver.of(driver)

‎module/geb-core/src/test/groovy/geb/conf/ConfigurationDriverCreationSpec.groovy

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ class ConfigurationDriverCreationSpec extends Specification {
168168
def config = new ConfigObject()
169169
config.cacheDriver = false
170170
config.driver = { new HtmlUnitDriver() }
171-
d = new Configuration(config, p()).driver
171+
d = new Configuration(config, p()).createDriver()
172172

173173
then:
174174
d instanceof HtmlUnitDriver
@@ -182,7 +182,7 @@ class ConfigurationDriverCreationSpec extends Specification {
182182
config.driver = { 'not a driver' }
183183

184184
when:
185-
new Configuration(config).driver
185+
new Configuration(config).createDriver()
186186

187187
then:
188188
thrown(DriverCreationException)

0 commit comments

Comments
 (0)