#655 FIX #682
#655 FIX #682
Conversation
| public AppiumCommandExecutor(Map<String, CommandInfo> additionalCommands, | ||
| URL addressOfRemoteServer, HttpClient.Factory httpClientFactory) { | ||
| super(additionalCommands, addressOfRemoteServer, httpClientFactory); | ||
| service = null; |
There was a problem hiding this comment.
can this be a reason of unexpected NPE?
There was a problem hiding this comment.
There was a problem hiding this comment.
then it might be useful to mark the property as Nullable
There was a problem hiding this comment.
or make it optional (I'd prefer this option)
There was a problem hiding this comment.
@mykola-mokhnach ok. I will improve it soon.
|
How about having some unit test to verify resulting capabilities structure? |
I think we don't need for these tests:
@Override public Response execute(Command command) throws IOException, WebDriverException {
...
return super.execute(command);
...
}
Our tests which run some AppiumDriver are successful. |
| response.setSessionId(command.getSessionId().toString()); | ||
| @Override public Response execute(Command command) throws WebDriverException { | ||
| if (DriverCommand.NEW_SESSION.equals(command.getName())) { | ||
| ofNullable(service).ifPresent(driverService -> { |
There was a problem hiding this comment.
I assume the code will be simpler if we make the service variable itself of type Optional
| } | ||
|
|
||
| return new WebDriverException("The appium server has accidentally died!", rootCause); | ||
| }).orElse(new WebDriverException(rootCause.getMessage(), rootCause)); |
There was a problem hiding this comment.
probably, orElseGet ?
|
@mykola-mokhnach There is another one improvement. |
Change list
#655 FIX
Types of changes
Details
io.appium.java_client.remote.AppiumCommandExecutorwas rolled back.It is breaking change because it is compatible with appium server > v1.6.5