From fdc7f20e17369d6b644764059ce308d9c4820a9b Mon Sep 17 00:00:00 2001 From: James Daugherty Date: Sun, 24 Nov 2024 11:08:54 -0500 Subject: [PATCH] feedback - formating, use existing enum instead of recording flag, and clean-up --- README.md | 14 ++++------- .../geb/ContainerAwareDownloadSupport.groovy | 3 ++- .../geb/ContainerGebConfiguration.groovy | 24 +++++++------------ .../geb/ContainerGebRecordingExtension.groovy | 23 +++++++++++------- .../grails/plugin/geb/ContainerGebSpec.groovy | 4 ++-- .../geb/ContainerGebTestDescription.groovy | 5 ++-- .../geb/ContainerGebTestListener.groovy | 12 ++++++---- 7 files changed, 42 insertions(+), 43 deletions(-) diff --git a/README.md b/README.md index df53bad..3997dcf 100644 --- a/README.md +++ b/README.md @@ -57,9 +57,11 @@ Just run `./gradlew integrationTest` and a container will be started and configu By default, all failed tests will generate a video recording of the test execution. These recordings are saved to a `recordings` directory under the project's `build` directory. The following system properties can be change to configure the recording behavior: -* `grails.geb.recording.enabled` - * purpose: toggle for recording - * possible values: `true` or `false` + +* `grails.geb.recording.mode` + * purpose: which tests to record + * possible values: `SKIP`, `RECORD_ALL`, or `RECORD_FAILING` + * defaults to `RECORD_FAILING` * `grails.geb.recording.directory` @@ -67,12 +69,6 @@ The following system properties can be change to configure the recording behavio * defaults to `build/recordings` -* `grails.geb.recording.mode` - * purpose: which tests to record via the enum `VncRecordingMode` - * possible values: `RECORD_ALL` or `RECORD_FAILING` - * defaults to `RECORD_FAILING` - - * `grails.geb.recording.format` * purpose: sets the format of the recording * possible values are `FLV` or `MP4` diff --git a/src/testFixtures/groovy/grails/plugin/geb/ContainerAwareDownloadSupport.groovy b/src/testFixtures/groovy/grails/plugin/geb/ContainerAwareDownloadSupport.groovy index 3663d3d..407ddbe 100644 --- a/src/testFixtures/groovy/grails/plugin/geb/ContainerAwareDownloadSupport.groovy +++ b/src/testFixtures/groovy/grails/plugin/geb/ContainerAwareDownloadSupport.groovy @@ -35,7 +35,7 @@ import java.util.regex.Pattern * setups, ensuring the host network context is used for download requests.

* * @author Mattias Reichel - * @since 5.0.0 + * @since 5.0 */ @CompileStatic @SelfType(ContainerGebSpec) @@ -45,6 +45,7 @@ trait ContainerAwareDownloadSupport implements DownloadSupport { private final DownloadSupport downloadSupport = new LocalhostDownloadSupport(browser, this) abstract Browser getBrowser() + abstract String getHostNameFromHost() private static class LocalhostDownloadSupport extends DefaultDownloadSupport { diff --git a/src/testFixtures/groovy/grails/plugin/geb/ContainerGebConfiguration.groovy b/src/testFixtures/groovy/grails/plugin/geb/ContainerGebConfiguration.groovy index 7f1a33b..a3afa2b 100644 --- a/src/testFixtures/groovy/grails/plugin/geb/ContainerGebConfiguration.groovy +++ b/src/testFixtures/groovy/grails/plugin/geb/ContainerGebConfiguration.groovy @@ -17,30 +17,25 @@ package grails.plugin.geb import groovy.transform.CompileStatic import groovy.transform.Memoized -import groovy.transform.PackageScope import groovy.util.logging.Slf4j import org.testcontainers.containers.BrowserWebDriverContainer import org.testcontainers.containers.VncRecordingContainer /** - * A container class to parse the configuration used by {@link grails.plugin.geb.ContainerGebRecordingExtension} + * Handles parsing various configuration used by {@link grails.plugin.geb.ContainerGebRecordingExtension} * * @author James Daugherty - * @since 5.0.0 + * @since 5.0 */ @Slf4j @CompileStatic class ContainerGebConfiguration { - String recordingDirectoryName - - boolean recording + String recordingDirectoryName BrowserWebDriverContainer.VncRecordingMode recordingMode - VncRecordingContainer.VncRecordingFormat recordingFormat ContainerGebConfiguration() { - recording = Boolean.parseBoolean(System.getProperty('grails.geb.recording.enabled', true as String)) recordingDirectoryName = System.getProperty('grails.geb.recording.directory', 'build/recordings') recordingMode = BrowserWebDriverContainer.VncRecordingMode.valueOf(System.getProperty('grails.geb.recording.mode', BrowserWebDriverContainer.VncRecordingMode.RECORD_FAILING.name())) recordingFormat = VncRecordingContainer.VncRecordingFormat.valueOf(System.getProperty('grails.geb.recording.format', VncRecordingContainer.VncRecordingFormat.MP4.name())) @@ -48,19 +43,18 @@ class ContainerGebConfiguration { @Memoized File getRecordingDirectory() { - if(!recording) { + if (recordingMode == BrowserWebDriverContainer.VncRecordingMode.SKIP) { return null } File recordingDirectory = new File(recordingDirectoryName) - if(!recordingDirectory.exists()) { - log.info("Could not find `${recordingDirectoryName}` directory for recording. Creating...") - recordingDirectory.mkdir() - } - else if(!recordingDirectory.isDirectory()) { + if (!recordingDirectory.exists()) { + log.info("Could not find `{}` directory for recording. Creating...", recordingDirectoryName) + recordingDirectory.mkdirs() + } else if (!recordingDirectory.isDirectory()) { throw new IllegalStateException("Recording Directory name expected to be `${recordingDirectoryName}, but found file instead.") } - recordingDirectory + return recordingDirectory } } diff --git a/src/testFixtures/groovy/grails/plugin/geb/ContainerGebRecordingExtension.groovy b/src/testFixtures/groovy/grails/plugin/geb/ContainerGebRecordingExtension.groovy index 83c937c..936c4fb 100644 --- a/src/testFixtures/groovy/grails/plugin/geb/ContainerGebRecordingExtension.groovy +++ b/src/testFixtures/groovy/grails/plugin/geb/ContainerGebRecordingExtension.groovy @@ -20,6 +20,7 @@ import groovy.transform.TailRecursive import groovy.util.logging.Slf4j import org.spockframework.runtime.extension.IGlobalExtension import org.spockframework.runtime.model.SpecInfo +import org.testcontainers.containers.BrowserWebDriverContainer import java.time.LocalDateTime @@ -27,7 +28,7 @@ import java.time.LocalDateTime * A Spock Extension that manages the Testcontainers lifecycle for a {@link grails.plugin.geb.ContainerGebSpec} * * @author James Daugherty - * @since 5.0.0 + * @since 5.0 */ @Slf4j @CompileStatic @@ -41,14 +42,19 @@ class ContainerGebRecordingExtension implements IGlobalExtension { @Override void visitSpec(SpecInfo spec) { - if (isContainerizedGebSpec(spec)) { + if (isContainerGebSpec(spec)) { ContainerGebTestListener listener = new ContainerGebTestListener(spec, LocalDateTime.now()) // TODO: We should initialize the web driver container once for all geb tests so we don't have to spin it up & down. spec.addSetupInterceptor { ContainerGebSpec gebSpec = it.instance as ContainerGebSpec gebSpec.initialize() - if(configuration.recording) { - listener.webDriverContainer = gebSpec.webDriverContainer.withRecordingMode(configuration.recordingMode, configuration.recordingDirectory, configuration.recordingFormat) + if (configuration.recordingMode != BrowserWebDriverContainer.VncRecordingMode.SKIP) { + listener.webDriverContainer = gebSpec.webDriverContainer + .withRecordingMode( + configuration.recordingMode, + configuration.recordingDirectory, + configuration.recordingFormat + ) } } spec.addCleanupInterceptor { @@ -61,13 +67,12 @@ class ContainerGebRecordingExtension implements IGlobalExtension { } @TailRecursive - private boolean isContainerizedGebSpec(SpecInfo spec) { - if(spec != null) { - if(spec.filename.startsWith('ContainerGebSpec.')) { + private boolean isContainerGebSpec(SpecInfo spec) { + if (spec != null) { + if (spec.filename.startsWith('ContainerGebSpec.')) { return true } - - return isContainerizedGebSpec(spec.superSpec) + return isContainerGebSpec(spec.superSpec) } return false } diff --git a/src/testFixtures/groovy/grails/plugin/geb/ContainerGebSpec.groovy b/src/testFixtures/groovy/grails/plugin/geb/ContainerGebSpec.groovy index 208a719..9eb0259 100644 --- a/src/testFixtures/groovy/grails/plugin/geb/ContainerGebSpec.groovy +++ b/src/testFixtures/groovy/grails/plugin/geb/ContainerGebSpec.groovy @@ -46,7 +46,7 @@ import java.time.Duration * * @author Søren Berg Glasius * @author Mattias Reichel - * @since 5.0.0 + * @since 5.0 */ @DynamicallyDispatchesToBrowser abstract class ContainerGebSpec extends Specification implements ManagedGebTest, ContainerAwareDownloadSupport { @@ -72,7 +72,7 @@ abstract class ContainerGebSpec extends Specification implements ManagedGebTest, @PackageScope void initialize() { - if(webDriverContainer) { + if (webDriverContainer) { return } diff --git a/src/testFixtures/groovy/grails/plugin/geb/ContainerGebTestDescription.groovy b/src/testFixtures/groovy/grails/plugin/geb/ContainerGebTestDescription.groovy index ea1b8d4..0002123 100644 --- a/src/testFixtures/groovy/grails/plugin/geb/ContainerGebTestDescription.groovy +++ b/src/testFixtures/groovy/grails/plugin/geb/ContainerGebTestDescription.groovy @@ -26,19 +26,18 @@ import java.time.format.DateTimeFormatter * Implements {@link org.testcontainers.lifecycle.TestDescription} to customize recording names. * * @see org.testcontainers.lifecycle.TestDescription - * * @author James Daugherty * @since 5.0 */ @CompileStatic class ContainerGebTestDescription implements TestDescription { + String testId String filesystemFriendlyName ContainerGebTestDescription(IterationInfo testInfo, LocalDateTime runDate) { testId = testInfo.displayName - - String safeName = testId.replaceAll("\\W+", "") + String safeName = testId.replaceAll('\\W+', '_') filesystemFriendlyName = "${DateTimeFormatter.ofPattern("yyyyMMdd_HHmmss").format(runDate)}_${safeName}" } } diff --git a/src/testFixtures/groovy/grails/plugin/geb/ContainerGebTestListener.groovy b/src/testFixtures/groovy/grails/plugin/geb/ContainerGebTestListener.groovy index b9fed69..059b38b 100644 --- a/src/testFixtures/groovy/grails/plugin/geb/ContainerGebTestListener.groovy +++ b/src/testFixtures/groovy/grails/plugin/geb/ContainerGebTestListener.groovy @@ -35,10 +35,11 @@ import java.time.LocalDateTime */ @CompileStatic class ContainerGebTestListener extends AbstractRunListener { + BrowserWebDriverContainer webDriverContainer - ErrorInfo errorInfo - SpecInfo spec - LocalDateTime runDate + ErrorInfo errorInfo + SpecInfo spec + LocalDateTime runDate ContainerGebTestListener(SpecInfo spec, LocalDateTime runDate) { this.spec = spec @@ -47,7 +48,10 @@ class ContainerGebTestListener extends AbstractRunListener { @Override void afterIteration(IterationInfo iteration) { - webDriverContainer.afterTest(new ContainerGebTestDescription(iteration, runDate), Optional.of(errorInfo?.exception)) + webDriverContainer.afterTest( + new ContainerGebTestDescription(iteration, runDate), + Optional.ofNullable(errorInfo?.exception) + ) errorInfo = null }