diff --git a/grails-geb/src/test/groovy/grails/plugin/geb/WebDriverContainerHolderSpec.groovy b/grails-geb/src/test/groovy/grails/plugin/geb/WebDriverContainerHolderSpec.groovy new file mode 100644 index 00000000000..df960932d40 --- /dev/null +++ b/grails-geb/src/test/groovy/grails/plugin/geb/WebDriverContainerHolderSpec.groovy @@ -0,0 +1,137 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package grails.plugin.geb + +import java.time.LocalDateTime + +import org.testcontainers.containers.BrowserWebDriverContainer +import org.testcontainers.containers.VncRecordingContainer + +import geb.Browser +import geb.test.GebTestManager +import spock.lang.Specification + +import static org.testcontainers.containers.BrowserWebDriverContainer.VncRecordingMode + +class WebDriverContainerHolderSpec extends Specification { + + WebDriverContainerHolder holder = new WebDriverContainerHolder(new GrailsGebSettings(LocalDateTime.now())) + + void 'stop() resets container, browser and testManager on the happy path'() { + given: 'a holder with an initialized container' + def container = Mock(BrowserWebDriverContainer) + holder.container = container + holder.browser = Mock(Browser) + holder.testManager = Mock(GebTestManager) + + when: 'the holder is stopped' + holder.stop() + + then: 'the underlying container is stopped' + 1 * container.stop() + + and: 'all held state is cleared' + holder.container == null + holder.browser == null + holder.testManager == null + !holder.initialized + } + + void 'stop() still resets all held state when container.stop() throws'() { + given: 'a holder whose container fails to stop cleanly' + def container = Mock(BrowserWebDriverContainer) + container.stop() >> { throw new IllegalStateException('boom') } + holder.container = container + holder.browser = Mock(Browser) + holder.testManager = Mock(GebTestManager) + + when: 'the holder is stopped' + holder.stop() + + then: 'the exception from stop() propagates' + thrown(IllegalStateException) + + and: 'held state is still cleared, so a broken container is never reported as initialized' + holder.container == null + holder.browser == null + holder.testManager == null + !holder.initialized + } + + void 'restartVncRecordingContainer() does nothing when recording is disabled'() { + given: + holder.settings.recordingMode = VncRecordingMode.SKIP + holder.settings.restartRecordingContainerPerTest = true + def container = Mock(BrowserWebDriverContainer) + holder.container = container + + when: + holder.restartVncRecordingContainer() + + then: + 0 * container._ + } + + void 'restartVncRecordingContainer() does nothing when per-test restart is disabled'() { + given: + holder.settings.recordingMode = VncRecordingMode.RECORD_ALL + holder.settings.restartRecordingContainerPerTest = false + def container = Mock(BrowserWebDriverContainer) + holder.container = container + + when: + holder.restartVncRecordingContainer() + + then: + 0 * container._ + } + + void 'restartVncRecordingContainer() does nothing when no container has been initialized'() { + given: + holder.settings.recordingMode = VncRecordingMode.RECORD_ALL + holder.settings.restartRecordingContainerPerTest = true + holder.container = null + + expect: 'no exception is thrown even though there is nothing to restart' + holder.restartVncRecordingContainer() + } + + void 'restartVncRecordingContainer() swallows a failure from the current recording container instead of propagating it'() { + given: 'a container whose active VNC recording container fails to stop' + holder.settings.recordingMode = VncRecordingMode.RECORD_ALL + holder.settings.restartRecordingContainerPerTest = true + def container = Mock(BrowserWebDriverContainer) + holder.container = container + + def vncContainer = Mock(VncRecordingContainer) + vncContainer.stop() >> { throw new IllegalStateException('vnc container refused to stop') } + def vncField = BrowserWebDriverContainer.getDeclaredField('vncRecordingContainer') + vncField.accessible = true + vncField.set(container, vncContainer) + + when: 'restarting the recording container' + holder.restartVncRecordingContainer() + + then: 'the failure is logged and swallowed rather than breaking test execution' + noExceptionThrown() + + and: "the field is left untouched - it still points at the container that just failed to stop, not a container that never started" + vncField.get(container).is(vncContainer) + } +} diff --git a/grails-geb/src/testFixtures/groovy/grails/plugin/geb/WebDriverContainerHolder.groovy b/grails-geb/src/testFixtures/groovy/grails/plugin/geb/WebDriverContainerHolder.groovy index 014e61ab176..eace148a1ac 100644 --- a/grails-geb/src/testFixtures/groovy/grails/plugin/geb/WebDriverContainerHolder.groovy +++ b/grails-geb/src/testFixtures/groovy/grails/plugin/geb/WebDriverContainerHolder.groovy @@ -88,11 +88,18 @@ class WebDriverContainerHolder { } void stop() { - container?.stop() - container = null - browser = null - testManager = null - containerConf = null + try { + container?.stop() + } finally { + // Reset state even if stop() throws - otherwise isInitialized() keeps reporting + // true for a container that's actually broken, and a later reinitialize() call + // would see matchesCurrentContainerConfiguration() as a false positive without + // ever attempting to recover. + container = null + browser = null + testManager = null + containerConf = null + } } boolean matchesCurrentContainerConfiguration(WebDriverContainerConfiguration specConf) { @@ -440,13 +447,18 @@ class WebDriverContainerHolder { if (vncContainer) { // Stop the current VNC recording container vncContainer.stop() - // Create and start a new VNC recording container for the next test + // Create and start a new VNC recording container for the next test. + // start() must succeed BEFORE the field is updated: if it throws (e.g. the + // "Connected" wait strategy times out), the exception below is deliberately + // swallowed to avoid breaking test execution - so if the field were already + // pointing at newVncContainer by then, every subsequent saveRecordingToFile() + // would silently target a container that never actually started. def newVncContainer = new VncRecordingContainer(container) .withVncPassword('secret') .withVncPort(5900) .withVideoFormat(settings.recordingFormat) - field.set(container, newVncContainer) newVncContainer.start() + field.set(container, newVncContainer) log.debug('Successfully restarted VNC recording container') } diff --git a/grails-test-examples/geb/src/integration-test/groovy/org/demo/spock/PerTestRecordingSpec.groovy b/grails-test-examples/geb/src/integration-test/groovy/org/demo/spock/PerTestRecordingSpec.groovy index 808d90c9c44..bd22ed58e34 100644 --- a/grails-test-examples/geb/src/integration-test/groovy/org/demo/spock/PerTestRecordingSpec.groovy +++ b/grails-test-examples/geb/src/integration-test/groovy/org/demo/spock/PerTestRecordingSpec.groovy @@ -85,12 +85,35 @@ class PerTestRecordingSpec extends ContainerGebSpec { names.contains('setup_running_a_test_to_create_a_recording') names.contains('setup_running_a_second_test_to_create_another') - and: 'the recording files should have different content' + and: 'each recording captured meaningful content, not just a near-blank connection handshake' + // A VNC recording container that was only just restarted (see + // WebDriverContainerHolder#restartVncRecordingContainer) is guaranteed to have + // connected, but not to have captured more than a frame or two by the time a fast + // iteration finishes. Two such near-blank captures can encode to identical, + // non-zero, stable-sized bytes via ffmpeg - passing a raw byte-difference check + // without actually being distinct, meaningful recordings. Requiring a sensible + // minimum size asserts the real framework contract - a real, played-out recording - + // rather than raw byte inequality of whatever ffmpeg happened to produce. def firstRecording = recordingFiles.find { it.name.contains('setup_running_a_test_to_create_a_recording') } def secondRecording = recordingFiles.find { it.name.contains('setup_running_a_second_test_to_create_another') } + firstRecording.length() > MIN_MEANINGFUL_RECORDING_BYTES + secondRecording.length() > MIN_MEANINGFUL_RECORDING_BYTES + + and: 'the recording files should have different content' + // Kept alongside the size check above rather than dropped in favor of it: the size + // check only rules out near-blank captures, it says nothing about two recordings + // accidentally being the *same* file (e.g. a future regression in how recording + // files are named or matched). The two checks guard against different failure modes. Files.mismatch(firstRecording.toPath(), secondRecording.toPath()) != -1 } + // Two local runs against a real VNC recording container measured every genuine, + // played-out recording at 75KB-855KB (org.demo.spock.PerTestRecordingSpec, recorded + // 2026-07-21 and 2026-07-31). 5,000 bytes stays more than an order of magnitude below + // the smallest of those, while still comfortably clearing a single near-blank keyframe + // from a just-restarted VNC connection. + private static final long MIN_MEANINGFUL_RECORDING_BYTES = 5_000L + private static final DateTimeFormatter RECORDING_DIR_FORMAT = DateTimeFormatter.ofPattern('yyyyMMdd_HHmmss') private static final LocalDateTime JVM_START = LocalDateTime.ofInstant(