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

Use set admin port for compatibility to arquillian cube. #34

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Use set admin port for compatibility to arquillian cube. #34

wants to merge 2 commits into from

Conversation

attrobit
Copy link

Short description of what this resolves:

Now it is possible to use this container adapter together with arquillian-cube-openshift.

Because arquillian-cube-openshift uses openshift port forwarding. But the local port is created randomly. With this change the randomly generated local port is used.

@attrobit
Copy link
Author

attrobit commented Sep 3, 2018

@aslakknutsen @bartoszmajsak No chance to merge this PR?

Copy link
Member

@bartoszmajsak bartoszmajsak left a comment

Choose a reason for hiding this comment

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

Thank you very much for this fix. I shared two minor things for your consideration.

Is there any way to enhance test suite in this adapter so that your changes can be covered?

@@ -241,6 +244,7 @@ public int getAdminListenPort() {
* The port of the admin server. This is optional. It can be derived from the adminUrl.
*/
public void setAdminListenPort(int adminListenPort) {
this.adminListenPortSet = true;
this.adminListenPort = adminListenPort;
Copy link
Member

Choose a reason for hiding this comment

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

Alternatively we could use Integer and rely on it being null instead. WDYT?

@@ -114,7 +115,9 @@ public void validate() throws ConfigurationException {
URI adminURI = new URI(adminUrl);
adminProtocol = adminURI.getScheme();
adminListenAddress = adminURI.getHost();
adminListenPort = adminURI.getPort();
if(!adminListenPortSet) {
Copy link
Member

Choose a reason for hiding this comment

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

Please fix formatting - if (...).

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