924233 - Gaia unit tests completely broken on TBPL

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Jonathan Griffin (:jgriffin)]

The gaia unit tests have been completely broken on TBPL since they were changed to run one test at a time, instead of in one large group; see bug 903536.

The failures are fairly inconsistent; the most common are similar to "gaia-unit-tests TEST-UNEXPECTED-FAIL | communications/dialer/test/unit/telephony_helper_test.js | telephony helper should sanitize the given phone number before dialing | navigator.mozL10n is undefined", but there are many others.

See the "G" tests at https://tbpl.mozilla.org/?showall=1&tree=B2g-Inbound.

These need to be fixed or turned off; they provide no value at present.

Comment 1

[Jonathan Griffin (:jgriffin)]

James, is there anyone on the Gaia team who could look at this?

Also, we may want to reconsider the approach in bug 903536, since it makes the tests run differently in Travis than in TBPL; a better fix would be to have the harness report filenames itself.

Comment 2

[Etienne Segonzac (:etienne)]

Attachment: Gaia PR for the dialer part

Here's a fix for the Dialer part.

Comment 3

[(inactive after 6/18) Alive Kuo [:alive]]

Attachment: Gaia patch for the system part

One line fix.

Comment 4

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Alive Kuo [:alive] from comment #3)
> Created attachment 814424 [details]
> Gaia patch for the system part
> 
> One line fix.

Not one line now - I checked all the tests under system.
I wonder why the one moves the mock_settings_listener didn't fix this...

Comment 5

[James Lal [:lightsofapollo]]

^^ looks like some people have accepted the challenge to make this work :)

test-agent has a lot of legacy now and we have plans to improve it dramatically https://bugzilla.mozilla.org/show_bug.cgi?id=923814 which will address our test-in-isolation concerns as well as filename reporting.

This is not my priority at the moment so we need to get what we have working first and as the test-agent people (and hopefully other people who care) have time we can move towards a much better solution and deploy that based on the marionette-js-runner mozharness that was done recently.

Comment 6

[Etienne Segonzac (:etienne)]

Comment on attachment 814424 [details]
Gaia patch for the system part

Stealing Rik's review as discussed with him.

All good, and Travis is happy :)

Comment 7

[Anthony Ricaud (:rik)]

Comment on attachment 814414 [details]
Gaia PR for the dialer part

Yarp!

Comment 8

[Etienne Segonzac (:etienne)]

https://github.com/mozilla-b2g/gaia/commit/ee8bf97c8824e850ea9669f0e273cd21eb817f4a
https://github.com/mozilla-b2g/gaia/commit/e378651483294c0e26c912b73ac6f6c906e553d3

Should be much better, please re-open if tbpl is still red.

Comment 9

[James Lal [:lightsofapollo]]

Still looks like we have some failures: https://tbpl.mozilla.org/php/getParsedLog.php?id=28991538

I will find someone to look at the email part is there someone who can look at FTU/Contacts

Comment 10

[Etienne Segonzac (:etienne)]

(In reply to James Lal [:lightsofapollo] from comment #9)
> who can look
> at FTU/Contacts

If someone can do both of them it's Francisco :)

Comment 11

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Comms pull request at https://github.com/mozilla-b2g/gaia/pull/12817

This should fix the contacts and ftu failures.

Comment 12

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Homescreen pull request at https://github.com/mozilla-b2g/gaia/pull/12818

This should fix the homescreen failures.

Comment 13

[Cristian Rodriguez (:crdlc)]

Comment on attachment 816593 [details]
Homescreen pull request at https://github.com/mozilla-b2g/gaia/pull/12818

10x

Comment 14

[Ben Kelly [:bkelly, not reviewing]]

Homescreen changes merged:

  https://github.com/mozilla-b2g/gaia/commit/af2f0dc898da9622c022956a7fc8f388ddc02fce

Comment 15

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 816582 [details]
Comms pull request at https://github.com/mozilla-b2g/gaia/pull/12817

Tried (locally) and working for me.

Thanks Ben!

Comment 16

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Video pull request at https://github.com/mozilla-b2g/gaia/pull/12819

This should fix the video errors.

Comment 17

[Ben Kelly [:bkelly, not reviewing]]

Comms patch merged:

 https://github.com/mozilla-b2g/gaia/commit/74e7e69d98206bda5c517c0ada57d43de662913e

Comment 18

[Ben Kelly [:bkelly, not reviewing]]

Attachment: System pull request at https://github.com/mozilla-b2g/gaia/pull/12821

This should fix the system app failure.  The icc_helper.js is being lazy loaded, but the test framework thinks this is a link.  Require icc_helper.js up front to avoid this.

Comment 19

[Francisco Jordano [:arcturus] [:francisco]]

Hopefully Ben would have solved this :)

Comment 20

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Francisco Jordano [:arcturus] from comment #19)
> Hopefully Ben would have solved this :)

Unfortunately not. :-( That was a quick pass to fix the easier failures.  The calendar, email, and gallery issues might require more investigation.

Comment 21

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Calendar pull request at https://github.com/mozilla-b2g/gaia/pull/12824

I think I figured out the calendar issue.  Some of the tests were using the wrong arguments to View.delegate().  Not sure how this was passing on travis.

Comment 22

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Gallery pull request at https://github.com/mozilla-b2g/gaia/pull/12828

Figured out the gallery issue.  The sinon xhr cleanup function was being run too many times because the tests trigger additional translations.  Again, not sure how this was passing on travis.

Comment 23

[Ben Kelly [:bkelly, not reviewing]]

I opened a separate bug for the email issue because there was a problem in non-test code.

Comment 24

[David Flanagan [:djf]]

Comment on attachment 816603 [details]
Video pull request at https://github.com/mozilla-b2g/gaia/pull/12819

Thanks, Ben!

Comment 25

[Ben Kelly [:bkelly, not reviewing]]

Thanks David!  Merged:

  https://github.com/mozilla-b2g/gaia/commit/8b09ca932ee3519f87caf24bf65cfb02b166032d

Comment 26

[Ben Kelly [:bkelly, not reviewing]]

Regarding my "not sure how this was passing on travis" comments, I have a couple ideas.  I sent this in email, but thought I would post here:

Many of those error differences can be explained by the "run each test by itself" vs "run them all in a batch".  A lot of the failures succeed in batch mode because they get things defined by earlier tests.  (We need better isolation in batch mode.)

I think the gallery and email failures might pass often on travis because they require some amount of timing to trigger.  If the main test completes before an event handler fires, then the test suite carries on.  If the event handler gets a chance to fire first, then the error occurs.

I don't have a theory for the calendar failure at the moment.

Comment 27

[Phil Ringnalda (:philor)]

Is it possible that travis can't count, like releng automation apparently can't (or more accurately, that the harness running the tests and deciding on the exit code can't count)? One of your recent runs on b2g-inbound, with 12 failed tests, was "green".

Comment 28

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Phil Ringnalda (:philor) from comment #27)
> Is it possible that travis can't count, like releng automation apparently
> can't (or more accurately, that the harness running the tests and deciding
> on the exit code can't count)? One of your recent runs on b2g-inbound, with
> 12 failed tests, was "green".

Like this one:

  https://tbpl.mozilla.org/php/getParsedLog.php?id=29090186&full=1&branch=b2g-inbound

Perhaps this should check self.failures to check the same value being used to display the summary:

  https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-unit-tests/gaia_unit_test/main.py#L52

I don't think travis runs the python code, though, so this probably does not effect those results.

Comment 29

[Etienne Segonzac (:etienne)]

Comment on attachment 816606 [details]
System pull request at https://github.com/mozilla-b2g/gaia/pull/12821

r+

And kudos on fixing all those tests so quickly!

Comment 30

[Ben Kelly [:bkelly, not reviewing]]

Thanks Etienne!  System patch merged:

  https://github.com/mozilla-b2g/gaia/commit/58b1117cf45831795cf763c5eeb370900830f256

Comment 31

[Ben Kelly [:bkelly, not reviewing]]

Thanks Kaze!  Gallery l10n.js part merged:

  https://github.com/mozilla-b2g/gaia/commit/f1dbbef1451b1d842111c5eafbddf8b7eda359bb

Comment 32

[James Lal [:lightsofapollo]]

Comment on attachment 816639 [details]
Calendar pull request at https://github.com/mozilla-b2g/gaia/pull/12824

https://github.com/mozilla-b2g/gaia/commit/3fde1915cbb61209be1397a455f130d9a92794a2

Comment 33

[Ben Kelly [:bkelly, not reviewing]]

Gaia unit tests appear to be green in recent TBPL and all attached fixes have been committed.  I'm going to mark this resolved.  Please re-open if you think this is in error.  Thanks!

Comment 34

[Phil Ringnalda (:philor)]

Well, I wouldn't call them green, more... permaorange-but-very-much-better, but there's nothing wrong with filing a separate bug for the remaining newly-introduced permaorange that's apparently the result of bug 905173.

Comment 35

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Phil Ringnalda (:philor) from comment #34)
> Well, I wouldn't call them green, more... permaorange-but-very-much-better,
> but there's nothing wrong with filing a separate bug for the remaining
> newly-introduced permaorange that's apparently the result of bug 905173.

Sorry, I was looking at the gaia-unit tests (Gu) by accident.