niloc132 / gxt-driver

Other
5 stars 7 forks source link

Stale elements break exported methods, all parameters must be checked before they are dispatched to the browser #13

Open PeterWippermann opened 8 years ago

PeterWippermann commented 8 years ago

From time to time I get the following exception, when I'm trying to find InfoPopups. It's not always reproducible, but happens at the same place in the code. Maybe a race condition?

Well, I don't really understand it - how can the method be missing?

java.lang.RuntimeException: error: Error: could not find method 'com.sencha.gxt.widget.core.client.info.Info'
    at org.senchalabs.gwt.gwtdriver.ModuleUtilities.executeExportedFunction(ModuleUtilities.java:99)
    at org.senchalabs.gwt.gwtdriver.invoke.ClientMethodsFactory$InvocationHandlerImplementation.invoke(ClientMethodsFactory.java:61)
    at com.sun.proxy.$Proxy14.instanceofwidget(Unknown Source)
    at org.senchalabs.gwt.gwtdriver.by.ByWidget.findElements(ByWidget.java:62)
    at org.openqa.selenium.remote.RemoteWebElement.findElements(RemoteWebElement.java:189)
    at org.openqa.selenium.support.pagefactory.ByChained.findElements(ByChained.java:72)
    at org.openqa.selenium.remote.RemoteWebDriver.findElements(RemoteWebDriver.java:341)
    at org.senchalabs.gwt.gwtdriver.gxt.models.InfoPopup$InfoPopupFinder.done(InfoPopup.java:72)
niloc132 commented 8 years ago

Without steps to reproduce (runnable gxt app that can sometimes get this, the test code that can intermittently fail, browser/OS combo, etc..), my best guess is that Info popups are transient beasts, and unless you have them set to just stay visible until the user clicks them, you might have a timing issue where the browser has already hidden it (or it isn't yet visible) when the done() call happens.

PeterWippermann commented 8 years ago

Thanks for your input :-) I think the visibility of the InfoPopups should only be important during the execution of the finder's done() method. Not during the call of atTop() or withText(). Right?

So do you mean that the error is caused if I'm searching for InfoPopups and there are no ones visible at that moment? I'm just confused by the explicit error message, that a method cannot be found. I thought the exported methods should be available all the time?!

niloc132 commented 8 years ago

Yeah, unsure about the error message myself, and not a great day for me to dig in and try to repro it. Betting that it is just a useless/incorrect string, but I haven't seen the issue before.

In a case like this where it will take time for it to show up, your test can be written with a delay of some kind, like the FluentWait that comes with selenium, and attempting to find until it succeeds (or some timeout occurs).


Pre-publish edit: Found the log line that you are hitting, https://github.com/niloc132/gwt-driver/blob/master/src/main/java/org/senchalabs/gwt/gwtdriver/client/SeleniumExporter.java#L154. This is in client code, and the method there in your stack trace is "com.sencha.gxt.widget.core.client.info.Info" which makes zero sense, since it is being called by instanceofwidget - either your generated JS code is totally mangled, your browser is completely bonkers, or the reflection code has gone insane. The key from https://github.com/niloc132/gwt-driver/blob/master/src/main/java/org/senchalabs/gwt/gwtdriver/invoke/ClientMethodsFactory.java#L57 is getting swapped with the actual parameter (of the expected type for ByWidget).

https://github.com/niloc132/gwt-driver/blob/master/src/main/java/org/senchalabs/gwt/gwtdriver/ModuleUtilities.java#L73 is the code that bridges the gap between the two above, which shows one more place that things could be going insane, like System.arraycopy just not copying into the right place.

If you end up being able to reproduce this, those are places to debug and see what the heck is going on. An intermittently breaking example would also put me in a position to help.

PeterWippermann commented 8 years ago

I circumvented the problem by using a FluentWait and ignoring that exception. Unfortunately it's only a generic RuntimeException and no specific exception (e.g. a new "ClientMethodNotFoundException"), so my suppression is very vague. But for me that's fine at the moment.

Still I'd propose to fix this in the framework. As of now I've got the impression that this exception occurs, if no InfoPopups are present...

So do you want to leave this issue open?

niloc132 commented 8 years ago

Lets leave it open for now, with the expected resolution to add a done()-like method that either waits for it to show up, or for a with___()-like method that tells the finder to wait until the popup is there.

However, unless your error is incomplete, the error message you provided still sounds like something else is horribly wrong, so hopefully just a copy/paste error there.

On Tue, Jan 26, 2016 at 4:20 AM peterwippermann notifications@github.com wrote:

I circumvented the problem by using a FluentWait and ignoring that exception. Unfortunately it's only a generic RuntimeException and no specific exception (e.g. a new "ClientMethodNotFoundException"), so my suppression is very vague. But for me that's fine at the moment.

Still I'd propose to fix this in the framework. As of now I've got the impression that this exception occurs, if no InfoPopups are present...

So do you want to leave this issue open?

— Reply to this email directly or view it on GitHub https://github.com/niloc132/gxt-driver/issues/13#issuecomment-174944060.

PeterWippermann commented 8 years ago

Well, I didn't copy the whole stacktrace, if that's what you mean. The next line after at org.senchalabs.gwt.gwtdriver.gxt.models.InfoPopup$InfoPopupFinder.done(InfoPopup.java:72) would be my test code calling this framework method.

niloc132 commented 8 years ago

Nope, just this:

java.lang.RuntimeException: error: Error: could not find method 'com.sencha.gxt.widget.core.client.info.Info'

That isn't a possible value for 'method', it should be something like 'org.senchalabs.gwt.gwtdriver.invoke.ExportedMethods::instanceofwidget' instead, based on that stack trace. The next argument should be the Info.class.getName() string.

Somehow your args array had either missing or out of order elements, which really shouldn't even be possible.

PeterWippermann commented 8 years ago

I got a different kind of error, althoug similar to this one and certainly related. This one is either more weird :-D

java.lang.RuntimeException: error: Error: could not find method 'function (value) { sendResponse(value, 0); }' at org.senchalabs.gwt.gwtdriver.ModuleUtilities.executeExportedFunction(ModuleUtilities.java:99) at org.senchalabs.gwt.gwtdriver.invoke.ClientMethodsFactory$InvocationHandlerImplementation.invoke(ClientMethodsFactory.java:61) at com.sun.proxy.$Proxy17.getNodeElement(Unknown Source) at org.senchalabs.gwt.gwtdriver.gxt.models.Tree.findItemWithText(Tree.java:124) at ....

niloc132 commented 8 years ago

Yeah, something is very wrong with your stack trace - its as though it is passing in the callback (executeAsyncScript calls the JS with on more arg, a callback back into Selenium) as the only param, or somehow is misordering the parameters.

If you can consistently reproduce this: what browser/OS, and what version of selenium? Can you share a minimal app/test that can cause this? Otherwise, the best I can think of is to pass in some strangely-assembled data, and have the client code kick and scream when it makes no sense, to try to isolate the cases where this happens. However, that would require making the entire library more complex to solve a bug that seems to be specific to your environment...

PeterWippermann commented 8 years ago

I'm on Windows 7, Firefox 44.0 and Selenium-java 2.48.2. I will update you, if I find a way to reproduce it.

PeterWippermann commented 8 years ago

Stumbled on this error again.

Mrz 07, 2016 1:54:04 PM org.senchalabs.gwt.gwtdriver.ModuleUtilities executeExportedFunction WARNING: Error executing script: error: Error: could not find method 'com.google.gwt.user.client.ui.Widget'

Too provide even more data I took a screenshot while debugging the Java side of the code: debug

Again, the instanceofwidget() method causes the issue. In my code, I'm calling myWidget.as(myModel.class) while the actual WebElement referenced by myWidget has already been disposed on the browser side. MyModel is annotated with @ForWidget(com.google.gwt.user.client.ui.Widget.class). While the type of myWidget is annotated with @ForWidget(com.sencha.gxt.widget.core.client.Window.class).

So could this be the final cause for these weird errors: The WebElements already being disposed (i.e. "stale", see StaleElementReferenceException)?

branflake2267 commented 8 years ago

Is this a new xpath query or are you reusing a reference to the widget. From the sounds of it, the WebElement reference is stale and you have to call driver.find...(something) again.

PeterWippermann commented 8 years ago

Yes @branflake2267 the reference is stale for sure!

  1. How can I safely check, if a reference became stale?
  2. When calling .as() on such a widget with a stale reference, there should be a better error message. Maybe even a StaleElementReferenceException?
branflake2267 commented 8 years ago

It's been awhile since I've had to write a couple tests. But what I remember is having to start a new query for the widget, starting with the driver.find. I found I couldn't use someGwtWidget.find again when the widget was redrawn or some piece of it, like a renderer that adds html. I'm not sure if it's getting detached and re-attached or the DOM has changed. Usually too fast for my eye. In general I always tried to use driver.find when finding a widget except for drilling into some hierarchy, and if I caused the screen to change rendering I would start over at driver.find. I think I was defensive in mostly starting with driver.find more often than widget. I do know that if you can drill further into the dom hierarchy sooner, such as planting some debugAs id's it helps with speed and saves on time with having to use a widget reference.

PeterWippermann commented 8 years ago

Thanks @branflake2267 , but that doesn't really answer my question :-) I know how to obtain a fresh reference, once my old reference has become stale. But how can I safely check, if a widget's reference has become stale in the first place?

branflake2267 commented 8 years ago

I think a try catch could be used, if it fails, start over.

PeterWippermann commented 8 years ago

I would expect that the GxtDriver lib would take a little more work here.

Let's image the following use case: I want to check that a certain tab has closed. Therefore I want to check if my reference to a Button on that tab has become stale. How would the library support me here?

A minimum solution would be: Throw a StaleElementReferenceException (instead of a generic RuntimeException with a confusing error message) so I can explicitly catch it. This would have to be implemented in the ExportedMethods I guess.

Optimal solution: the GwtWidget class offers a member method "isDisposed()" for this.

niloc132 commented 8 years ago

Unfortunately, ChromeDriver just loves to stale the crap out of things. This is apparently a feature.

From org.openqa.selenium.support.ui.ExpectedConditions#stalenessOf you can check easily with a try/catch around just about any call to the element to see if it is stale. I'm unclear exactly why this isn't part of the WebElement itself.

If the bug you are seeing here (miss-ordered arguments when passed through ExportedMethods) is indeed a result of stale elements, then yes, we can fix this internally, but I'll want to confirm that first by forcing the element to be stale, and passing it in, then checking that it simply never arrives to the browser to be checked correctly, and thrown in a coherent way.

If that isn't what is causing your issue, then I'm guessing its time to add a lot more logging just to hunt down whatever insanity your tests are causing...

PeterWippermann commented 8 years ago

I just checked in one of my test cases and yes: a stale reference is causing my issue.

Clicking my "Login" button will cause the screen to be rerendered: Button test = GwtWidget.find(Button.class, driver).withText(LOGIN_BUTTON_LABEL).done(); test.click();

After a check for the new screen to have loaded the old button definitely became stale. So I call: Boolean b = test.is(GwtWidget.class); I reliably get an Exception then:

Mrz 09, 2016 3:16:32 PM org.senchalabs.gwt.gwtdriver.ModuleUtilities executeExportedFunction WARNING: Error executing script: error: Error: could not find method 'com.google.gwt.user.client.ui.Widget'

So I'd be very happy, if you could implement a check here :-)

One remark, calling click() on my stale Button didn't work because this actually called findElement() which would raise a StaleElementReferenceException first. So I avoided this by calling is() instead.

niloc132 commented 8 years ago

Great, thanks - I've updated the title to reflect what is actually wrong.

PeterWippermann commented 8 years ago

:+1: