Investigating fixes for same recordings

spock-container-test-app/src/integration-test/groovy/org/demo/spock/DownloadSupportSpec.groovy

@@ -13,4 +13,12 @@ class DownloadSupportSpec extends ContainerGebSpec {
         then:
         downloadText().contains('Welcome to Grails')
     }
+
+    void 'should display the correct title on the home page'() {
+        when: 'visiting the home page'
+        go '/'
+
+        then: 'the page title is correct'
+        title == 'Welcome to Grails'
+    }
 }

src/testFixtures/groovy/grails/plugin/geb/GebRecordingTestListener.groovy

@@ -16,9 +16,15 @@
 package grails.plugin.geb
 
 import groovy.transform.CompileStatic
+import groovy.util.logging.Slf4j
+import org.junit.runner.Description
 import org.spockframework.runtime.AbstractRunListener
 import org.spockframework.runtime.model.ErrorInfo
 import org.spockframework.runtime.model.IterationInfo
+import org.testcontainers.containers.BrowserWebDriverContainer
+import org.testcontainers.containers.DefaultRecordingFileFactory
+import org.testcontainers.containers.RecordingFileFactory
+import org.testcontainers.containers.VncRecordingContainer
 
 /**
  * A test listener that reports the test result to {@link org.testcontainers.containers.BrowserWebDriverContainer} so
@@ -29,23 +35,71 @@ import org.spockframework.runtime.model.IterationInfo
  * @author James Daugherty
  * @since 4.1
  */
+@Slf4j
 @CompileStatic
 class GebRecordingTestListener extends AbstractRunListener {
 
-    WebDriverContainerHolder containerHolder
     ErrorInfo errorInfo
+    VncRecordingContainer recordingContainer
+    WebDriverContainerHolder containerHolder
+    RecordingFileFactory recordingFileFactory = new DefaultRecordingFileFactory()
 
     GebRecordingTestListener(WebDriverContainerHolder containerHolder) {
         this.containerHolder = containerHolder
     }
 
+    @Override
+    void beforeIteration(IterationInfo iteration) {
+        recordingContainer = new VncRecordingContainer(containerHolder.currentContainer)
+                .withVncPassword("secret")
+                .withVncPort(5900)
+                .withVideoFormat(containerHolder.grailsGebSettings.recordingFormat)
+
+        recordingContainer.start()
+    }
+
     @Override
     void afterIteration(IterationInfo iteration) {
-        containerHolder.currentContainer.afterTest(
-                new ContainerGebTestDescription(iteration),
-                Optional.ofNullable(errorInfo?.exception)
-        )
+        ContainerGebTestDescription description = new ContainerGebTestDescription(iteration)
+
+        retainRecordingIfNeeded(description.getFilesystemFriendlyName(), errorInfo?.exception == null)
         errorInfo = null
+
+        recordingContainer.stop()
+        recordingContainer = null
+    }
+
+    /**
+     * This method is copied from BrowserWebDriverContainer until upstream allows triggering a recording
+     */
+    private void retainRecordingIfNeeded(String prefix, boolean succeeded) {
+        final boolean shouldRecord
+        switch (containerHolder.grailsGebSettings.recordingMode) {
+            case BrowserWebDriverContainer.VncRecordingMode.RECORD_ALL:
+                shouldRecord = true
+                break
+            case BrowserWebDriverContainer.VncRecordingMode.RECORD_FAILING:
+                shouldRecord = !succeeded
+                break
+            default:
+                shouldRecord = false
+                break
+        }
+
+        if (shouldRecord) {
+            File recordingFile = recordingFileFactory.recordingFileForTest(
+                    containerHolder.grailsGebSettings.recordingDirectory,
+                    prefix,
+                    succeeded,
+                    recordingContainer.getVideoFormat()
+            )
+            log.info("Screen recordings for test {} will be stored at: {}", prefix, recordingFile)
+            //TODO: There's some type of race condition here, since the file will always exist if you use the debugger, but when running the test through it errors with a message like:
+            /*
+            Status 404: {"message":"Could not find the file /newScreen.mp4 in container 044005e429dd268d8149b5c8cda5b1c29d815f6b84705545bc62962bbde9851f"}
+             */
+            recordingContainer.saveRecordingToFile(recordingFile)
+        }
     }
 
     @Override

src/testFixtures/groovy/grails/plugin/geb/WebDriverContainerHolder.groovy

@@ -16,6 +16,7 @@
 package grails.plugin.geb
 
 import com.github.dockerjava.api.model.ContainerNetwork
+import com.google.common.collect.ImmutableSet
 import geb.Browser
 import geb.Configuration
 import geb.spock.SpockGebTestManagerBuilder
@@ -24,13 +25,15 @@ import groovy.transform.CompileStatic
 import groovy.transform.EqualsAndHashCode
 import groovy.transform.PackageScope
 import groovy.util.logging.Slf4j
+import org.jetbrains.annotations.NotNull
 import org.openqa.selenium.WebDriver
 import org.openqa.selenium.chrome.ChromeOptions
 import org.openqa.selenium.remote.RemoteWebDriver
 import org.spockframework.runtime.extension.IMethodInvocation
 import org.spockframework.runtime.model.SpecInfo
 import org.testcontainers.Testcontainers
 import org.testcontainers.containers.BrowserWebDriverContainer
+import org.testcontainers.containers.Network
 import org.testcontainers.containers.PortForwardingContainer
 
 import java.time.Duration
@@ -98,11 +101,12 @@ class WebDriverContainerHolder {
         }
 
         currentConfiguration = specConfiguration
-        currentContainer = new BrowserWebDriverContainer().withRecordingMode(
-                grailsGebSettings.recordingMode,
-                grailsGebSettings.recordingDirectory,
-                grailsGebSettings.recordingFormat
-        )
+
+        // Recording needs to be restarted after every test, so do the configuration in the listener
+        currentContainer = new GrailsBrowserWebDriverContainer(grailsGebSettings.recordingMode)
+        .withRecordingMode(BrowserWebDriverContainer.VncRecordingMode.SKIP, null)
+        .withNetwork(Network.SHARED) as BrowserWebDriverContainer
+
         currentContainer.tap {
             withEnv('SE_ENABLE_TRACING', grailsGebSettings.tracingEnabled)
             withAccessToHost(true)
@@ -212,3 +216,33 @@ class WebDriverContainerHolder {
     }
 }
 
+/**
+ * The only reason this class exists is so we can set the correct liveness ports so the container isn't considered started
+ * until both the selenium server & vnc server are running
+ */
+class GrailsBrowserWebDriverContainer<SELF extends GrailsBrowserWebDriverContainer<SELF>>
+        extends BrowserWebDriverContainer<SELF> {
+    private BrowserWebDriverContainer.VncRecordingMode recordingMode
+
+    GrailsBrowserWebDriverContainer(BrowserWebDriverContainer.VncRecordingMode recordingMode) {
+        // parent recording mode will be set to skip, this is here to just expose the correct liveness check
+        this.recordingMode = recordingMode
+    }
+
+    // Must override to ensure the container isn't considered "ready" until it's vnc server is
+    @NotNull
+    @Override
+    protected Set<Integer> getLivenessCheckPorts() {
+        Integer seleniumPort = getMappedPort(4444) // selenium port
+        if (recordingMode == BrowserWebDriverContainer.VncRecordingMode.SKIP) {
+            return ImmutableSet.of(seleniumPort);
+        } else {
+            return ImmutableSet.of(seleniumPort, getMappedPort(5900)) // VNC port
+        }
+    }
+
+    // This is likely a groovy bug, should not be required since there's a default implementation on Startable
+    void close() {
+        stop()
+    }
+}