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

RESOLVED FIXED in mozilla36

Status

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: mdas, Assigned: mdas)

Tracking

Trunk
mozilla36
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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.
Created attachment 8508136 [details] [diff] [review]
if test fails, raise clearer error
Attachment #8508136 - Flags: review?(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
Attachment #8508136 - Flags: review?(jgriffin) → review+
(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
https://hg.mozilla.org/mozilla-central/rev/3fefbf7f0b78
Assignee: nobody → mdas
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.