-
Notifications
You must be signed in to change notification settings - Fork 60
Fix port conflict when running multiple unit tests in parallel #220
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
Open
cretzel
wants to merge
1
commit into
SpectoLabs:master
Choose a base branch
from
cretzel:fix_port_conflict
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
12 changes: 12 additions & 0 deletions
12
src/main/java/io/specto/hoverfly/junit/api/HoverflyClientFactory.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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
75
src/main/java/io/specto/hoverfly/junit/api/HoverflyOkHttpClientFactory.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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() | ||
| .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); | ||
| } | ||
| } | ||
|
|
||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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; | ||
|
|
@@ -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; | ||
|
|
@@ -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; | ||
|
|
@@ -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(); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -139,7 +137,10 @@ public void start() { | |
|
|
||
| if (!hoverflyConfig.isRemoteInstance()) { | ||
| startHoverflyProcess(); | ||
| } else { | ||
| } | ||
| this.hoverflyClient = hoverflyClientFactory.createHoverflyClient(hoverflyConfig); | ||
|
|
||
| if (hoverflyConfig.isRemoteInstance()) { | ||
| resetJournal(); | ||
| } | ||
|
|
||
|
|
@@ -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 { | ||
|
|
@@ -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
Member
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. 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): |
||
|
|
||
| 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); | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -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 | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
withAuthToken()is optional. It's better to expose the builder for the user to customize the client: