provide more accurate accounting of selenium failures

[MarcelWilson]

While updating tests for the latest changes to my current project I realized the errors appearing in the log weren't adequate enough for me to realize why the test had failed.

By all accounts the way this logs it is easy to assume that the element in question is missing.

11:53:53  INFO Admin sees if the logo is clickable.
11:53:53  INFO     Admin inspects the logo.
11:53:53  INFO     ... hoping it's clickable
11:53:53  INFO         => False
11:53:53  INFO     ***ERROR***

AssertionError: 
Expected: the element is enabled/clickable
     but: was not even present

Technically speaking, it couldn't be found because the target was incorrect. The TargetingError raised in Element is simply caught and the log never reflects the reason why it is not present.

It would be nice to show the selenium exception that caused the test to fail:
no such element: Unable to locate element: {"method":"xpath","selector":"//li[@data-testid=side-nav-logo-expanded]"}

Something like:

AssertionError: 
Expected: the element is enabled/clickable
     but: Unable to locate element: {"method":"xpath","selector":"//li[@data-testid=side-nav-logo-expanded]"}

[MarcelWilson]

I don't see a good way to incorporate the selenium exception in the matcher classes. Perhaps it would be sufficient to report the exception after the fact.

AssertionError: 
Expected: the element is enabled/clickable
     but: was not even present
reason: Unable to locate element: {"method":"xpath","selector":"//li[@data-testid=side-nav-logo-expanded]"}

[perrygoy]

Hm... what makes me unsure if this approach will help is that Selenium doesn't know if the locator is incorrect or whether the correct locator can't locate the non-existant element. But maybe showing the locator would be helpful for someone reading the logs.

I think we might be able to store the exception in the matcher to reference later?

Edit: Ah, that's right, the Element question is the one that does the finding. We could add an aside to Element if there's an exception caught, maybe that would work? It would show the exception before the test, though.

[MarcelWilson]

"It would show the exception before the test, though."

That's what I was afraid of. We would almost need some way of capturing the exception making it accessible higher up the stack.

In the following situation See is handling the test. We'd want the exception to be available at that point.
actor.should(See(Element(SIDE_NAV_DASHBOARD), IsClickable()))

What if question objects stored caught exception?

I noticed hamcrest assertions have the argument 'reason' that could be used as a way to log the exception.

[MarcelWilson]

I have the basic concept working. It requires making changes to screenpy as well.

bandophahita/screenpy@fde3435

[MarcelWilson]

Put into practice with a bad xpath:

    administrator.should(See(Element(LOGO), IsClickable()))

10:27:38  INFO Admin sees if the logo is clickable.
10:27:38  INFO     Admin inspects the logo.
10:27:38  INFO     ... hoping it's clickable
10:27:38  INFO         => False
10:27:38  INFO     ***ERROR***

AssertionError: Message: no such element: Unable to locate element: {"method":"xpath","selector":"//li[@data-testid=side-nav-logo-expanded]"}
<< truncated chrome stacktrace >> 
 raised while trying to find logo.
Expected: the element is enabled/clickable
     but: was not even present

[MarcelWilson]

I'm happy with the end result.

I'm curious what everyone's thoughts are on going this route. If we pursue this, does it apply to all places where questions are answered (e.g. MakeNoteOf) or only in places where tests occur (e.g. See).

[perrygoy]

bandophahita/screenpy@8f7a2fc

I think i do like this approach, especially how you've made it opt-in through the protocol. This way, any Questions which catch exceptions and could make a clearer failure during See can use that feature, while leaving the rest alone.

I agree that MakeNoteOf could benefit from this handling, too, but maybe as an aside just in case something happens later. Thinking about the Element case here, it might be useful to know the reason the element was not found when debugging whatever you made a note about. Something like "by the way, this exception was caught, maybe that's relevant or something i dunno" (probably more sterile language than that but you know what i mean).

[MarcelWilson]

I'm thinking that who/whatever uses the Answerable can check if there is an exception caught. Since they are ultimately the ones to decide what to do about it. Otherwise they would be facing a potential logging that occurs when they don't want it to. I'm thinking of those cases where we are expecting a failure and catching it.

As for MakeNote I started thinking it doesn't behave like a test (See) because Director().notes() doesn't have a reason argument. Question is, should it?