matrei commented on code in PR #15067:
URL: https://github.com/apache/grails-core/pull/15067#discussion_r2372600421
##########
grails-geb/src/testFixtures/groovy/grails/plugin/geb/WebDriverContainerHolder.groovy:
##########
@@ -270,58 +367,117 @@ class WebDriverContainerHolder {
Class<? extends ContainerFileDetector> fileDetector
WebDriverContainerConfiguration(SpecInfo spec) {
- ContainerGebConfiguration configuration
+
+ ContainerGebConfiguration conf
// Check if the class implements the interface
if (IContainerGebConfiguration.isAssignableFrom(spec.reflection)) {
- configuration = spec.reflection.getConstructor().newInstance()
as ContainerGebConfiguration
+ conf = spec.reflection.getConstructor().newInstance() as
ContainerGebConfiguration
} else {
// Check for the annotation
- configuration = spec.annotations.find {
+ conf = spec.annotations.find {
it.annotationType() == ContainerGebConfiguration
} as ContainerGebConfiguration
}
- protocol = configuration?.protocol() ?:
ContainerGebConfiguration.DEFAULT_PROTOCOL
- hostName = configuration?.hostName() ?:
ContainerGebConfiguration.DEFAULT_HOSTNAME_FROM_CONTAINER
- reporting = configuration?.reporting() ?: false
- fileDetector = configuration?.fileDetector() ?:
ContainerGebConfiguration.DEFAULT_FILE_DETECTOR
+ protocol = conf?.protocol() ?:
ContainerGebConfiguration.DEFAULT_PROTOCOL
+ hostName = conf?.hostName() ?:
ContainerGebConfiguration.DEFAULT_HOSTNAME_FROM_CONTAINER
+ reporting = conf?.reporting() ?: false
+ fileDetector = conf?.fileDetector() ?:
ContainerGebConfiguration.DEFAULT_FILE_DETECTOR
}
}
/**
* Workaround for
https://github.com/testcontainers/testcontainers-java/issues/3998
- * Restarts the VNC recording container to enable separate recording files
for each test method.
- * This method uses reflection to access the VNC recording container field
in BrowserWebDriverContainer.
- * Should be called BEFORE each test starts.
+ * <p>
+ * Restarts the VNC recording container to enable separate recording files
for each
+ * test method. This method uses reflection to access the VNC recording
container
+ * field in BrowserWebDriverContainer. Should be called BEFORE each test
starts.
*/
void restartVncRecordingContainer() {
- if (!grailsGebSettings.recordingEnabled ||
!grailsGebSettings.restartRecordingContainerPerTest || !currentContainer) {
+ if (!settings.recordingEnabled ||
!settings.restartRecordingContainerPerTest || !container) {
return
}
try {
// Use reflection to access the VNC recording container field
- Field vncRecordingContainerField =
BrowserWebDriverContainer.getDeclaredField('vncRecordingContainer')
- vncRecordingContainerField.setAccessible(true)
-
- VncRecordingContainer vncContainer =
vncRecordingContainerField.get(currentContainer) as VncRecordingContainer
+ def field =
BrowserWebDriverContainer.getDeclaredField('vncRecordingContainer').tap {
+ accessible = true
+ }
+ def vncContainer = field.get(container) as VncRecordingContainer
if (vncContainer) {
// Stop the current VNC recording container
vncContainer.stop()
// Create and start a new VNC recording container for the next
test
- VncRecordingContainer newVncContainer = new
VncRecordingContainer(currentContainer)
+ def newVncContainer = new VncRecordingContainer(container)
.withVncPassword('secret')
.withVncPort(5900)
- .withVideoFormat(grailsGebSettings.recordingFormat)
- vncRecordingContainerField.set(currentContainer,
newVncContainer)
+ .withVideoFormat(settings.recordingFormat)
+ field.set(container, newVncContainer)
newVncContainer.start()
log.debug('Successfully restarted VNC recording container')
}
} catch (Exception e) {
- log.warn("Failed to restart VNC recording container:
${e.message}", e)
+ log.warn("Failed to restart VNC recording container: $e.message",
e)
// Don't throw the exception to avoid breaking the test execution
}
}
+
+ @CompileStatic
+ private static class ClosureDecorators {
+
+ /**
+ * Wraps a closure so that during its execution,
System.getProperty(key)
+ * returns a custom value instead of what is actually in the system
properties.
+ */
+ static Closure withSystemProperty(Closure target, String key, Object
value) {
+ Closure wrapped = { Object... args ->
+ SysPropScope.withProperty(key, value.toString()) {
+ InvokerHelper.invokeClosure(target, args)
+ }
+ }
+
+ // keep original closure semantics
+ wrapped.rehydrate(target.delegate, target.owner,
target.thisObject).tap {
+ resolveStrategy = target.resolveStrategy
+ }
+ }
+
+ @CompileStatic
+ private static class SysPropScope {
+
+ private static final ThreadLocal<Map<String,String>>
OVERRIDDEN_SYSTEM_PROPERTIES =
+ ThreadLocal.withInitial { [:] as Map<String,String> }
+
+ @Lazy // Thread-safe wrapping of system properties
+ private static Properties propertiesWrappedOnFirstAccess = {
+ new InterceptingProperties().tap {
+ putAll(System.getProperties())
+ System.setProperties(it)
+ }
+ }()
+
+ static <T> T withProperty(String key, String value, Closure<T>
body) {
+ propertiesWrappedOnFirstAccess // Access property to trigger
property wrapping
+ def map = OVERRIDDEN_SYSTEM_PROPERTIES.get()
+ def prev = map.put(key, value)
+ try {
+ return body.call()
+ } finally {
+ if (prev == null) map.remove(key) else map[key] = prev
+ if (map.isEmpty()) OVERRIDDEN_SYSTEM_PROPERTIES.remove()
Review Comment:
No, it should stay.
```groovy
def c = ClosureDecorators.withSystemProperty({
println System.getProperty('my.custom.property') // should print
"customValue"
}, 'my.custom.property', 'customValue')
c()
new Thread({
sleep 1000
c()
}).start()
println System.getProperty('my.custom.property') // should print original
value, probably null
```
```console
customValue
null
customValue
```
Without it:
```console
customValue
customValue
customValue
```
--
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]
