Closed Bug 924233 Opened 6 years ago Closed 6 years ago

Gaia unit tests completely broken on TBPL

Categories

(Firefox OS Graveyard :: Gaia, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jgriffin, Unassigned)

References

Details

Attachments

(8 files)

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.
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)
Here's a fix for the Dialer part.
Attachment #814414 - Flags: review?(anthony)
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
Closed: 6 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 → ---
(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)
This should fix the contacts and ftu failures.
Attachment #816582 - Flags: review?(francisco.jordano)
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+
This should fix the video errors.
Attachment #816603 - Flags: review?(dflanagan)
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.
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)
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)
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+
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
Closed: 6 years ago6 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.