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

RESOLVED FIXED in mozilla37

Status

defect
P2
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

unspecified
mozilla37
Points:
1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [tech-debt])

Attachments

(1 attachment, 1 obsolete attachment)

+++ 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.
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".
Attachment #8478274 - Flags: review?(cmanchester)
Attachment #8478274 - Flags: feedback?(dmose)
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
Attachment #8478274 - Flags: feedback?(dmose) → feedback+
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?
Attachment #8478274 - Flags: review?(cmanchester) → review+
Whiteboard: [tech-debt]
backlog: --- → Fx36+
backlog: Fx36+ → Fx37+
Hi David,  Are you planning for you or someone from your team to finish this off - it looks like it was really close.
Flags: needinfo?(dburns)
Seeing as standard8 was doing the work on this I will let him answer
Flags: needinfo?(dburns) → needinfo?(standard8)
just for clarification, this requires nothing from the marionette team from what I can see
Moving this to P2 based on our new priority definitions.
Priority: P1 → P2
(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.
Flags: needinfo?(standard8)
https://hg.mozilla.org/integration/fx-team/rev/4cc15f56104c
Iteration: --- → 37.3
Points: --- → 1
Target Milestone: --- → mozilla37
https://hg.mozilla.org/mozilla-central/rev/4cc15f56104c
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.