Hi Edy, On 22 Feb 2015 at 14:54:32, Eduard Moraru ([email protected](mailto:[email protected])) wrote:

Hi,

I do see the value in moving the generic methods outside of XWiki's page objects and into a common place where we keep "utility" methods, mostly for cleanup and refactoring purposes, so +1 for that.

I am not entirely sure that the common place for "utility" methods should be an overload of RemoteWebDriver, instead of just an utility class/tool, just like we do now. The advantage right now is that we do not pollute the WebDriver API and it is very clear what is standard API and what is our utility package. IMO, this helps when debugging (seemingly) WebDriver related issues, as otherwise we might be confused into believing that we are using a standard WebDriver feature (which is not working well) when we are actually using something that we have added and needs to be fixed at our level. However, I also see the commondity in not having to know what is default and what is custom and just using one tool to do the job, so I am +0 for it.

While implementing it I’ve found it a lot more natural and it groups together all driver-related methods. Actually you could also view these methods as methods that should be provided by WebDriver but that they don’t provide ATM but they may provide them in the future.

I am not sure I understand your 3rd bullet, since, AFAIK, whenever an element is failed to be found by the WebDriver API, we get an exception and the test generally fails. Are you talking here about the wrapper code and the default behavior for our utility methods? Need more details here, but all I have to say here is that we need to make sure we allow looking for possibly inexistent elements on the page, even if we are waiting (for some JS to execute).

I’m talking about the places where we use findElement(s) instead of findElement(s)WithoutWaiting when an element is expected to not be there. For example thanks to this I’ve just improved the Panel tests and they now take 50 seconds less to execute. Yes, I know there might be rare places where we need to wait dynamically for elements to not be there. For these cases, we should use our hasElementWithoutWaiting() and hasElement() methods. ATM I haven't put the timeout check on those but one idea I have is to introduce a doesntHaveElement() for these special cases. But I believe that for 99% of other cases, when findElement(s) reaches the timeout, it means the test is not written correctly. I’m still verifying what’s happening in our tests ATM. Now, I’m still testing this and I’m still unsure about what will make sense to commit yet :) Still experimenting. Thanks for the feedback! -Vincent PS: Just committed the first part. If someone doesn’t agree I can still revert but I find it better than before personally.

Thanks, Eduard

On Sun, Feb 22, 2015 at 11:37 AM, [email protected] wrote:

...

Hi devs,

I’m working on improving our test framework by doing the following:

* Introduce a wrapping RemoteWebDriver called XWikiWebDriver * Move all the generic methods (i.e. that are not related to XWiki’s specific UI) we have in TestUtil + BaseElement into XWikiWebDriver (all the findElement(s)WithoutWaiting(), hasElement*(), waitFor*(), etc). The idea is simply to augment the WebDriver-provided APIs with all the methods we have created because they were useful to us. * Add an automated check when calling method that wait (e.g. findElement(), findElements()) so that if the timeout is incurred, we fail the test. The reason is that if we incur the timeout it means the test itself is not doing it right and is taking too much time!

Let me know if you don’t agree with something (as quickly as possible since I’ve already spent a few hours on it ;)).

Thanks! -Vincent