Skip to content

Fixed dynamically created link for selenium#47

Open
KarishoK27 wants to merge 3 commits intoAccenture:masterfrom
KarishoK27:master
Open

Fixed dynamically created link for selenium#47
KarishoK27 wants to merge 3 commits intoAccenture:masterfrom
KarishoK27:master

Conversation

@KarishoK27
Copy link

If ADOP is running at URL that contains domain, function createURL was creating URL for selenium that looks like this http://selenium.54.237.225.72.nip.io.xip.io///grid/console Such link are not working, so you need manually remove .xip.io.
Added to function block that checks hostname. If ADOP hostname is IP address, function is adding .xip.io to the end of link. If hostname contains domain, function will not add .xip.io.
Also removed extra '/' from URL ending part for selenium.

Copy link

@luismsousa luismsousa left a comment

Choose a reason for hiding this comment

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

@KarishoK27 can you update the string to remove the hardcoded HTTP?

return "http://"+ host + "." + window.location.host+ (window.location.host.indexOf(".xip.io") > -1 ? "" : ".xip.io/");
var ipformat = /^(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$/;
if (window.location.hostname.match(ipformat)) {
return "http://" + host + "." + window.location.host + ".xip.io";

Choose a reason for hiding this comment

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

Hi @KarishoK27 can you replace this to add the protocol dynamically by using the window.location.protocol method?

it should look something like this:
return window.location.protocol + "//" + host + "." + window.location.host + ".xip.io";

if (window.location.hostname.match(ipformat)) {
return "http://" + host + "." + window.location.host + ".xip.io";
} else {
return "http://" + host + "." + window.location.host;

Choose a reason for hiding this comment

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

same on this line:

by using the window.location.protocol method?

it should look something like this:
return window.location.protocol + "//" + host + "." + window.location.host;

Copy link

@luismsousa luismsousa left a comment

Choose a reason for hiding this comment

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

LGTM @RobertNorthard @dsingh07 can you please test?

cc: @jeremych1000 @nickdgriffin

@nickdgriffin
Copy link
Contributor

Could it be switched to "nip.io" instead please?

Reason being is that nip doesn't have the "resolving text to IPs" problem

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.

3 participants