Cache session details to avoid making unnecessary queries

[mykola-mokhnach]

Change list

Addresses #764

Types of changes

• No changes in production code.
• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

[SrinivasanTarget]

👍

[vrunoa]

Works great! reduced a test from 2hs to 25minutes.

[saikrishna321]

Wow, that's a great time difference .....

[saikrishna321]

@vrunoa was this tested on parallel sessions ? if not I can take it ...

[vrunoa]

@saikrishna321 no, just a test on a cloud service. I havent test parallel sessions, if you cant take it it would be great.

[saikrishna321]

done I will do it today

[TikhomirovSergey]

I am against this change. Session details data is not immutable. It may change when driver is switched to another context (NATIVE APP or WEB VIEW).

I think the right way is to find places where it is invoked frequently.

[fhoeben]

If full copy is used, shouldn't there at least be an 'expiry time' or a method to force the details to be refreshed?

When I ran into this issue I decided to use an expiry time, based on the time it took to retrieve the information, in my (not so nice) workaround in https://github.com/fhoeben/hsac-fitnesse-appium.
The approach I currently use there is to have some cache at the element level, and using my own 'json to element' converter.
My custom elements require less retrieval of session details (for me the 'foundBy' information does not require session details, which is currently required for the driver's toString(), and they 'cache' information retrieved from the driver about an element for a short while (so my test code does not need to worry too much about for instance checking isDisplayed() twice)).

[TikhomirovSergey]

@vrunoa There are some question to make it clear:

• do you use get session details frequently at the client side?
• do you use page objects?

[vrunoa]

@TikhomirovSergey the test I'm running is not calling to getSessionDetails.
If the session will change at some point, should be getSessionDetails only at that point and not every time ?

Not sure about page object, can you help me figure out if it is used.

[saikrishna321]

@vrunoa as my tests are running locally, i don't see much time difference its 2mins difference in total

[vrunoa]

@saikrishna321 yeah, the big difference it's when running on a cloud service since those getSessionDetails calls take like 5 seconds or more between commands but locally you just lose a few milliseconds.
@TikhomirovSergey do we know where are session details changed ?

[TikhomirovSergey]

@vrunoa I suppose we don't all the actions which can change session parameters.
I know one driver.context(someContext)

If the session will change at some point, should be getSessionDetails only at that point and not every time ?

We know only one place in java_client where getSessionDetails is asked very often. And I am going to optimize it. I have asked the question before. Does your code build and use page objects using @AndroidFindBy /@iOSFindBy etc and AppiumFieldDecorator? Otherwise could you show the sample of your code which has the performance issue?

[vrunoa]

@TikhomirovSergey yes, it does use those decorators. Sorry but I cannot share the source code of this particular test.

[TikhomirovSergey]

@vrunoa ok. So it seems I was right. Working on improvements.

[vrunoa]

thanks @TikhomirovSergey. Let me know if there's something I can help with.

[vrunoa]

@TikhomirovSergey Sorry to bother, but do you have any updates on this fix?

[TikhomirovSergey]

@vrunoa Yep. Please look at the related issue #764. There are 3 commits. Tomorrow I am going to propose the new PR.

[mykola-mokhnach]

Closed in favour of #769