Skip to content

fix: #1660 - correction for ping method to get proper status URL in case of a basepath provided #1661

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

Merged
merged 8 commits into from
Mar 19, 2022
Merged

fix: #1660 - correction for ping method to get proper status URL in case of a basepath provided #1661

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e8da284
Fix for basePath appending logic to fix Issue#1660 AppiumDriverLocalS…
Mar 17, 2022
72cf29f
Added test to check the service instantiation with basepath
Mar 17, 2022
c19a926
Merge pull request #1 from robinGupta11392/1660-fix-for-checkStatus-url
robinGupta11392 Mar 17, 2022
c553b91
Update src/main/java/io/appium/java_client/service/local/AppiumDriver…
robinGupta11392 Mar 18, 2022
69d2a41
fix: Modified the logic to validate basePath as per the pull request …
robinGupta11392 Mar 18, 2022
e013f70
Formatted code to fix the spacing issue
robinGupta11392 Mar 18, 2022
3061c99
Update src/main/java/io/appium/java_client/service/local/AppiumServic…
robinGupta11392 Mar 19, 2022
5b76ab9
Import order correction
robinGupta11392 Mar 19, 2022
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
12 changes: 12 additions & 0 deletions src/main/java/io/appium/java_client/service/local/AppiumServiceBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package io.appium.java_client.service.local;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static org.openqa.selenium.remote.CapabilityType.PLATFORM_NAME;

import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -217,13 +219,23 @@ public AppiumServiceBuilder withArgument(ServerArgument argument, String value)
case "-g":
withLogFile(new File(value));
break;
case "--base-path":
serverArguments.put(argName, sanitizeBasePath(value));
break;
default:
serverArguments.put(argName, value);
break;
}
return this;
}

private static String sanitizeBasePath(String basePath) {
basePath = checkNotNull(basePath).trim();
checkArgument(!basePath.isEmpty(),
"Given base path is not valid - blank or empty values are not allowed for base path");
return basePath.endsWith("/") ? basePath : basePath + "/";
}

/**
* Adds capabilities.
*
Expand Down
45 changes: 45 additions & 0 deletions src/test/java/io/appium/java_client/service/local/ServerBuilderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
import static io.appium.java_client.TestUtils.getLocalIp4Address;
import static io.appium.java_client.service.local.AppiumDriverLocalService.buildDefaultService;
import static io.appium.java_client.service.local.AppiumServiceBuilder.APPIUM_PATH;
import static io.appium.java_client.service.local.AppiumServiceBuilder.BROADCAST_IP_ADDRESS;
import static io.appium.java_client.service.local.AppiumServiceBuilder.DEFAULT_APPIUM_PORT;
import static io.appium.java_client.service.local.flags.GeneralServerFlag.BASEPATH;
import static io.appium.java_client.service.local.flags.GeneralServerFlag.CALLBACK_ADDRESS;
import static io.appium.java_client.service.local.flags.GeneralServerFlag.SESSION_OVERRIDE;
import static io.github.bonigarcia.wdm.WebDriverManager.chromedriver;
Expand All @@ -18,6 +21,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;

import com.google.common.collect.ImmutableMap;
Expand Down Expand Up @@ -312,4 +316,45 @@ public void checkAbilityToStartServiceWithLogFileUsingShortFlag() {
service.start();
assertTrue(testLogFile.exists());
}

@Test
public void checkAbilityToStartServiceUsingValidBasePathWithMultiplePathParams() {
String baseUrl = String.format("http://%s:%d/", BROADCAST_IP_ADDRESS, DEFAULT_APPIUM_PORT);
String basePath = "wd/hub";
service = new AppiumServiceBuilder().withArgument(BASEPATH, basePath).build();
service.start();
assertTrue(service.isRunning());
assertEquals(baseUrl + basePath + "/", service.getUrl().toString());
}

@Test
public void checkAbilityToStartServiceUsingValidBasePathWithSinglePathParams() {
String baseUrl = String.format("http://%s:%d/", BROADCAST_IP_ADDRESS, DEFAULT_APPIUM_PORT);
String basePath = "/wd/";
service = new AppiumServiceBuilder().withArgument(BASEPATH, basePath).build();
service.start();
assertTrue(service.isRunning());
assertEquals(baseUrl + basePath.substring(1), service.getUrl().toString());
}

@Test
public void checkAbilityToValidateBasePathForEmptyBasePath() {
assertThrows(IllegalArgumentException.class, () -> {
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI: you could add expected exception signature to Test annotations: https://www.guru99.com/junit-exception-test.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Noted with thanks.

new AppiumServiceBuilder().withArgument(BASEPATH, "").build();
});
}

@Test
public void checkAbilityToValidateBasePathForBlankBasePath() {
assertThrows(IllegalArgumentException.class, () -> {
new AppiumServiceBuilder().withArgument(BASEPATH, " ").build();
});
}

@Test
public void checkAbilityToValidateBasePathForNullBasePath() {
assertThrows(NullPointerException.class, () -> {
new AppiumServiceBuilder().withArgument(BASEPATH, null).build();
});
}
}