appium / appium-espresso-driver

Espresso integration for Appium
Apache License 2.0
191 stars 75 forks source link

fix: Cache children in serializeComposeNode #1027

Closed idyatlov closed 2 months ago

idyatlov commented 2 months ago

We can have modifications in the nodes when traversing through them in serializeComposeNode, so it's important to cache children before printing them. Similar approach is used in compose-ui-test when printing compose nodes.

This is to prevent stack traces like that one ```Responding to server with error: {error=unknown error, message=java.lang.IndexOutOfBoundsException: Index 5 out of bounds for length 5, stacktrace=java.lang.IndexOutOfBoundsException: Index 5 out of bounds for length 5 at jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:64) at jdk.internal.util.Preconditions.outOfBoundsCheckIndex(Preconditions.java:70) at jdk.internal.util.Preconditions.checkIndex(Preconditions.java:266) at java.util.Objects.checkIndex(Objects.java:359) at java.util.ArrayList.get(ArrayList.java:434) at io.appium.espressoserver.lib.model.SourceDocument.serializeComposeNode(SourceDocument.java:244) at io.appium.espressoserver.lib.model.SourceDocument.toStream(SourceDocument.java:288) at io.appium.espressoserver.lib.model.SourceDocument.access$toStream(SourceDocument.java:87) at io.appium.espressoserver.lib.model.SourceDocument$findMatchingNodeIds$1.invoke(SourceDocument.java:376) at io.appium.espressoserver.lib.model.SourceDocument$findMatchingNodeIds$1.invoke(SourceDocument.java:375) at io.appium.espressoserver.lib.helpers.extensions.SemaphoreKt.withPermit(SemaphoreKt.java:17) at io.appium.espressoserver.lib.model.SourceDocument.findMatchingNodeIds(SourceDocument.java:375) at io.appium.espressoserver.lib.helpers.ComposeNodeFinderKt.hasXpath(ComposeNodeFinderKt.java:72) at io.appium.espressoserver.lib.helpers.ComposeNodeFinderKt.semanticsMatcherForLocator(ComposeNodeFinderKt.java:58) at io.appium.espressoserver.lib.helpers.ComposeNodeFinderKt.toNodeInteractionsCollection(ComposeNodeFinderKt.java:45) at io.appium.espressoserver.lib.handlers.FindElements.handleCompose(FindElements.java:44) at io.appium.espressoserver.lib.handlers.FindElements.handleCompose(FindElements.java:28) at io.appium.espressoserver.lib.handlers.RequestHandler$DefaultImpls.invokeStrategy(RequestHandler.java:34) at io.appium.espressoserver.lib.handlers.FindElements.invokeStrategy(FindElements.java:28) at io.appium.espressoserver.lib.handlers.FindElements.invokeStrategy(FindElements.java:28) at io.appium.espressoserver.lib.handlers.RequestHandler$DefaultImpls.handleInternal(RequestHandler.java:29) at io.appium.espressoserver.lib.handlers.FindElements.handleInternal(FindElements.java:28) at io.appium.espressoserver.lib.handlers.FindElements.handleInternal(FindElements.java:28) at io.appium.espressoserver.lib.handlers.RequestHandler$DefaultImpls.handle(RequestHandler.java:57) at io.appium.espressoserver.lib.handlers.FindElements.handle(FindElements.java:28) at io.appium.espressoserver.lib.handlers.FindElements.handle(FindElements.java:28) at io.appium.espressoserver.lib.http.Router.route(Router.java:232) at io.appium.espressoserver.lib.http.Server.serve(Server.java:78) ```
Some decompiled code to show the difference **Old code** Size is calculated once, but we calculate a new collection each time when getting a new child so if the collection has fewer elements there will be `IndexOutOfBoundsException` ``` for(int var15 = ((Collection)semanticsNode.getChildren()).size(); index < var15; ++index) { this.serializeComposeNode((SemanticsNode)semanticsNode.getChildren().get(index), depth + 1); } ``` **New code** we calculate children collection only once ``` List children = semanticsNode.getChildren(); int index = 0; for(int var17 = ((Collection)children).size(); index < var17; ++index) { this.serializeComposeNode((SemanticsNode)children.get(index), depth + 1); } ```
github-actions[bot] commented 2 months ago

:tada: This issue has been resolved in version 3.3.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

jlipps commented 3 weeks ago

Hi @idyatlov, congrats, the Appium project wants to compensate you for this contribution! Please reply to this comment mentioning me and sharing your OpenCollective account name, so that we can initiate payment! Or let me know if you decline to receive compensation via OpenCollective. Thank you!

idyatlov commented 3 weeks ago

Thanks @jlipps, my OpenCollective account name is ivan-dyatlov - and it will be my first compensation, I appreciate that!