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

Improve the error message on session creation #994

Merged
merged 4 commits into from
Aug 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 11 additions & 5 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ apply plugin: 'checkstyle'
apply plugin: 'signing'
apply plugin: 'maven-publish'

import org.apache.tools.ant.filters.*

group 'io.appium'
version '6.1.0'

Expand Down Expand Up @@ -50,21 +52,19 @@ compileJava {
]
}

ext.seleniumVersion = '3.14.0'

dependencies {
compile ("org.seleniumhq.selenium:selenium-java:${seleniumVersion}") {
compile ("org.seleniumhq.selenium:selenium-java:${project.property('selenium.version')}") {
force = true

exclude group: 'com.google.code.gson'
exclude module: 'htmlunit-driver'
exclude group: 'net.sourceforge.htmlunit'

}
compile ("org.seleniumhq.selenium:selenium-support:${seleniumVersion}") {
compile ("org.seleniumhq.selenium:selenium-support:${project.property('selenium.version')}") {
force = true
}
compile ("org.seleniumhq.selenium:selenium-api:${seleniumVersion}") {
compile ("org.seleniumhq.selenium:selenium-api:${project.property('selenium.version')}") {
force = true
}
compile 'com.google.code.gson:gson:2.8.4'
Expand Down Expand Up @@ -199,6 +199,12 @@ wrapper {
distributionType = Wrapper.DistributionType.ALL
}

processResources {
filter ReplaceTokens, tokens: [
'selenium.version': project.property('selenium.version')
]
}

task xcuiTest( type: Test ) {
useJUnit()
testLogging.showStandardStreams = true
Expand Down
2 changes: 2 additions & 0 deletions gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,5 @@ signing.secretKeyRingFile=PathToYourKeyRingFile

ossrhUsername=your-jira-id
ossrhPassword=your-jira-password

selenium.version=3.14.0
71 changes: 71 additions & 0 deletions src/main/java/io/appium/java_client/internal/Config.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package io.appium.java_client.internal;

import java.io.IOException;
import java.io.InputStream;
import java.io.UncheckedIOException;
import java.util.Map;
import java.util.Optional;
import java.util.Properties;
import java.util.concurrent.ConcurrentHashMap;

public class Config {
private static Config mainInstance = null;
private static final String MAIN_CONFIG = "main.properties";
private static final Map<String, Properties> cache = new ConcurrentHashMap<>();
private final String configName;

/**
* Retrieve a facade for the main config.
*
* @return interaction helper for 'main.properties' config
*/
public static synchronized Config main() {
if (mainInstance == null) {
mainInstance = new Config(MAIN_CONFIG);
}
return mainInstance;
}

private Config(String configName) {
this.configName = configName;
}

/**
* Retrieve a value from properties file.
*
* @param key the name of the corresponding key which value to retrieve
* @param valueType the expected type of the value to be retrieved
* @return the actual value
* @throws IllegalArgumentException if the given key does not exist
* @throws ClassCastException if the retrieved value cannot be cast to `valueType` type
*/
public <T> T getValue(String key, Class<T> valueType) {
return getOptionalValue(key, valueType)
.orElseThrow(() -> new IllegalArgumentException(
String.format("There is no '%s' key in '%s' config", key, configName)
));
}

/**
* Retrieve a value from properties file.
*
* @param key the name of the corresponding key which value to retrieve
* @param valueType the expected type of the value to be retrieved
* @return the actual value or {@link Optional#empty()} if the key is not present
* @throws UncheckedIOException if the given properties file does not exist/not accessible
* @throws ClassCastException if the retrieved value cannot be cast to `valueType` type
*/
public <T> Optional<T> getOptionalValue(String key, Class<T> valueType) {
final Properties cachedProps = cache.computeIfAbsent(configName, (k) -> {
try (InputStream configFileStream = getClass().getClassLoader().getResourceAsStream(configName)) {
final Properties p = new Properties();
p.load(configFileStream);
return p;
} catch (IOException e) {
throw new UncheckedIOException(String.format("Configuration file '%s' cannot be loaded",
configName), e);
}
});
return cachedProps.containsKey(key) ? Optional.of(valueType.cast(cachedProps.get(key))) : Optional.empty();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.google.common.io.CountingOutputStream;
import com.google.common.io.FileBackedOutputStream;

import io.appium.java_client.internal.Config;
import org.openqa.selenium.Capabilities;
import org.openqa.selenium.ImmutableCapabilities;
import org.openqa.selenium.SessionNotCreatedException;
Expand Down Expand Up @@ -190,17 +191,21 @@ public Result createSession(HttpClient client, Command command)
.info(format("Detected dialect: %s", toReturn.getDialect()));
return toReturn;
}).orElseThrow(() -> new SessionNotCreatedException(
format("Unable to create new remote session. desired capabilities = %s", desired)));
format("Unable to create a new remote session. Desired capabilities = %s", desired)));
} catch (NoSuchMethodException | IllegalAccessException e) {
throw new WebDriverException(format("It is impossible to create a new session "
+ "because 'createSession' which takes %s, %s and %s was not found "
+ "or it is not accessible",
HttpClient.class.getSimpleName(),
InputStream.class.getSimpleName(),
long.class.getSimpleName()), e);
throw new SessionNotCreatedException(format("Unable to create a new remote session. "
+ "Make sure your project dependencies config does not override "
+ "Selenium API version %s used by java-client library.",
Config.main().getValue("selenium.version", String.class)), e);
} catch (InvocationTargetException e) {
throw new SessionNotCreatedException(
format("Unable to create new remote session. Desired capabilities: %s", desired), e);
String message = "Unable to create a new remote session.";
if (e.getCause() != null) {
if (e.getCause() instanceof WebDriverException) {
message += " Please check the server log for more details.";
}
message += format(" Original error: %s", e.getCause().getMessage());
Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Contributor Author

@mykola-mokhnach mykola-mokhnach Aug 8, 2018

Choose a reason for hiding this comment

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

fixed checkstyle

}
throw new SessionNotCreatedException(message, e);
}
} finally {
os.reset();
Expand Down
1 change: 1 addition & 0 deletions src/main/resources/main.properties
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
selenium.version=@selenium.version@
34 changes: 34 additions & 0 deletions src/test/java/io/appium/java_client/internal/ConfigTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package io.appium.java_client.internal;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.greaterThan;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

import org.junit.Test;

public class ConfigTest {
private static final String EXISTING_KEY = "selenium.version";
private static final String MISSING_KEY = "bla";

@Test
public void verifyGettingExistingValue() {
assertThat(Config.main().getValue(EXISTING_KEY, String.class).length(), greaterThan(0));
assertTrue(Config.main().getOptionalValue(EXISTING_KEY, String.class).isPresent());
}

@Test(expected = IllegalArgumentException.class)
public void verifyGettingNonExistingValue() {
assertThat(Config.main().getValue(MISSING_KEY, String.class).length(), greaterThan(0));
}

@Test(expected = ClassCastException.class)
public void verifyGettingExistingValueWithWrongClass() {
assertThat(Config.main().getValue(EXISTING_KEY, Integer.class), greaterThan(0));
}

@Test
public void verifyGettingNonExistingOptionalValue() {
assertFalse(Config.main().getOptionalValue(MISSING_KEY, String.class).isPresent());
}
}