-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add "VncRecordingContainer" #526
Changes from 1 commit
7f599e8
0020cba
0f09dbe
fd409b9
176eafe
067f2d2
d283f0e
bd5eb9b
1add62a
f0c6336
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
package org.testcontainers.containers; | ||
|
||
import com.github.dockerjava.api.model.ContainerNetwork; | ||
import com.github.dockerjava.api.model.Frame; | ||
import lombok.Getter; | ||
import lombok.NonNull; | ||
|
@@ -10,6 +11,7 @@ | |
import org.rnorth.ducttape.TimeoutException; | ||
import org.rnorth.ducttape.unreliables.Unreliables; | ||
import org.testcontainers.containers.output.FrameConsumerResultCallback; | ||
import org.testcontainers.utility.Base58; | ||
import org.testcontainers.utility.TestcontainersConfiguration; | ||
|
||
import java.io.Closeable; | ||
|
@@ -19,6 +21,7 @@ | |
import java.nio.file.StandardCopyOption; | ||
import java.util.concurrent.CountDownLatch; | ||
import java.util.concurrent.TimeUnit; | ||
import java.util.function.Supplier; | ||
|
||
/** | ||
* 'Sidekick container' with the sole purpose of recording the VNC screen output from another container. | ||
|
@@ -34,7 +37,7 @@ public class VncRecordingContainer extends GenericContainer<VncRecordingContaine | |
|
||
public static final int DEFAULT_VNC_PORT = 5900; | ||
|
||
private final String targetNetworkAlias; | ||
private final Supplier<String> targetContainerIdSupplier; | ||
|
||
private String vncPassword = DEFAULT_VNC_PASSWORD; | ||
|
||
|
@@ -43,22 +46,12 @@ public class VncRecordingContainer extends GenericContainer<VncRecordingContaine | |
private int frameRate = 30; | ||
|
||
public VncRecordingContainer(@NonNull GenericContainer<?> targetContainer) { | ||
this( | ||
targetContainer.getNetwork(), | ||
targetContainer.getNetworkAliases().stream() | ||
.findFirst() | ||
.orElseThrow(() -> new IllegalStateException("Target container must have a network alias")) | ||
); | ||
this(targetContainer::getContainerId); | ||
} | ||
|
||
/** | ||
* Create a sidekick container and attach it to another container. The VNC output of that container will be recorded. | ||
*/ | ||
public VncRecordingContainer(@NonNull Network network, @NonNull String targetNetworkAlias) throws IllegalStateException { | ||
public VncRecordingContainer(@NonNull Supplier<String> targetContainerIdSupplier) { | ||
super(TestcontainersConfiguration.getInstance().getVncRecordedContainerImage()); | ||
|
||
this.targetNetworkAlias = targetNetworkAlias; | ||
withNetwork(network); | ||
this.targetContainerIdSupplier = targetContainerIdSupplier; | ||
|
||
waitingFor(new AbstractWaitStrategy() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Our standard LogWaitStrategy didn't work here and also requires a Regexp while here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps we could/should ship a log wait strategy that is based on Not saying we have to do that now, but keen for your thoughts. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see your point not needing a Regex, but why didn't |
||
|
||
|
@@ -112,13 +105,40 @@ public VncRecordingContainer withFrameRate(int frameRate) { | |
@Override | ||
protected void configure() { | ||
withCreateContainerCmdModifier(it -> it.withEntrypoint("/bin/sh")); | ||
|
||
if (getNetwork() == null) { | ||
withNetwork(Network.newNetwork()); | ||
} | ||
|
||
String alias = "vnchost-" + Base58.randomString(8); | ||
dockerClient.connectToNetworkCmd() | ||
.withContainerId(targetContainerIdSupplier.get()) | ||
.withNetworkId(getNetwork().getId()) | ||
.withContainerNetwork(new ContainerNetwork().withAliases(alias)) | ||
.exec(); | ||
|
||
setCommand( | ||
"-c", | ||
"echo '" + Base64.encodeBase64String(vncPassword.getBytes()) + "' | base64 -d > /vnc_password && " + | ||
"flvrec.py -o " + RECORDING_FILE_NAME + " -d -r " + frameRate + " -P /vnc_password " + targetNetworkAlias + " " + vncPort | ||
"flvrec.py -o " + RECORDING_FILE_NAME + " -d -r " + frameRate + " -P /vnc_password " + alias + " " + vncPort | ||
); | ||
} | ||
|
||
@Override | ||
public void stop() { | ||
try { | ||
dockerClient.disconnectFromNetworkCmd() | ||
.withContainerId(targetContainerIdSupplier.get()) | ||
.withNetworkId(getNetwork().getId()) | ||
.withForce(true) | ||
.exec(); | ||
} catch (Exception e) { | ||
logger().warn("Failed to disconnect container with id '{}' from network '{}'", targetContainerIdSupplier.get(), getNetwork().getId(), e); | ||
} | ||
|
||
super.stop(); | ||
} | ||
|
||
@SneakyThrows | ||
public InputStream streamRecord() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the whole point is having "volume-less" recorder because:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2 is definitely an awesome reason to do this 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a wording thing, but please could this be |
||
TarArchiveInputStream archiveInputStream = new TarArchiveInputStream( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,6 @@ | |
import org.testcontainers.containers.wait.LogMessageWaitStrategy; | ||
import org.testcontainers.containers.wait.WaitAllStrategy; | ||
import org.testcontainers.containers.wait.WaitStrategy; | ||
import org.testcontainers.utility.Base58; | ||
|
||
import java.io.File; | ||
import java.net.MalformedURLException; | ||
|
@@ -102,14 +101,6 @@ protected void configure() { | |
} | ||
|
||
if (recordingMode != VncRecordingMode.SKIP) { | ||
if (getNetwork() == null) { | ||
withNetwork(Network.SHARED); | ||
} | ||
|
||
if (getNetworkAliases().isEmpty()) { | ||
withNetworkAliases("vnchost-" + Base58.randomString(8)); | ||
} | ||
|
||
vncRecordingContainer = new VncRecordingContainer(this) | ||
.withVncPassword(DEFAULT_PASSWORD) | ||
.withVncPort(VNC_PORT); | ||
|
@@ -177,15 +168,15 @@ public int getPort() { | |
|
||
@Override | ||
protected void containerIsStarted(InspectContainerResponse containerInfo) { | ||
if (vncRecordingContainer != null) { | ||
LOGGER.debug("Starting VNC recording"); | ||
vncRecordingContainer.start(); | ||
} | ||
|
||
driver = Unreliables.retryUntilSuccess(30, TimeUnit.SECONDS, | ||
Timeouts.getWithTimeout(1, TimeUnit.SECONDS, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not directly related to this PR, but sometimes it fails to init, and this code waits for 10s while 1s is enough There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
() -> | ||
() -> new RemoteWebDriver(getSeleniumAddress(), desiredCapabilities))); | ||
|
||
if (vncRecordingContainer != null) { | ||
LOGGER.debug("Starting VNC recording"); | ||
vncRecordingContainer.start(); | ||
} | ||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ | |
import java.util.concurrent.TimeUnit; | ||
|
||
import static org.rnorth.visibleassertions.VisibleAssertions.assertEquals; | ||
import static org.rnorth.visibleassertions.VisibleAssertions.assertTrue; | ||
|
||
/** | ||
* | ||
|
@@ -37,6 +38,9 @@ private static RemoteWebDriver setupDriverFromRule(BrowserWebDriverContainer rul | |
protected static void doSimpleExplore(BrowserWebDriverContainer rule) { | ||
RemoteWebDriver driver = setupDriverFromRule(rule); | ||
driver.get("http://en.wikipedia.org/wiki/Randomness"); | ||
|
||
// Oh! The irony! | ||
assertTrue("Randomness' description has the word 'pattern'", driver.findElementByPartialLinkText("pattern").isDisplayed()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Leading to Wiki authors breaking our tests 🤣 |
||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thought it's nice to have it configurable :) Some people do crazy stuff during their tests :D