Gaia unit tests completely broken on TBPL

RESOLVED FIXED

Status

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jgriffin, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(8 attachments)

(Reporter)

Description

5 years ago
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.
(Reporter)

Comment 1

5 years ago
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.
Flags: needinfo?(jlal)
Created attachment 814414 [details]
Gaia PR for the dialer part

Here's a fix for the Dialer part.
Attachment #814414 - Flags: review?(anthony)
Created attachment 814424 [details]
Gaia patch for the system part

One line fix.
Attachment #814424 - Flags: review?(anthony)
(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...
^^ 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.
Flags: needinfo?(jlal)
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 :)
Attachment #814424 - Flags: review?(anthony) → review+
Comment on attachment 814414 [details]
Gaia PR for the dialer part

Yarp!
Attachment #814414 - Flags: review?(anthony) → review+
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.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
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
Status: RESOLVED → REOPENED
Flags: needinfo?(etienne)
Resolution: FIXED → ---
Blocks: 833666
(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 :)
Flags: needinfo?(etienne) → needinfo?(francisco.jordano)
Created attachment 816582 [details]
Comms pull request at https://github.com/mozilla-b2g/gaia/pull/12817

This should fix the contacts and ftu failures.
Attachment #816582 - Flags: review?(francisco.jordano)
Created attachment 816593 [details]
Homescreen pull request at https://github.com/mozilla-b2g/gaia/pull/12818

This should fix the homescreen failures.
Attachment #816593 - Flags: review?(crdlc)
Comment on attachment 816593 [details]
Homescreen pull request at https://github.com/mozilla-b2g/gaia/pull/12818

10x
Attachment #816593 - Flags: review?(crdlc) → review+
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!
Attachment #816582 - Flags: review?(francisco.jordano) → review+
Created attachment 816603 [details]
Video pull request at https://github.com/mozilla-b2g/gaia/pull/12819

This should fix the video errors.
Attachment #816603 - Flags: review?(dflanagan)
Created attachment 816606 [details]
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.
Attachment #816606 - Flags: review?(etienne)
Hopefully Ben would have solved this :)
(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.
Created attachment 816639 [details]
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.
Attachment #816639 - Flags: review?(jlal)
Flags: needinfo?(francisco.jordano)
Created attachment 816663 [details]
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.
Attachment #816663 - Flags: review?(kaze)
Depends on: 926524
I opened a separate bug for the email issue because there was a problem in non-test code.
Comment on attachment 816603 [details]
Video pull request at https://github.com/mozilla-b2g/gaia/pull/12819

Thanks, Ben!
Attachment #816603 - Flags: review?(dflanagan) → review+
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.
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".
(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 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!
Attachment #816606 - Flags: review?(etienne) → review+
(Reporter)

Updated

5 years ago
Depends on: 927097
Attachment #816663 - Flags: review?(kaze) → review+
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!
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
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.
(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.
You need to log in before you can comment on or make changes to this bug.