Andreas Tolfsen ❲:ato❳ Assignee
	Description • 10 years ago

The last raised exception (last_exc) from the condition passed to the marionette.wait.Wait class should be a TimeoutException containing the original exception's stack trace.

The current behaviour is confusing in a scenario like the following, where an exception is explicitly ignored yet raises if the p tag isn't found.

    el = Wait(marionette, ignored_exceptions=errors.NoSuchElementException).until(lambda m: m.find_element(By.TAG_NAME, "p"))

	Andreas Tolfsen ❲:ato❳ Assignee
	Comment 1 • 10 years ago

try: https://tbpl.mozilla.org/?tree=Try&rev=2249e0c00b5a

	Dave Hunt [:davehunt] [he/him] ⌚BST
	Comment 3 • 10 years ago

Comment on attachment 8362479 [details] [diff] [review]
0001-Bug-958024-Wrap-last-exception-from-Marionette-s-Wai.patch

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

Looks good to me, but I'd be more comfortable with mdas given an additional review.

	Malini Das [:mdas] - Away, not checking bugmail
	Comment 4 • 10 years ago

Comment on attachment 8362479 [details] [diff] [review]
0001-Bug-958024-Wrap-last-exception-from-Marionette-s-Wai.patch

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

::: testing/marionette/client/marionette/errors.py
@@ +52,3 @@
>          if self.stacktrace:
> +            stack = "".join(["\t%s\n" % x for x in self.stacktrace.splitlines()])
> +            msg += "\nstacktrace:\n%s" % stack

thanks, this is nicer.

::: testing/marionette/client/marionette/tests/unit/test_errors.py
@@ +13,5 @@
> +    try:
> +        raise ValueError("bar")
> +    except ValueError as e:
> +        return sys.exc_info()
> +    raise

is this 'raise' needed? Try/except blocks should implicitly raise any exception that isn't ValueError.

	Andreas Tolfsen ❲:ato❳ Assignee
	Comment 5 • 10 years ago

Well spotted, thanks!  I've removed the extra raise and carried over the r+.

	Ryan VanderMeulen [:RyanVM]
	Comment 6 • 10 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/3c5f276268e8

	Ryan VanderMeulen [:RyanVM]
	Comment 7 • 10 years ago

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