1027740 - Give more verbosity for accessibility test failures

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::UI Tests, defect)

Description

[Eitan Isaacson [:eeejay]]

We are having issues with intermittent failures on accessibility tests. There should be more verbosity to allow us to figure out the failure.

Comment 1

[Eitan Isaacson [:eeejay]]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21310

Comment 2

[Zac C (:zac)]

eeejay I triggered several TBPL builds to test for new intermittents.

https://tbpl.mozilla.org/?tree=Gaia-Try&rev=bdf411434b820f17cd1e213d24616ccfe38f953c

This error came up:
https://tbpl.mozilla.org/php/getParsedLog.php?id=42428864&tree=Gaia-Try&full=1

Do you expect to catch this error or is this the kind of debugging you want to be seeing?

Comment 3

[Zac C (:zac)]

Actually I can see now this test failing eleswhere without this patch so I don't think it's the patch causing the failure but still want to check with Eitan before we merge.

Comment 4

[Eitan Isaacson [:eeejay]]

(In reply to Zac C (:zac) from comment #2)
> eeejay I triggered several TBPL builds to test for new intermittents.
> 
> https://tbpl.mozilla.org/?tree=Gaia-
> Try&rev=bdf411434b820f17cd1e213d24616ccfe38f953c
> 
> This error came up:
> https://tbpl.mozilla.org/php/getParsedLog.php?id=42428864&tree=Gaia-
> Try&full=1
> 
> Do you expect to catch this error or is this the kind of debugging you want
> to be seeing?

Yup, this is what we want to see. Instead of the js silently failing and timing out the test, we should return the error and throw it in python.

Comment 5

[Zac C (:zac)]

Comment on attachment 8442947 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21310

r+ but it needs a rebase now.

Comment 6

[Eitan Isaacson [:eeejay]]

Comment on attachment 8442947 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21310

I'm re-requesting review since this patch reverts changes made in bug 1021710.

The problem with that change was that even though the tests will time out, the js will continue an infinite loop of recursive setTimeouts. This could interfere with future tests if the b2g process is not restarted. On devices this could get even worse, with these recursive timeouts recurring throughout the entire power cycle of the device.

I increased the timeout for getting an accessible from 100ms (10 attempts at 10ms apart, before bug 1021710), to 5000ms (500 attempts at 10ms apart). This should help avoid intermittent failures.

Comment 7

[Zac C (:zac)]

Is this loop in lieu of a wait? Are you trying to do a wait for 5000ms basically?

Marionette has a mechanism for this in waitFor , which will take care of timeouts, callbacks and so forth.
https://github.com/mozilla-b2g/gaia/blob/master/tests/atoms/gaia_apps.js#L209

Does that make it easier?

Comment 8

[Eitan Isaacson [:eeejay]]

(In reply to Zac C (:zac) from comment #7)
> Is this loop in lieu of a wait? Are you trying to do a wait for 5000ms
> basically?
> 
> Marionette has a mechanism for this in waitFor , which will take care of
> timeouts, callbacks and so forth.
> https://github.com/mozilla-b2g/gaia/blob/master/tests/atoms/gaia_apps.js#L209
> 
> Does that make it easier?

I think the waitFor function has a few issues:
1. It will defer to the script timeout and not allow catching the timeout for a more accurate error reporting.
2. It doesn't allow expected negative results, for example in our case we need a way to successfully return from the script when we don't find an accessible.

I tried using it after you suggested it, but problem #2 makes it fail.

Comment 9

[Zac C (:zac)]

I think this is a bit out of my ability so I'm going to ask someone else to r? it on my behalf.

Comment 10

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8442947 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21310

Added some comments in the PR.

Comment 11

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8442947 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21310

Made some comments in the PR; re-flag for review when ready.

Comment 12

[Eitan Isaacson [:eeejay]]

Comment on attachment 8442947 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21310

Pushed a new version for review.

Comment 13

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8442947 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21310

Looks good with two minor nits noted in the PR.

Comment 14

[Eitan Isaacson [:eeejay]]

https://github.com/mozilla-b2g/gaia/commit/390ea246e53c595b0bae8616a4ae09a4438f034b