1085558 - test_single_finger*.py tests should output the actual value if assertion fails

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Malini Das [:mdas] - Away, not checking bugmail]

the wait_for_condition calls in the test_single_finger*.py files are used to check if the text we expect is the text get. If it isn't, the test just throws "TimeoutException: TimeoutException: wait_for_condition timed out" instead of telling you what the actual and expected values were. Pretty useless if you're trying to see what's going on! We should report the actual/expected values on failure.

Comment 1

[Malini Das [:mdas] - Away, not checking bugmail]

Attachment: if test fails, raise clearer error

Comment 2

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8508136 [details] [diff] [review]
if test fails, raise clearer error

Review of attachment 8508136 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/marionette/client/marionette/tests/unit/single_finger_functions.py
@@ +2,5 @@
> +from errors import TimeoutException
> +
> +def wait_for_condition_else_raise(marionette, wait_for_condition, expected, script):
> +    try:
> +        wait_for_condition(lambda m: expected in m.execute_script(script))

Why do we need to pass wait_for_condition around?  It looks like we only use self.wait_for_condition.

@@ +5,5 @@
> +    try:
> +        wait_for_condition(lambda m: expected in m.execute_script(script))
> +    except TimeoutException as e:
> +        raise TimeoutException(e.msg + " got %s instead of %s" % (marionette.execute_script(script), expected))
> +    

nit: extra whitespace at beginning of line

Comment 3

[Malini Das [:mdas] - Away, not checking bugmail]

(In reply to Jonathan Griffin (:jgriffin) from comment #2)
> Comment on attachment 8508136 [details] [diff] [review]
> if test fails, raise clearer error
> 
> Review of attachment 8508136 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> :::
> testing/marionette/client/marionette/tests/unit/single_finger_functions.py
> @@ +2,5 @@
> > +from errors import TimeoutException
> > +
> > +def wait_for_condition_else_raise(marionette, wait_for_condition, expected, script):
> > +    try:
> > +        wait_for_condition(lambda m: expected in m.execute_script(script))
> 
> Why do we need to pass wait_for_condition around?  It looks like we only use
> self.wait_for_condition.
> 

single_finger_functions.py only provides functions, with no 'self' since they aren't classes, so the wait_for_condition_else_raise helper function has to be passed any objects it needs to use

Comment 4

[Malini Das [:mdas] - Away, not checking bugmail]

landed in m-i:
 https://hg.mozilla.org/integration/mozilla-inbound/rev/3fefbf7f0b78

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3fefbf7f0b78