Closed Bug 897924 Opened 6 years ago Closed 6 years ago

Intermittent test_contacts_basics.html | No contacts after clear - got 124 (or 84), expected 0

Categories

(Core :: DOM: Device Interfaces, defect)

ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25
Tracking Status
firefox24 --- unaffected
firefox25 --- wontfix
firefox26 --- fixed

People

(Reporter: cbook, Assigned: stully)

References

()

Details

(Keywords: intermittent-failure, Whiteboard: [test disabled on Android])

Attachments

(3 files, 5 obsolete files)

Android Tegra 250 mozilla-inbound opt test mochitest-3 on 2013-07-25 02:12:25 PDT for push fb1a587df374

slave: tegra-256

https://tbpl.mozilla.org/php/getParsedLog.php?id=25707970&tree=Mozilla-Inbound

21050 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/contacts/tests/test_contacts_basics.html | No contacts after clear - got 124, expected 0
Reuben, can you take a look please?
Component: IPC → DOM: Device Interfaces
Flags: needinfo?(reuben.bmo)
Summary: Intermittent TEST-UNEXPECTED-FAIL | /tests/dom/contacts/tests/test_contacts_basics.html | No contacts after clear - got 124, expected 0 → Intermittent TEST-UNEXPECTED-FAIL | /tests/dom/contacts/tests/test_contacts_basics.html | No contacts after clear - got 124 (or 84), expected 0
Or Gregor? This is trending to easily be a top orange by next week.
Flags: needinfo?(anygregor)
Assignee: nobody → stully
Flags: needinfo?(reuben.bmo)
Flags: needinfo?(anygregor)
It seems like clearing the DB is not working.
Oh this is only Android and contacts API for android landed.
Should we backout bug 857730 for now? Looks like things are rather broken.
I don't have a problem clearing the database locally so I think it's a problem unique to Android 2.2 on the test server (since there's no problem on Android 4.0). I don't have a 2.2 device to test on at the moment so if it can wait until tomorrow morning, I can investigate what's happening on 2.2 closer.
https://hg.mozilla.org/mozilla-central/rev/3bc8a3f77169
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Reopening as disabling the tests does not fix the problem.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [test disabled on Android][leave open]
(In reply to Shane Tully (:stully: from comment #93)
> Reopening as disabling the tests does not fix the problem.

The merge script would have looked for the presence of [leave open] in the whiteboard :-)
Summary: Intermittent TEST-UNEXPECTED-FAIL | /tests/dom/contacts/tests/test_contacts_basics.html | No contacts after clear - got 124 (or 84), expected 0 → Intermittent test_contacts_basics.html | No contacts after clear - got 124 (or 84), expected 0
Attached patch test.diffSplinter Review
Not directly related to this bug, but these are small changes to the Android contacts code that were made while investigating this bug.
Attachment #791425 - Flags: review?(cpeterson)
Attached patch disable-tests.diff (obsolete) — Splinter Review
The contacts tests pass locally on all devices tested, including the tegra devices used on tbpl. They only fail in the test environment on Android 2.2. Thus, we should disabled the tests on Android 2.2 only to avoid regressions in Android 4.0+ (where the tests do not fail).
Attachment #791426 - Flags: review?(cpeterson)
Comment on attachment 791426 [details] [diff] [review]
disable-tests.diff

Review of attachment 791426 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, but someone more familiar with the Contacts tests should review this patch.
Attachment #791426 - Flags: review?(cpeterson) → feedback+
Comment on attachment 791425 [details] [diff] [review]
test.diff

Review of attachment 791425 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM!
Attachment #791425 - Flags: review?(cpeterson) → review+
Comment on attachment 791425 [details] [diff] [review]
test.diff

Review of attachment 791425 [details] [diff] [review]:
-----------------------------------------------------------------

Don't forget to update the other tests.
Comment on attachment 791426 [details] [diff] [review]
disable-tests.diff

Review of attachment 791426 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/contacts/tests/test_contacts_basics.html
@@ +39,5 @@
> +                                    .getProperty('version');
> +
> +  // Skip tests on Android < 4.0 due to test failures on tbpl (see bugs 897924 & 888891)
> +  if (androidVersion < 14) {
> +    skipTests = true;

Remove the boolean and just call SimpleTest.finish(). Same for the other tests.
(In reply to Chris Peterson (:cpeterson) from comment #109)
> Landed test.diff for ContactService.java:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/a279ce237586

You also landed the other chunks. Please fix the other tests in a follow-up…
Attached patch disable-tests.diff (obsolete) — Splinter Review
Keeping the skipTests variable since calling SimpleTest.finish() twice throws an error.
Attachment #791426 - Attachment is obsolete: true
Attachment #791540 - Flags: review?(reuben.bmo)
Comment on attachment 791540 [details] [diff] [review]
disable-tests.diff

Review of attachment 791540 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Shane Tully (:stully: from comment #111)
> Keeping the skipTests variable since calling SimpleTest.finish() twice
> throws an error.

That's because you haven't called SimpleTest.waitForExplicitFinish yet. Just move the call next to |addLoadEvent| to the beginning of the test.

>try: -b do -p android,android-armv6,android-noion -u mochitest-3 -t none
>Bug 897924 - Intermittent test_contacts_basics.html | No contacts after clear - got 124 (or 84), expected 0
Remember to update the message before pushing. Describe the fix, not the bug, see https://developer.mozilla.org/en-US/docs/Developer_Guide/Committing_Rules_and_Responsibilities#Checkin_comment

r=me with these things fixed.

::: dom/contacts/tests/test_contacts_basics.html
@@ +39,5 @@
> +                                    .getProperty('version');
> +
> +  // Skip tests on Android < 4.0 due to test failures on tbpl (see bugs 897924 & 888891)
> +  if (androidVersion < 14) {
> +    ok(true, "Skip tests on Android < 4.0 (bugs 897924 & 888891");

nit: Use info(). (Yea, I know, we use ok(true, …) elsewhere, but might as well do the right thing in new code).
Attachment #791540 - Flags: review?(reuben.bmo) → review+
Attached patch disable-tests.diff (obsolete) — Splinter Review
Changed title
Attachment #791540 - Attachment is obsolete: true
Attached patch disable-tests.diff (obsolete) — Splinter Review
Attachment #791570 - Attachment is obsolete: true
Attachment #792402 - Flags: review?(stully) → review+
And once Android got around to running tests, https://tbpl.mozilla.org/php/getParsedLog.php?id=26745301&tree=Mozilla-Inbound - apparently info() doesn't count as running a test, so you have to todo(false, "sure wish I could run this test on Android")
Attached patch disable-tests.diff (obsolete) — Splinter Review
Attachment #792308 - Attachment is obsolete: true
Removed try syntax
Attachment #792888 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/74523d9f3f61
https://hg.mozilla.org/mozilla-central/rev/87517eb76d6b
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Duplicate of this bug: 897657
You need to log in before you can comment on or make changes to this bug.