cbmarcum commented on code in PR #313:
URL: https://github.com/apache/groovy-geb/pull/313#discussion_r3005341372
##########
integration/geb-testcontainers/geb-testcontainers.gradle:
##########
@@ -103,9 +103,6 @@ tasks.register('integrationTest', Test) {
systemProperty('geb.env', System.getProperty('geb.env'))
Review Comment:
add a default environment in case one isn't passed in since integrationTest
is being ran during check instead of firefoxTest.
```suggestion
systemProperty('geb.env', System.getProperty('geb.env') ?: 'firefox')
```
##########
integration/geb-testcontainers/src/main/groovy/geb/testcontainers/WebDriverContainerHolder.groovy:
##########
@@ -176,29 +161,21 @@ class WebDriverContainerHolder {
}
// Ensure that the driver points to the re-initialized container with
the correct host.
- // The driver is explicitly quit by us in stop() method, to fulfill
our resulting responsibility.
gebConf.cacheDriver = false
-
- // As we don't cache, this will have been defaulted to true. We
override to false.
gebConf.quitDriverOnBrowserReset = false
-
gebConf.baseUrl = container.seleniumAddress
+
if (containerConf.reporting) {
gebConf.reportsDir = settings.reportingDirectory
gebConf.reporter = (methodInvocation.sharedInstance as
ContainerGebSpec).createReporter()
}
- if (gebConf.driverConf) {
- // As a custom `GebConfig` cannot know the `remoteAddress` of the
container beforehand,
- // the `RemoteWebDriver` will be instantiated using the
`webdriver.remote.server`
- // system property. We set that property to inform the driver of
the container address.
- gebConf.driverConf = ClosureDecorators.withSystemProperty(
- gebConf.driverConf as Closure,
- REMOTE_ADDRESS_PROPERTY,
- container.seleniumAddress
- )
- } else {
- // If no driver was set in GebConfig, create a Firefox driver
+ // Set the selenium address as a system property so that
RemoteWebDriver
+ // constructors (without explicit URL) can find it. Tests run serially
+ // (enforced by ExclusiveResource) so this is safe.
+ System.setProperty(REMOTE_ADDRESS_PROPERTY,
container.seleniumAddress.toString())
+
+ if (!gebConf.driverConf) {
Review Comment:
codenarc wants this if block to use an elvis operator instead
```suggestion
gebConf.driverConf = gebConf.driverConf ?: { ->
log.info('Using default Firefox RemoteWebDriver for {}',
container.seleniumAddress)
new RemoteWebDriver(container.seleniumAddress, new
FirefoxOptions())
}
```
##########
integration/geb-testcontainers/src/main/groovy/geb/testcontainers/ContainerGebSpec.groovy:
##########
@@ -18,20 +18,22 @@
*/
package geb.testcontainers
-import geb.Page
+import geb.download.DownloadSupport
+import geb.report.CompositeReporter
+import geb.report.PageSourceReporter
+import geb.report.Reporter
+import geb.report.ScreenshotReporter
import geb.test.GebTestManager
-import geb.testcontainers.support.ContainerSupport
-import geb.testcontainers.support.ReportingSupport
-import geb.testcontainers.support.delegate.BrowserDelegate
-import geb.testcontainers.support.delegate.DownloadSupportDelegate
-import geb.testcontainers.support.delegate.DriverDelegate
-import geb.testcontainers.support.delegate.PageDelegate
-import groovy.transform.CompileStatic
+import geb.transform.DynamicallyDispatchesToBrowser
+import geb.testcontainers.support.ContainerGebFileInputSource
+import org.testcontainers.containers.BrowserWebDriverContainer
+import org.testcontainers.containers.GenericContainer
Review Comment:
GenericContainer import is unused.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]