1093719 - Some indexedDB multi-process xpcshell tests run on Android

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Geoff Brown [:gbrown]]

Current Android xpcshell test logs show child and parent versions of some dom/indexedDB tests:

05:39:54     INFO -  TEST-START | xpcshell-child-process.ini:/builds/panda-0022/test/build/tests/xpcshell/tests/dom/indexedDB/test/unit/test_add_put.js
05:39:58     INFO -  TEST-PASS | xpcshell-child-process.ini:/builds/panda-0022/test/build/tests/xpcshell/tests/dom/indexedDB/test/unit/test_add_put.js | took 4152ms
...
05:44:18     INFO -  TEST-START | xpcshell-parent-process.ini:/builds/panda-0022/test/build/tests/xpcshell/tests/dom/indexedDB/test/unit/test_add_put.js
05:44:22     INFO -  TEST-PASS | xpcshell-parent-process.ini:/builds/panda-0022/test/build/tests/xpcshell/tests/dom/indexedDB/test/unit/test_add_put.js | took 3103ms

These are explicitly skipped on Android:

http://hg.mozilla.org/mozilla-central/annotate/cadcd47db610/dom/indexedDB/test/unit/xpcshell-parent-process.ini#l9
http://hg.mozilla.org/mozilla-central/annotate/cadcd47db610/dom/indexedDB/test/unit/xpcshell-child-process.ini#l9

But that may be lost when we include xpcshell-shared.ini.

This may have gone wrong in bug 1079278 or bug 1066735.


Some of these tests failed in spectacular fashion in bug 1091916.

Comment 1

[Geoff Brown [:gbrown]]

Historically (at least a few months ago), xpcshell tests in dom/indexedDB/test/unit were enumerated in dom/indexedDB/test/unit/xpcshell.ini. On Android, these tests were explicitly run via an include entry in  testing/xpcshell/xpcshell_android.ini.

In September, in bug 994190, dom/indexedDB/test/unit/xpcshell.ini was deleted and replaced by xpcshell-{client/parent/shared}.ini. At that time, since xpcshell_android.ini was not updated, all dom/indexedDB xpcshell tests no longer ran on Android.

In October, in bug 1066735, testing/xpcshell/xpcshell_android.ini was deleted and Android xpcshell tests started using the normal manifests. At that point, the indexedDB manifests would be read for Android once again. But xpcshell-shared.ini skipped all tests on Android -- so no tests were run on Android, still.

In bug 1079278, the skip-if(android) was removed from xpcshell-shared.ini, and indexedDB began running on Android in both child and parent versions, apparently without error (until bug 1091916).


:bent -- Can you clarify, what is the intent for indexedDB xpcshell tests on Android? Should they be run at all? Via xpcshell-client-process.ini / xpcshell-parent-process.ini?

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

I'd like all of them to run everywhere ideally. There was one test that didn't work due to some kind of packaging thing but that's already explicitly disabled.

Comment 3

[Geoff Brown [:gbrown]]

My concern is that tests run via xpcshell-client-process.ini will run multi-process (e10s) via run_test_in_child():

https://developer.mozilla.org/en-US/docs/Mozilla/QA/Writing_xpcshell-based_unit_tests#Testing_under_Electrolysis

I confirmed with mfinkle on irc today -- fennec does not support e10s and e10s xpcshell tests should be disabled on android.

I appreciate -- and am impressed! -- that these tests are currently running reliably on Android. But if they fail on Android, I suspect the failure would be considered invalid, since the test is running in an unsupported way. I also have concerns about crash reporting for the client, based on the experience in bug 1091916.

I think we need to modify the manifests such that none of the tests are run on Android via xpcshell-child-process.ini. It looks like tests run via xpcshell-parent-process.ini are single process and can continue to run. Can you confirm?

Comment 4

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Ok, let's just disable the child process ones on android then. We can file a followup to figure out why the tests break when run oop.

Comment 5

[Geoff Brown [:gbrown]]

Attachment: on Android, run parent only, not child

I think https://bugzilla.mozilla.org/show_bug.cgi?id=676876#c1 is coming into play here: The shared skip-if(gonk) is over-riding the skip-if(gonk|android) in the parent and child manifests.

Here I remove the DEFAULT section from the shared file and update the parent manifest to allow Android. (The client manifest already skips on gonk or android).

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=5efd4644cddc verifies:
 - on Android, only parent tests run; child tests are now skipped;
 - on B2G emulator, tests are still skipped for both parent and child;
 - on Linux, tests are still run for both parent and child.

Comment 6

[Luke Wagner [:luke]]

I pushed this patch along with patch for bug 1091916 to try
  https://tbpl.mozilla.org/?tree=Try&rev=0c5b0638e1d4
and I still seem to be getting a crash in a plugin-container with no crash symbols in dom/indexedDB/test/unit/test_globalObjects_ipc.js.  Should this test be disabled as well?

Comment 7

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Yes

Comment 8

[Geoff Brown [:gbrown]]

Attachment: on Android, run parent tests only, not child

This is the same as the previous patch (see Comment 5) with an additional special case to skip test_globalObjects_ipc.js (Comment 6 - thanks Luke!).

Comment 9

[Geoff Brown [:gbrown]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1e2dc324b7c7

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1e2dc324b7c7