#764 alternative fix#769
#764 alternative fix#769TikhomirovSergey merged 6 commits intoappium:masterappium/java-client:masterfrom TikhomirovSergey:#764_FIXCopy head branch name to clipboard
Conversation
Also: - WidgetListInterceptor was optimized - some code style improvements
|
@vrunoa You can review it too. It would be cool if you could try to test this change on your environment. |
| ElementMap element = Optional.ofNullable(mobileElementMap.get(String | ||
| .valueOf(hasSessionDetails.getAutomationName()).toLowerCase().trim())) | ||
| ElementMap element = Optional.ofNullable(mobileElementMap.get( | ||
| String.valueOf(platform).toLowerCase().trim())) |
There was a problem hiding this comment.
does it make any sense to use String.valueOf for values of type String?
There was a problem hiding this comment.
@mykola-mokhnach It may return null value
|
|
||
| private static By getBy(By currentBy, SearchContext currentContent) { | ||
| if (!ContentMappedBy.class.isAssignableFrom(currentBy.getClass())) { | ||
| return currentBy; |
There was a problem hiding this comment.
it might be easier to understand the code if this condition had an explanation comment
| List<WebElement> list = searchContext.findElements(by); | ||
| List<WebElement> list = searchContext | ||
| .findElements(getBy(by, searchContext)); | ||
| if (list.size() > 0) { |
There was a problem hiding this comment.
return list.size() > 0 ? list : null
| || !HasSessionDetails.class.isAssignableFrom(originalDriver.getClass())) { | ||
| hasSessionDetails = null; | ||
| HasSessionDetails hasSessionDetails = ofNullable(this.originalDriver).map(webDriver -> { | ||
| if (!HasSessionDetails.class.isAssignableFrom(originalDriver.getClass())) { |
There was a problem hiding this comment.
originalDriver -> webDriver
|
|
||
| @Override public WebElement findElement(SearchContext context) { | ||
| return context.findElement(map.get(getCurrentContentType(context))); | ||
| return context.findElement(map.get(currentContent)); |
There was a problem hiding this comment.
is it ok if map.get returns null in any of these methods?
There was a problem hiding this comment.
It is not possible. It will use NATIVE_MOBILE_SPECIFIC as it is defined by default or it will use HTML_OR_DEFAULT when there is no SearchContext which also implements ContextArare or HasSessionDetails. Also it will use HTML_OR_DEFAULT when HasSessionDetails.isBrowser returns true or current context != NATIVE_CONTEXT.
Also there is the method
/**
* This method sets required content type for the further searching.
* @param type required content type {@link ContentType}
* @return self-reference.
*/
public By useContent(@Nonnull ContentType type) {
checkNotNull(type);
currentContent = type;
return this;
}
mykola-mokhnach
left a comment
There was a problem hiding this comment.
LGTM, just minor comments
|
@mykola-mokhnach I have improved some things that you pointed. |
|
LGTM |
| HasSessionDetails hasSessionDetails = ofNullable(this.originalDriver).map(webDriver -> { | ||
| if (!HasSessionDetails.class.isAssignableFrom(originalDriver.getClass())) { | ||
| this.webDriver = unpackWebDriverFromSearchContext(context); | ||
| HasSessionDetails hasSessionDetails = ofNullable(this.webDriver).map(webDriver -> { |
There was a problem hiding this comment.
I meant it's better to use the mapped local variable webDriver -> inside the lambda handler rather than class-level this.originalDriverone.
|
@vrunoa |
|
@TikhomirovSergey i'm running tests, i'll get back to you as soon as I have some results. |
|
@vrunoa and what the result? |
|
ping @vrunoa |
|
@TikhomirovSergey sorry for the delay. I'm session a lot less getSession calls and test times got reduced. Lets merge it. |
|
@vrunoa How much time was reduced? I will merge it later today |
|
@TikhomirovSergey from 2hs to 50minutes approximately. |
Awesome |
Change list
#764 FIX. This change was made according to the confersation #765
Types of changes