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.

Sign up for GitHub

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

Fix port conflict when running multiple unit tests in parallel #220

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix port conflict when running multiple unit tests in parallel #220

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 0 additions & 59 deletions src/main/java/io/specto/hoverfly/junit/api/HoverflyClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import io.specto.hoverfly.junit.api.view.DiffView;
import io.specto.hoverfly.junit.api.view.HoverflyInfoView;
import io.specto.hoverfly.junit.api.view.StateView;
import io.specto.hoverfly.junit.core.HoverflyConstants;
import io.specto.hoverfly.junit.core.HoverflyMode;
import io.specto.hoverfly.junit.core.model.Journal;
import io.specto.hoverfly.junit.core.model.Request;
Expand Down Expand Up @@ -94,62 +93,4 @@ public interface HoverflyClient {
*/
boolean getHealth();

/**
* Static factory method for creating a {@link Builder}
* @return a builder for HoverflyClient
*/
static Builder custom() {
return new Builder();
}

/**
* Static factory method for default Hoverfly client
* @return a default HoverflyClient
*/
static HoverflyClient createDefault() {
return new Builder().build();
}

/**
* HTTP client builder for Hoverfly admin API
*/
class Builder {

private String scheme = HoverflyConstants.HTTP;
private String host = HoverflyConstants.LOCALHOST;
private int port = HoverflyConstants.DEFAULT_ADMIN_PORT;
private String authToken = null;

Builder() {
}

public Builder scheme(String scheme) {
this.scheme = scheme;
return this;
}

public Builder host(String host) {
this.host = host;
return this;
}

public Builder port(int port) {
this.port = port;
return this;
}

/**
* Get token from environment variable "HOVERFLY_AUTH_TOKEN" to authenticate with admin API
* @return this Builder for further customizations
*/
public Builder withAuthToken() {
this.authToken = System.getenv(HoverflyConstants.HOVERFLY_AUTH_TOKEN);
return this;
}

public HoverflyClient build() {
return new OkHttpHoverflyClient(scheme, host, port, authToken);
}
}

Comment on lines -97 to -154
Copy link
Member

@tommysitu tommysitu Oct 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

We should try not to remove public method hoverfly-java users depends on. For example, people may do the following to create a hoverfly client:

HoverflyClient.createDefault()

Why not create a new builder method to set config from a HoverflyConfiguration object, something like this:

HoverflyClient.custom().withConfig(configuration);

}
12 changes: 12 additions & 0 deletions src/main/java/io/specto/hoverfly/junit/api/HoverflyClientFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package io.specto.hoverfly.junit.api;

import io.specto.hoverfly.junit.core.config.HoverflyConfiguration;

/**
* Factory for creating {@link HoverflyClient}s
*/
public interface HoverflyClientFactory {

HoverflyClient createHoverflyClient(HoverflyConfiguration hoverflyConfig);

}
75 changes: 75 additions & 0 deletions src/main/java/io/specto/hoverfly/junit/api/HoverflyOkHttpClientFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package io.specto.hoverfly.junit.api;

import io.specto.hoverfly.junit.core.HoverflyConstants;
import io.specto.hoverfly.junit.core.config.HoverflyConfiguration;

public class HoverflyOkHttpClientFactory implements HoverflyClientFactory {

public HoverflyClient createHoverflyClient(HoverflyConfiguration hoverflyConfig) {
return custom()
.scheme(hoverflyConfig.getScheme())
.host(hoverflyConfig.getHost())
.port(hoverflyConfig.getAdminPort())
.withAuthToken()
Copy link
Member

Choose a reason for hiding this comment

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

withAuthToken() is optional. It's better to expose the builder for the user to customize the client:

HoverflyClient.custom().withConfig(configuration).withAuthToken();

.build();
}

/**
* Static factory method for creating a {@link Builder}
* @return a builder for HoverflyClient
*/
static Builder custom() {
return new Builder();
}

/**
* Static factory method for default Hoverfly client
* @return a default HoverflyClient
*/
static HoverflyClient createDefault() {
return new Builder().build();
}

/**
* HTTP client builder for Hoverfly admin API
*/
static class Builder {

private String scheme = HoverflyConstants.HTTP;
private String host = HoverflyConstants.LOCALHOST;
private int port = HoverflyConstants.DEFAULT_ADMIN_PORT;
private String authToken = null;

Builder() {
}

public Builder scheme(String scheme) {
this.scheme = scheme;
return this;
}

public Builder host(String host) {
this.host = host;
return this;
}

public Builder port(int port) {
this.port = port;
return this;
}

/**
* Get token from environment variable "HOVERFLY_AUTH_TOKEN" to authenticate with admin API
* @return this Builder for further customizations
*/
public Builder withAuthToken() {
this.authToken = System.getenv(HoverflyConstants.HOVERFLY_AUTH_TOKEN);
return this;
}

public HoverflyClient build() {
return new OkHttpHoverflyClient(scheme, host, port, authToken);
}
}

}
168 changes: 94 additions & 74 deletions src/main/java/io/specto/hoverfly/junit/core/Hoverfly.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import java.io.File;
import java.io.IOException;
import java.io.OutputStream;
import java.net.ServerSocket;
import java.nio.file.Files;
import java.nio.file.Path;
import java.time.Duration;
Expand All @@ -37,6 +38,8 @@
import com.fasterxml.jackson.databind.ObjectWriter;
import io.specto.hoverfly.junit.api.HoverflyClient;
import io.specto.hoverfly.junit.api.HoverflyClientException;
import io.specto.hoverfly.junit.api.HoverflyClientFactory;
import io.specto.hoverfly.junit.api.HoverflyOkHttpClientFactory;
import io.specto.hoverfly.junit.api.model.ModeArguments;
import io.specto.hoverfly.junit.api.view.DiffView;
import io.specto.hoverfly.junit.api.view.HoverflyInfoView;
Expand Down Expand Up @@ -83,7 +86,8 @@ public class Hoverfly implements AutoCloseable {
private final HoverflyMode hoverflyMode;
private final ProxyConfigurer proxyConfigurer;
private final SslConfigurer sslConfigurer = new SslConfigurer();
private final HoverflyClient hoverflyClient;
private final HoverflyClientFactory hoverflyClientFactory;
private HoverflyClient hoverflyClient;

private final TempFileManager tempFileManager = new TempFileManager();
private StartedProcess startedProcess;
Expand All @@ -100,14 +104,8 @@ public class Hoverfly implements AutoCloseable {
public Hoverfly(HoverflyConfig hoverflyConfigBuilder, HoverflyMode hoverflyMode) {
hoverflyConfig = hoverflyConfigBuilder.build();
this.proxyConfigurer = new ProxyConfigurer(hoverflyConfig);
this.hoverflyClient = HoverflyClient.custom()
.scheme(hoverflyConfig.getScheme())
.host(hoverflyConfig.getHost())
.port(hoverflyConfig.getAdminPort())
.withAuthToken()
.build();
this.hoverflyMode = hoverflyMode;

this.hoverflyClientFactory = new HoverflyOkHttpClientFactory();
}

/**
Expand Down Expand Up @@ -139,7 +137,10 @@ public void start() {

if (!hoverflyConfig.isRemoteInstance()) {
startHoverflyProcess();
} else {
}
this.hoverflyClient = hoverflyClientFactory.createHoverflyClient(hoverflyConfig);

if (hoverflyConfig.isRemoteInstance()) {
resetJournal();
}

Expand All @@ -152,7 +153,7 @@ public void start() {
}

if (hoverflyConfig.getProxyCaCertificate().isPresent()) {
sslConfigurer.setDefaultSslContext(hoverflyConfig.getProxyCaCertificate().get());
sslConfigurer.setDefaultSslContext(hoverflyConfig.getProxyCaCertificate().get());
} else if (StringUtils.isNotBlank(hoverflyConfig.getSslCertificatePath())) {
sslConfigurer.setDefaultSslContext(hoverflyConfig.getSslCertificatePath());
} else {
Expand All @@ -163,81 +164,90 @@ public void start() {
}

private void startHoverflyProcess() {
checkPortInUse(hoverflyConfig.getProxyPort());
checkPortInUse(hoverflyConfig.getAdminPort());
synchronized (Hoverfly.class) {

final SystemConfig systemConfig = new SystemConfigFactory(hoverflyConfig).createSystemConfig();
if (hoverflyConfig.getProxyPort() == 0) {
hoverflyConfig.setProxyPort(findUnusedPort());
}
if (hoverflyConfig.getAdminPort() == 0) {
hoverflyConfig.setAdminPort(findUnusedPort());
}
checkPortInUse(hoverflyConfig.getProxyPort());
checkPortInUse(hoverflyConfig.getAdminPort());
Comment on lines +169 to +176
Copy link
Member

Choose a reason for hiding this comment

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

We should move any port checking and setting right before the start of hoverfly, to avoid synchronizing the whole method, only synchronizing the following is what you wanted (in pseudocode):

allOtherSetup()

synchronized() {
    findAndSetPort()
    checkPortInUse()
    setPortToCommandArgs()
    startHoverfly()
}


if (hoverflyConfig.getBinaryLocation() != null) {
tempFileManager.setBinaryLocation(hoverflyConfig.getBinaryLocation());
}
Path binaryPath = tempFileManager.copyHoverflyBinary(systemConfig);
final SystemConfig systemConfig = new SystemConfigFactory(hoverflyConfig).createSystemConfig();

LOGGER.info("Executing binary at {}", binaryPath);
final List<String> commands = new ArrayList<>();
commands.add(binaryPath.toString());
if (hoverflyConfig.getBinaryLocation() != null) {
tempFileManager.setBinaryLocation(hoverflyConfig.getBinaryLocation());
}
Path binaryPath = tempFileManager.copyHoverflyBinary(systemConfig);

if (!hoverflyConfig.getCommands().isEmpty()) {
commands.addAll(hoverflyConfig.getCommands());
}
commands.add("-pp");
commands.add(String.valueOf(hoverflyConfig.getProxyPort()));
commands.add("-ap");
commands.add(String.valueOf(hoverflyConfig.getAdminPort()));

if (StringUtils.isNotBlank(hoverflyConfig.getSslCertificatePath())) {
tempFileManager.copyClassPathResource(hoverflyConfig.getSslCertificatePath(), "ca.crt");
commands.add("-cert");
commands.add("ca.crt");
}
if (StringUtils.isNotBlank(hoverflyConfig.getSslKeyPath())) {
tempFileManager.copyClassPathResource(hoverflyConfig.getSslKeyPath(), "ca.key");
commands.add("-key");
commands.add("ca.key");
}
if (hoverflyConfig.isPlainHttpTunneling()) {
commands.add("-plain-http-tunneling");
}
LOGGER.info("Executing binary at {}", binaryPath);
final List<String> commands = new ArrayList<>();
commands.add(binaryPath.toString());

if (hoverflyConfig.isWebServer()) {
commands.add("-webserver");
}
if (!hoverflyConfig.getCommands().isEmpty()) {
commands.addAll(hoverflyConfig.getCommands());
}
commands.add("-pp");
commands.add(String.valueOf(hoverflyConfig.getProxyPort()));
commands.add("-ap");
commands.add(String.valueOf(hoverflyConfig.getAdminPort()));

if (StringUtils.isNotBlank(hoverflyConfig.getSslCertificatePath())) {
tempFileManager.copyClassPathResource(hoverflyConfig.getSslCertificatePath(), "ca.crt");
commands.add("-cert");
commands.add("ca.crt");
}
if (StringUtils.isNotBlank(hoverflyConfig.getSslKeyPath())) {
tempFileManager.copyClassPathResource(hoverflyConfig.getSslKeyPath(), "ca.key");
commands.add("-key");
commands.add("ca.key");
}
if (hoverflyConfig.isPlainHttpTunneling()) {
commands.add("-plain-http-tunneling");
}

if (hoverflyConfig.isTlsVerificationDisabled()) {
commands.add("-tls-verification=false");
}
if (hoverflyConfig.isWebServer()) {
commands.add("-webserver");
}

if (hoverflyConfig.getHoverflyLogger().isPresent()) {
commands.add("-logs");
commands.add("json");
}
if (hoverflyConfig.isTlsVerificationDisabled()) {
commands.add("-tls-verification=false");
}

if (hoverflyConfig.getLogLevel().isPresent()) {
commands.add("-log-level");
commands.add(hoverflyConfig.getLogLevel().get().name().toLowerCase());
}
if (hoverflyConfig.getHoverflyLogger().isPresent()) {
commands.add("-logs");
commands.add("json");
}

if (hoverflyConfig.isMiddlewareEnabled()) {
final String path = hoverflyConfig.getLocalMiddleware().getPath();
final String scriptName = path.contains(File.separator) ? path.substring(path.lastIndexOf(File.separator) + 1) : path;
tempFileManager.copyClassPathResource(path, scriptName);
commands.add("-middleware");
commands.add(hoverflyConfig.getLocalMiddleware().getBinary() + " " + scriptName);
}
if (hoverflyConfig.getLogLevel().isPresent()) {
commands.add("-log-level");
commands.add(hoverflyConfig.getLogLevel().get().name().toLowerCase());
}

if (StringUtils.isNotBlank(hoverflyConfig.getUpstreamProxy())) {
commands.add("-upstream-proxy");
commands.add(hoverflyConfig.getUpstreamProxy());
}
if (hoverflyConfig.isMiddlewareEnabled()) {
final String path = hoverflyConfig.getLocalMiddleware().getPath();
final String scriptName = path.contains(File.separator) ? path.substring(path.lastIndexOf(File.separator) + 1) : path;
tempFileManager.copyClassPathResource(path, scriptName);
commands.add("-middleware");
commands.add(hoverflyConfig.getLocalMiddleware().getBinary() + " " + scriptName);
}

try {
startedProcess = new ProcessExecutor()
.command(commands)
.redirectOutput(hoverflyConfig.getHoverflyLogger().<OutputStream>map(LoggingOutputStream::new).orElse(System.out))
.directory(tempFileManager.getTempDirectory().toFile())
.start();
} catch (IOException e) {
throw new IllegalStateException("Could not start Hoverfly process", e);
if (StringUtils.isNotBlank(hoverflyConfig.getUpstreamProxy())) {
commands.add("-upstream-proxy");
commands.add(hoverflyConfig.getUpstreamProxy());
}

try {
startedProcess = new ProcessExecutor()
.command(commands)
.redirectOutput(hoverflyConfig.getHoverflyLogger().<OutputStream>map(LoggingOutputStream::new).orElse(System.out))
.directory(tempFileManager.getTempDirectory().toFile())
.start();
} catch (IOException e) {
throw new IllegalStateException("Could not start Hoverfly process", e);
}
}
}

Expand Down Expand Up @@ -505,6 +515,16 @@ private void persistSimulation(Path path, Simulation simulation) throws IOExcept
JSON_PRETTY_PRINTER.writeValue(path.toFile(), simulation);
}

/**
* Looks for an unused port on the current machine
*/
private int findUnusedPort() {
try (final ServerSocket serverSocket = new ServerSocket(0)) {
return serverSocket.getLocalPort();
} catch (IOException e) {
throw new RuntimeException("Cannot find available port", e);
}
}

/**
* Blocks until the Hoverfly process becomes healthy, otherwise time out
Expand Down
Loading