Skip to content

GenericContainer improvements - #5386

Open
eddumelendez wants to merge 10 commits into
mainfrom
gh-5116
Open

GenericContainer improvements#5386
eddumelendez wants to merge 10 commits into
mainfrom
gh-5116

Conversation

@eddumelendez

@eddumelendez eddumelendez commented May 18, 2022

Copy link
Copy Markdown
Member
  • Avoid configure container multiple times
  • Avoid adding an existing bind

@eddumelendez
eddumelendez requested a review from a team May 18, 2022 23:35

@Container
public static final BrowserWebDriverContainer<?> container = new BrowserWebDriverContainer<>("selenium/standalone-chrome:2.45.0")
.withCapabilities(new ChromeOptions());

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.withCapabilities(new ChromeOptions());
.withCapabilities(new ChromeOptions())
.withRecordingMode(VncRecordingMode.RECORD_ALL, new File("./build"));

If recordingMode is added then tests fail because of checks for internal tests are not executed 🤔

18:37:32.525 INFO  🐳 [selenium/standalone-chrome:2.45.0] - Container selenium/standalone-chrome:2.45.0 is starting: 964500b6f8b7b13c6dd1dfcda7c157aa9d8186f7f449d9a329c45c4ecdbe54a5
18:37:33.073 DEBUG org.testcontainers.containers.output.WaitingConsumer - STDOUT: 18:37:33.175 INFO - Launching a standalone server
18:37:33.121 DEBUG org.testcontainers.containers.output.WaitingConsumer - STDOUT: 18:37:33.206 INFO - Java: Oracle Corporation 24.79-b02
18:37:33.122 DEBUG org.testcontainers.containers.output.WaitingConsumer - STDOUT: 18:37:33.216 INFO - v2.45.0, with Core v2.45.0. Built from revision 5017cb8
18:37:33.122 DEBUG org.testcontainers.containers.output.WaitingConsumer - STDOUT: 18:37:33.206 INFO - OS: Linux 5.4.0-109-generic amd64
18:37:33.169 DEBUG org.testcontainers.containers.output.WaitingConsumer - STDOUT: 18:37:33.273 INFO - Default driver org.openqa.selenium.ie.InternetExplorerDriver registration is skipped: registration capabilities Capabilities [{platform=WINDOWS, ensureCleanSession=true, browserName=internet explorer, version=}] does not match with current platform: LINUX
18:37:33.216 DEBUG org.testcontainers.containers.output.WaitingConsumer - STDOUT: 18:37:33.299 INFO - RemoteWebDriver instances should connect to: http://127.0.0.1:4444/wd/hub
18:37:33.228 DEBUG org.testcontainers.containers.ExecInContainerPattern - /nifty_golick: Running "exec" command: /bin/sh -c while true; do ( true  &&  (grep -i ':0*115c' /proc/net/tcp* || nc -vz -w 1 localhost 4444 || /bin/bash -c '</dev/tcp/localhost/4444') &&  (grep -i ':0*170c' /proc/net/tcp* || nc -vz -w 1 localhost 5900 || /bin/bash -c '</dev/tcp/localhost/5900') ) && exit 0 || sleep 0.1; done
18:37:33.236 DEBUG org.testcontainers.containers.wait.strategy.HostPortWaitStrategy - External port check passed for [4444, 5900] mapped as [58575, 58576] in PT0.01S
18:37:48.119 ERROR 🐳 [selenium/standalone-chrome:2.45.0] - Could not start container
org.rnorth.ducttape.TimeoutException: java.util.concurrent.TimeoutException
	at org.rnorth.ducttape.timeouts.Timeouts.callFuture(Timeouts.java:70)
	at org.rnorth.ducttape.timeouts.Timeouts.doWithTimeout(Timeouts.java:60)
	at org.testcontainers.containers.wait.strategy.WaitAllStrategy.waitUntilReady(WaitAllStrategy.java:53)
	at org.testcontainers.containers.GenericContainer.waitUntilContainerStarted(GenericContainer.java:926)
	at org.testcontainers.containers.GenericContainer.tryStart(GenericContainer.java:480)
	at org.testcontainers.containers.GenericContainer.lambda$doStart$0(GenericContainer.java:340)
	at org.rnorth.ducttape.unreliables.Unreliables.retryUntilSuccess(Unreliables.java:81)
	at org.testcontainers.containers.GenericContainer.doStart(GenericContainer.java:338)
	at org.testcontainers.containers.GenericContainer.start(GenericContainer.java:326)
	at org.testcontainers.junit.jupiter.TestcontainersExtension$StoreAdapter.start(TestcontainersExtension.java:242)
	at org.testcontainers.junit.jupiter.TestcontainersExtension$StoreAdapter.access$200(TestcontainersExtension.java:229)
	at org.testcontainers.junit.jupiter.TestcontainersExtension.lambda$null$1(TestcontainersExtension.java:59)
	at org.junit.jupiter.engine.execution.ExtensionValuesStore.lambda$getOrComputeIfAbsent$4(ExtensionValuesStore.java:86)
	at org.junit.jupiter.engine.execution.ExtensionValuesStore$MemoizingSupplier.computeValue(ExtensionValuesStore.java:223)
	at org.junit.jupiter.engine.execution.ExtensionValuesStore$MemoizingSupplier.get(ExtensionValuesStore.java:211)
	at org.junit.jupiter.engine.execution.ExtensionValuesStore$StoredValue.evaluate(ExtensionValuesStore.java:191)
	at org.junit.jupiter.engine.execution.ExtensionValuesStore$StoredValue.access$100(ExtensionValuesStore.java:171)
	at org.junit.jupiter.engine.execution.ExtensionValuesStore.getOrComputeIfAbsent(ExtensionValuesStore.java:89)
	at org.junit.jupiter.engine.execution.NamespaceAwareStore.getOrComputeIfAbsent(NamespaceAwareStore.java:53)
	at org.testcontainers.junit.jupiter.TestcontainersExtension.lambda$beforeAll$2(TestcontainersExtension.java:59)
	at java.util.ArrayList.forEach(ArrayList.java:1259)
	at org.testcontainers.junit.jupiter.TestcontainersExtension.beforeAll(TestcontainersExtension.java:59)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.lambda$invokeBeforeAllCallbacks$10(ClassBasedTestDescriptor.java:381)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.invokeBeforeAllCallbacks(ClassBasedTestDescriptor.java:381)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.before(ClassBasedTestDescriptor.java:205)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.before(ClassBasedTestDescriptor.java:80)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:148)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at java.util.ArrayList.forEach(ArrayList.java:1259)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:35)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:54)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:107)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:88)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:54)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:67)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:52)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:114)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:86)
	at org.junit.platform.launcher.core.DefaultLauncherSession$DelegatingLauncher.execute(DefaultLauncherSession.java:86)
	at org.junit.platform.launcher.core.SessionPerRequestLauncher.execute(SessionPerRequestLauncher.java:53)
	at com.intellij.junit5.JUnit5IdeaTestRunner.startRunnerWithArgs(JUnit5IdeaTestRunner.java:71)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater$1.execute(IdeaTestRunner.java:38)
	at com.intellij.rt.execution.junit.TestsRepeater.repeat(TestsRepeater.java:11)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:35)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:235)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:54)
Caused by: java.util.concurrent.TimeoutException: null
	at java.util.concurrent.FutureTask.get(FutureTask.java:205)
	at org.rnorth.ducttape.timeouts.Timeouts.callFuture(Timeouts.java:65)
	... 62 common frames omitted

Comment on lines +332 to +335
if (!this.configured) {
configure();
configured = true;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should probably synchronize this block, as otherwise concurrent invocations of start() (e.g. with Startables.deepStart()) would race

import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;

class GH5116FirstTest extends BaseTest {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

given testContainerIsAlreadyConfiguredWhenUsedMultipleTimes, do you still think we need these tests? Browser containers are expensive, would be nice to avoid having to start them if not needed (although I appreciate the full reproducer according to the original issue 💯)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added a better description of what we are improving in this PR. Indeed, we can remove those expensive test, currently those are helping to test the duplicate binding which I think can use small images.

@eddumelendez eddumelendez changed the title Fix selenium: avoid adding an existing bind GenericContainer improvements May 26, 2022
@eddumelendez
eddumelendez requested a review from a team as a code owner August 19, 2025 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants