Skip to content
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

Merged
merged 10 commits into from
Dec 19, 2017
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file.

### Changed
- Added `getDatabaseName` method to JdbcDatabaseContainer, MySQLContainer, PostgreSQLContainer ([\#473](https://github.com/testcontainers/testcontainers-java/issues/473))
- Added `VncRecordingContainer` - Network-based, attachable re-implementation of `VncRecordingSidekickContainer` ([\#526](https://github.com/testcontainers/testcontainers-java/pull/526))

## [1.5.0] - 2017-12-12
### Fixed
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
package org.testcontainers.containers;

import com.github.dockerjava.api.model.ContainerNetwork;
import com.github.dockerjava.api.model.Frame;
import lombok.Getter;
import lombok.NonNull;
import lombok.SneakyThrows;
import lombok.ToString;
import org.apache.commons.codec.binary.Base64;
import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
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;
import java.io.File;
import java.io.InputStream;
import java.nio.file.Files;
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.
*
*/
@Getter
@ToString
public class VncRecordingContainer extends GenericContainer<VncRecordingContainer> {

private static final String RECORDING_FILE_NAME = "/screen.flv";

public static final String DEFAULT_VNC_PASSWORD = "secret";

public static final int DEFAULT_VNC_PORT = 5900;

private final Supplier<String> targetContainerIdSupplier;

private String vncPassword = DEFAULT_VNC_PASSWORD;

private int vncPort = 5900;

private int frameRate = 30;
Copy link
Member Author

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


public VncRecordingContainer(@NonNull GenericContainer<?> targetContainer) {
this(targetContainer::getContainerId);
}

public VncRecordingContainer(@NonNull Supplier<String> targetContainerIdSupplier) {
super(TestcontainersConfiguration.getInstance().getVncRecordedContainerImage());
this.targetContainerIdSupplier = targetContainerIdSupplier;

waitingFor(new AbstractWaitStrategy() {
Copy link
Member Author

Choose a reason for hiding this comment

The 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 contains is more than enough. Also, in debug mode vncrec.py produces a lot of output, makes sense to do as little as possible here

Copy link
Member

Choose a reason for hiding this comment

The 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 contains anyway. It seems to me that LogWaitStrategy is used for contains semantics more often than for a full blown regex.

Not saying we have to do that now, but keen for your thoughts.

Copy link
Member

Choose a reason for hiding this comment

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

I see your point not needing a Regex, but why didn't LogWaitStrategy work?


@Override
protected void waitUntilReady() {
try {
Unreliables.retryUntilTrue((int) startupTimeout.toMillis(), TimeUnit.MILLISECONDS, () -> {
CountDownLatch latch = new CountDownLatch(1);

FrameConsumerResultCallback callback = new FrameConsumerResultCallback() {
@Override
public void onNext(Frame frame) {
if (frame != null && new String(frame.getPayload()).contains("Connected")) {
latch.countDown();
}
}
};

try (
Closeable __ = dockerClient.logContainerCmd(containerId)
Copy link
Member Author

Choose a reason for hiding this comment

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

__ == unused

Copy link
Member

Choose a reason for hiding this comment

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

Starting with Java 9, underscores won't be legal variable names (Java Language Changes for Java SE 9) (okay, maybe double underscore is fine 😜 )

.withFollowStream(true)
.withSince(0)
.withStdErr(true)
.exec(callback)
) {
return latch.await(1, TimeUnit.SECONDS);
}
});
} catch (TimeoutException e) {
throw new ContainerLaunchException("Timed out waiting for log output", e);
}
}
});
}

public VncRecordingContainer withVncPassword(@NonNull String vncPassword) {
this.vncPassword = vncPassword;
return this;
}

public VncRecordingContainer withVncPort(int vncPort) {
this.vncPort = vncPort;
return this;
}

public VncRecordingContainer withFrameRate(int frameRate) {
this.frameRate = frameRate;
return this;
}

@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 " + 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() {
Copy link
Member Author

Choose a reason for hiding this comment

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

the whole point is having "volume-less" recorder because:

  1. for some reason, on Mac, files were incomplete (read - empty) once a recording is flushed to the FS ( See VNC recorder for selenium container creates empty video files #466 )
  2. allows Docker to be on another host. Not-so-critical for the integration testing, but I've received some feedback about CI setups for functional/e2e testing, people really run their Docker hosts "somewhere else"

Copy link
Member

Choose a reason for hiding this comment

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

2 is definitely an awesome reason to do this 👍

Copy link
Member

Choose a reason for hiding this comment

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

Just a wording thing, but please could this be streamRecording? The same for saveRecordToFile (->saveRecordingToFile)

TarArchiveInputStream archiveInputStream = new TarArchiveInputStream(
dockerClient.copyArchiveFromContainerCmd(containerId, RECORDING_FILE_NAME).exec()
);
archiveInputStream.getNextEntry();
return archiveInputStream;
}

@SneakyThrows
public void saveRecordToFile(File file) {
try(InputStream inputStream = streamRecord()) {
Files.copy(inputStream, file.toPath(), StandardCopyOption.REPLACE_EXISTING);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@

/**
* 'Sidekick container' with the sole purpose of recording the VNC screen output from another container.
*
* @deprecated please use {@link VncRecordingContainer}
*/
@Deprecated
public class VncRecordingSidekickContainer<SELF extends VncRecordingSidekickContainer<SELF, T>, T extends VncService & LinkableContainer> extends GenericContainer<SELF> {
private final T vncServiceContainer;
private final Path tempDir;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
/**
* A container which exposes a VNC server.
*/
@Deprecated
public interface VncService {
/**
* @return a URL which can be used to connect to the VNC server from the machine running the tests. Exposed for convenience, e.g. to aid manual debugging
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
import java.net.MalformedURLException;
import java.net.URL;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Set;
import java.util.concurrent.TimeUnit;

Expand Down Expand Up @@ -54,7 +52,7 @@ public class BrowserWebDriverContainer<SELF extends BrowserWebDriverContainer<SE
private RecordingFileFactory recordingFileFactory;
private File vncRecordingDirectory = new File("/tmp");

private final Collection<VncRecordingSidekickContainer> currentVncRecordings = new ArrayList<>();
private VncRecordingContainer vncRecordingContainer = null;

private static final Logger LOGGER = LoggerFactory.getLogger(BrowserWebDriverContainer.class);

Expand Down Expand Up @@ -102,6 +100,12 @@ protected void configure() {
throw new IllegalStateException();
}

if (recordingMode != VncRecordingMode.SKIP) {
vncRecordingContainer = new VncRecordingContainer(this)
.withVncPassword(DEFAULT_PASSWORD)
.withVncPort(VNC_PORT);
}

if (!customImageNameIsSet) {
super.setDockerImageName(getImageForCapabilities(desiredCapabilities));
}
Expand Down Expand Up @@ -164,19 +168,15 @@ public int getPort() {

@Override
protected void containerIsStarted(InspectContainerResponse containerInfo) {
if (recordingMode != VncRecordingMode.SKIP) {
LOGGER.debug("Starting VNC recording");

VncRecordingSidekickContainer recordingSidekickContainer = new VncRecordingSidekickContainer<>(this);

recordingSidekickContainer.start();
currentVncRecordings.add(recordingSidekickContainer);
}

driver = Unreliables.retryUntilSuccess(30, TimeUnit.SECONDS,
Timeouts.getWithTimeout(10, TimeUnit.SECONDS,
Timeouts.getWithTimeout(1, TimeUnit.SECONDS,
Copy link
Member Author

Choose a reason for hiding this comment

The 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

Copy link
Member

Choose a reason for hiding this comment

The 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();
}
}

/**
Expand All @@ -193,39 +193,41 @@ public RemoteWebDriver getWebDriver() {

@Override
protected void failed(Throwable e, Description description) {
switch (recordingMode) {
case RECORD_FAILING:
case RECORD_ALL:
stopAndRetainRecordingForDescriptionAndSuccessState(description, false);
break;
}
currentVncRecordings.clear();
stopAndRetainRecordingForDescriptionAndSuccessState(description, false);
}

@Override
protected void succeeded(Description description) {
switch (recordingMode) {
case RECORD_ALL:
stopAndRetainRecordingForDescriptionAndSuccessState(description, true);
break;
}
currentVncRecordings.clear();
stopAndRetainRecordingForDescriptionAndSuccessState(description, true);
}

@Override
protected void finished(Description description) {
public void stop() {
if (driver != null) {
driver.quit();
try {
driver.quit();
} catch (Exception e) {
LOGGER.debug("Failed to quit the driver", e);
}
}

if (vncRecordingContainer != null) {
try {
vncRecordingContainer.stop();
} catch (Exception e) {
LOGGER.debug("Failed to stop vncRecordingContainer", e);
}
}
this.stop();

super.stop();
}

private void stopAndRetainRecordingForDescriptionAndSuccessState(Description description, boolean succeeded) {
File recordingFile = recordingFileFactory.recordingFileForTest(vncRecordingDirectory, description, succeeded);
LOGGER.info("Screen recordings for test {} will be stored at: {}", description.getDisplayName(), recordingFile);
if (recordingMode == VncRecordingMode.RECORD_ALL || (!succeeded && recordingMode == VncRecordingMode.RECORD_FAILING)) {
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer a variable for (!succeeded && recordingMode == VncRecordingMode.RECORD_FAILING), since I initially needed some time to parse this boolean expression 😉
Maybe recordFailingTest?

Copy link
Member

Choose a reason for hiding this comment

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

Good idea 👍

File recordingFile = recordingFileFactory.recordingFileForTest(vncRecordingDirectory, description, succeeded);
LOGGER.info("Screen recordings for test {} will be stored at: {}", description.getDisplayName(), recordingFile);

for (VncRecordingSidekickContainer container : currentVncRecordings) {
container.stopAndRetainRecording(recordingFile);
vncRecordingContainer.saveRecordToFile(recordingFile);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.util.concurrent.TimeUnit;

import static org.rnorth.visibleassertions.VisibleAssertions.assertEquals;
import static org.rnorth.visibleassertions.VisibleAssertions.assertTrue;

/**
*
Expand All @@ -28,15 +29,18 @@ protected void doSimpleWebdriverTest(BrowserWebDriverContainer rule) {
}

@NotNull
private RemoteWebDriver setupDriverFromRule(BrowserWebDriverContainer rule) {
private static RemoteWebDriver setupDriverFromRule(BrowserWebDriverContainer rule) {
RemoteWebDriver driver = rule.getWebDriver();
driver.manage().timeouts().implicitlyWait(30, TimeUnit.SECONDS);
return driver;
}

protected void doSimpleExplore(BrowserWebDriverContainer rule) {
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());
Copy link
Member

Choose a reason for hiding this comment

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

Leading to Wiki authors breaking our tests 🤣

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.runners.Enclosed;
import org.junit.runner.RunWith;
import org.openqa.selenium.remote.DesiredCapabilities;
import org.testcontainers.containers.BrowserWebDriverContainer;
import org.testcontainers.containers.DefaultRecordingFileFactory;
Expand All @@ -10,29 +12,32 @@

import static org.testcontainers.containers.BrowserWebDriverContainer.VncRecordingMode.RECORD_ALL;

/**
*
*/
@RunWith(Enclosed.class)
Copy link
Member Author

Choose a reason for hiding this comment

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

had to replace with Enclosed - was using this class for testing, but it was starting 2 containers every time, while only one of them is used to actually run the test

Copy link
Member

Choose a reason for hiding this comment

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

Sounds sensible.

public class ChromeRecordingWebDriverContainerTest extends BaseWebDriverContainerTest {

@Rule
public BrowserWebDriverContainer chromeThatRecordsAllTests = new BrowserWebDriverContainer()
.withDesiredCapabilities(DesiredCapabilities.chrome())
.withRecordingMode(RECORD_ALL, new File("./target/"))
.withRecordingFileFactory(new DefaultRecordingFileFactory());
public static class ChromeThatRecordsAllTests {

@Rule
public BrowserWebDriverContainer chromeThatRecordsFailingTests = new BrowserWebDriverContainer()
.withDesiredCapabilities(DesiredCapabilities.chrome());
@Rule
public BrowserWebDriverContainer chrome = new BrowserWebDriverContainer()
.withDesiredCapabilities(DesiredCapabilities.chrome())
.withRecordingMode(RECORD_ALL, new File("./target/"))
.withRecordingFileFactory(new DefaultRecordingFileFactory());


@Test
public void recordingTestThatShouldBeRecordedButDeleted() {
doSimpleExplore(chromeThatRecordsFailingTests);
@Test
public void recordingTestThatShouldBeRecordedAndRetained() {
doSimpleExplore(chrome);
}
}

@Test
public void recordingTestThatShouldBeRecordedAndRetained() {
doSimpleExplore(chromeThatRecordsAllTests);
public static class ChromeThatRecordsFailingTests {

@Rule
public BrowserWebDriverContainer chrome = new BrowserWebDriverContainer()
.withDesiredCapabilities(DesiredCapabilities.chrome());

@Test
public void recordingTestThatShouldBeRecordedButDeleted() {
doSimpleExplore(chrome);
}
}
}