1058038 - Improve Marionette failure notices for frontend tests when the test fails to find "Complete"

Status: RESOLVED FIXED

Description

[Mark Banner (:standard8) (afk 19th - 28th July)]

+++ This bug was initially created as a clone of Bug #1054793 +++

Split off from bug 1054793.

Currently we get:

>  0:27.58 TEST_END: MainThread ERROR, expected PASS
> Traceback (most recent call last):
>   File "/Users/Jan/moz/mozilla-central/testing/marionette/client/marionette/marionette_test.py", line 171, in run
>     testMethod()
>   File "/Users/Jan/moz/mozilla-central/browser/components/loop/test/shared/test_shared_all.py", line 16, in test_units
>     self.check_page("index.html")
>   File "/Users/Jan/moz/mozilla-central/browser/components/loop/test/shared/frontend_tester.py", line 89, in check_page
>     self.marionette.find_element("id", 'complete')
>   File "/Users/Jan/moz/mozilla-central/testing/marionette/client/marionette/marionette.py", line 1278, in find_element
>     response = self._send_message('findElement', 'value', **kwargs)
>   File "/Users/Jan/moz/mozilla-central/testing/marionette/client/marionette/decorators.py", line 35, in _
>     return func(*args, **kwargs)
>   File "/Users/Jan/moz/mozilla-central/testing/marionette/client/marionette/marionette.py", line 638, in _send_message
>     self._handle_error(response)
>   File "/Users/Jan/moz/mozilla-central/testing/marionette/client/marionette/marionette.py", line 672, in _handle_error
>     raise errors.NoSuchElementException(message=message, status=status, stacktrace=stacktrace)
> NoSuchElementException: NoSuchElementException: Unable to locate element: complete

with no specific error message.

Comment 1

[Mark Banner (:standard8) (afk 19th - 28th July)]

Attachment: Improve Marionette failure notices for frontend tests when the test fails to find "Complete"

I'm attaching a patch which changes the failure messages to:

 0:17.46 TEST_END: MainThread FAIL, expected PASS
Traceback (most recent call last):
  File "/Users/mark/loop/gecko-dev/testing/marionette/client/marionette/marionette_test.py", line 171, in run
    testMethod()
  File "/Users/mark/loop/gecko-dev/browser/components/loop/test/shared/test_shared_all.py", line 16, in test_units
    self.check_page("index.html")
  File "/Users/mark/loop/gecko-dev/browser/components/loop/test/shared/frontend_tester.py", line 100, in check_page
    raise AssertionError(details)
AssertionError: browser/components/loop/test/shared/index.html: 1 failure encountered
TEST-UNEXPECTED-FAIL | browser/components/loop/test/shared/index.html | Waiting for Completion - Could not find the test complete indicator

This should at least make the failure mode better on tbpl for sheriffs, as we'll get the TEST-UNEXPECTED-FAIL returned.

I tested this by changing the find_element from "complete" to "complete1".

Comment 2

[Dan Mosedale (:dmose, :dmosedale)]

Comment on attachment 8478274 [details] [diff] [review]
Improve Marionette failure notices for frontend tests when the test fails to find "Complete"

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

Looks good to me

Comment 3

[Chris Manchester (Mostly Offline until July 29)]

Comment on attachment 8478274 [details] [diff] [review]
Improve Marionette failure notices for frontend tests when the test fails to find "Complete"

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

If this is providing useful diagnostics, looks good to me. Ongoing (under-publicized, I'm realizing) efforts by myself and others mean logging "TEST-UNEXPECTED-FAIL" is no longer the primary means for a test harness to communicate failure. Test harnesses are being converted to log a raw data format for test results that is serialized as JSON and can be made useful to a range of consumers. The formatter used by automation translates a failed test or error into a "TEST-UNEXPECTED" prefixed line for compatibility with tools like tbpl, but the default formatter used by mach does not. You can pass "--log-tbpl -" to mach to see how failure output would be formatted in this context.

::: browser/components/loop/test/shared/frontend_tester.py
@@ +87,5 @@
>  
>          self.marionette.navigate(urlparse.urljoin(self.server_prefix, page))
> +        try:
> +            self.marionette.find_element("id", 'complete1')
> +        except:

This commits to emitting the message below for any exception raised. Could this lead to confusion if an unexpected exception type is raised?

Comment 4

[:shell escalante]

Hi David,  Are you planning for you or someone from your team to finish this off - it looks like it was really close.

Comment 5

[David Burns :automatedtester]

Seeing as standard8 was doing the work on this I will let him answer

Comment 6

[David Burns :automatedtester]

just for clarification, this requires nothing from the marionette team from what I can see

Comment 7

[Maire Reavy [:mreavy] Plz needinfo]

Moving this to P2 based on our new priority definitions.

Comment 8

[Mark Banner (:standard8) (afk 19th - 28th July)]

(In reply to Chris Manchester (away until January 5th) [:chmanchester] from comment #3)
> If this is providing useful diagnostics, looks good to me. Ongoing
> (under-publicized, I'm realizing) efforts by myself and others mean logging
> "TEST-UNEXPECTED-FAIL" is no longer the primary means for a test harness to
> communicate failure. Test harnesses are being converted to log a raw data
> format for test results that is serialized as JSON and can be made useful to
> a range of consumers. The formatter used by automation translates a failed
> test or error into a "TEST-UNEXPECTED" prefixed line for compatibility with
> tools like tbpl, but the default formatter used by mach does not. You can
> pass "--log-tbpl -" to mach to see how failure output would be formatted in
> this context.

Filed bug 1117110 as a follow-up.

> ::: browser/components/loop/test/shared/frontend_tester.py
> @@ +87,5 @@
> >  
> >          self.marionette.navigate(urlparse.urljoin(self.server_prefix, page))
> > +        try:
> > +            self.marionette.find_element("id", 'complete1')
> > +        except:
> 
> This commits to emitting the message below for any exception raised. Could
> this lead to confusion if an unexpected exception type is raised?

Yep, limited to NoSuchElementException.

Comment 9

[Mark Banner (:standard8) (afk 19th - 28th July)]

Attachment: Improve Marionette failure notices for frontend tests when the test fails to find "Complete". v2

Updated patch

Comment 10

[Mark Banner (:standard8) (afk 19th - 28th July)]

https://hg.mozilla.org/integration/fx-team/rev/4cc15f56104c

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/4cc15f56104c