Provide a "ViewIsLoaded" matcher which describes the details of why an unmatched view was not loaded

[jvervaec]

For example, if you receive a TimeoutException during a transition event for a page object with many Required (or RequireAll) elements, it would be helpful to know which element was not found.

com.redhat.synq.TimeoutException: Timed out after PT2M waiting for event to occur.
Event occurs when action is finished (com.redhat.darcy.webdriver.WebDriverBrowser$$Lambda$30/1785111044@1849db1a)
and then the item under examination is a View that is loaded in context, com.redhat.darcy.webdriver.WebDriverBrowser@89ff02e,
however the last examined result was, "gis.harness.salesforce.pages.partner.AccountPage@5f3b9c57",
which is not a View that is loaded in context, com.redhat.darcy.webdriver.WebDriverBrowser@89ff02e (as determined by polling every PT1S)

[alechenninger]

Hrm. This would definitely be nice, but not exactly sure how to solve this. Right now this event gets its description from its matcher, which is: https://github.com/darcy-framework/darcy-ui/blob/master/src/main/java/com/redhat/darcy/ui/matchers/ViewIsLoadedInContext.java

This matcher has no idea about any of the specifics of the load condition, it's just asking the view if it is loaded.

[alechenninger]

/cc @jblashka -- We had chatted about this same issue before, so, if you're curious...

So, good news and bad news.

Bad news is this can't be done in any intelligent way inside a Matcher. Matchers can't describe a match based on the thing that it is matching. In other words, a specific View determines the details of how it is loaded. A Matcher is not based on a specific View, therefore it can only say that it can be used to match "any View that is loaded" -- it knows nothing about the details of how any particular View is loaded, because it is not tied to any particular View.

The good news is that Conditions, a construct in Synq, are perfectly suited for this task. A condition is not generic like a Matcher -- a condition is associated with one object (or supplier of objects), and some logic about that object. So in our case, that object is a View, and the logic is about determining whether or not it is loaded. The question now is how to get at the details of that logic.

And here is where it gets tricky.

Option 1 is rely on reflection magic to get the view's specific load condition logic. The good thing about this is that there is already a reusable facility to do this: Analyzer. This is what AbstractView uses to find out load conditions from annotations right now. So we could just use the same thing. Done!

The issue with this is this assumes you are using these annotations. Which, while a safe assumption, is a fairly big deal for the rest of the framework. Everything else about darcy's inner workings -- the stuff that actually produces those not-descriptive-enough-timeout-messages -- currently assumes you are using vanilla views. And therefore switching that assumption is a pretty big deal, and would be necessary to get any benefit.

Option 2 is much more elegant, but would break everyone already consuming darcy. In this option, we change the View API significantly (as well as ViewElement). Instead of isLoaded returning true or false, it returns a Condition<T>, where T is your View subtype (or we just use Condition<View>, which is kind of lame but might work okay ish still). If we use T, this mean's View is now parameterized, and so all Views would have to change to class MyView extends AbstractView<MyView>. Now, consuming this API would look like this...

Instead of...

assertTrue(myView.isLoaded());

// or if you wish to wait using Synq
expect(myView, is(loaded())).waitUpTo(1, MINUTES);

You would do...

// isLoaded returns a Condition now, remember...
assertTrue(myView.isLoaded().isMet());

// expect works with conditions directly as an argument
expect(myView.isLoaded()).waitUpTo(1, MINUTES);

Long term, I am more or less convinced that option 2 is the best way to go, and I kick myself because IIRC this is how it used to work in the very beginning days of the framework. Doh! Serves me right for switching to primitives instead of domain-specific constructs which can extend and live and breathe :).

So, I'm prepared to start the upheaval. It'll take a solid weekend. Unfortunately early adopters will have to be prepared to do some refactoring too once they bump versions. On the other hand though, the refactoring will be very easy... you could almost script it. Any class MyView extends AbstractView becomes class MyView extends AbstractView<MyView>. And for most people I think that'll be it. Most of the time you don't have a need to call isLoaded or use the loaded matcher directly. And if you do that is just a few characters change too. And once you do all that, you get really awesome load condition descriptions for your views.

So I think it'll be a fair trade for the productivity gain. Whataya think?

Good thing we're still in 0.x.x versions... I knew I was putting of 1.0.0 for a reason :P

[alechenninger]

/cc @dmcneil @abocz ^ potential breaking change incoming to View API

[alechenninger]

Also, internally, lots would change, but for the most part consumers would not be affected. For instance, the common expect(transition().to(new SomeOtherView)) would still work exactly the same with the new API (except you'd get much better timeout messages).

And to be clear, the timeout messages would be much better. They would say everything about the view's load (or displayed or presence) condition that wasn't met. So if 10 out of the 11 required elements are not being found, you'd see a message for each of them instead of just 1 of them, so you could diagnose accordingly.

[alechenninger]

One thing I don't like about option 2 is that it breaks the pattern of the other "is" methods. There is "isPresent" and "isDisplayed" as well, and those can be awaited. For ViewElements those can encapsulate some complicate set of element conditions as well. So to be consistent that would be mean changing all of the element API's to so that isPresent and isDisplayed returns conditions, too. That's a pretty big deal.

Have to think on that one.

[alechenninger]

So after thinking about this for a few days ultimately I'm failing to come up with anything that doesn't have lots of holes in it. So for now I would recommend you just set some breakpoints when you need to look into the details of why your view is not being considered loaded.