Closed Bug 897924 Opened 11 years ago Closed 11 years ago

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

Categories

(Core :: DOM: Device Interfaces, defect)

Product:
Core
Component:
DOM: Device Interfaces
Platform:
ARM
Android
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

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

People

(Reporter: cbook, Assigned: stully)

References

(
URL
)

Details

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

Attachments

(3 files, 5 obsolete files)

test.diff
6.78 KB, patch
cpeterson
: review+
Details | Diff | Splinter Review
reenable-contacts-tests.patch
976 bytes, patch
stully
: review+
Details | Diff | Splinter Review
disable-tests.diff
8.91 KB, patch
Details | Diff | Splinter Review 
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: 11 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.diff — — Splinter 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: 11 years ago → 11 years ago
Resolution: --- → FIXED

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: