Closed Bug 878587 Opened 11 years ago Closed 10 years ago

Intermittent testMasterPassword | Exception caught - junit.framework.AssertionFailedError: Click can not be completed!

Categories

(Firefox for Android Graveyard :: General, defect)

Product:
Firefox for Android Graveyard
Component:
General
Platform:
ARM
Android
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED WORKSFORME
Milestone:
Firefox 24

People

(Reporter: philor, Unassigned)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file, 2 obsolete files)

MasterPassword.patch
11.98 KB, patch
jmaher
: review-
Details | Diff | Splinter Review 
https://tbpl.mozilla.org/php/getParsedLog.php?id=23694618&tree=Mozilla-Central
Android 4.0 Panda mozilla-central opt test robocop-2 on 2013-06-02 04:27:49 PDT for push 295a11e1efef
slave: panda-0708

16 INFO TEST-PASS | testMasterPassword | Doorhanger notification is displayed - true should equal true
Exception caught during test!
junit.framework.AssertionFailedError: Click can not be completed!
	at junit.framework.Assert.fail(Assert.java:47)
	at junit.framework.Assert.assertTrue(Assert.java:20)
	at com.jayway.android.robotium.solo.Clicker.clickOnScreen(Clicker.java:91)
	at com.jayway.android.robotium.solo.Clicker.clickOnScreen(Clicker.java:166)
	at com.jayway.android.robotium.solo.Clicker.clickOnScreen(Clicker.java:139)
	at com.jayway.android.robotium.solo.Clicker.clickOn(Clicker.java:356)
	at com.jayway.android.robotium.solo.Solo.clickOnButton(Solo.java:685)
	at org.mozilla.fennec.tests.testMasterPassword.verifyLoginPage(testMasterPassword.java:206)
	at org.mozilla.fennec.tests.testMasterPassword.testMasterPassword(testMasterPassword.java:24)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:511)
	at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:214)
	at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:199)
	at android.test.ActivityInstrumentationTestCase2.runTest(ActivityInstrumentationTestCase2.java:192)
	at org.mozilla.fennec.tests.BaseTest.runTest(BaseTest.java:129)
	at junit.framework.TestCase.runBare(TestCase.java:127)
	at junit.framework.TestResult$1.protect(TestResult.java:106)
	at junit.framework.TestResult.runProtected(TestResult.java:124)
	at junit.framework.TestResult.run(TestResult.java:109)
	at junit.framework.TestCase.run(TestCase.java:118)
	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:169)
	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:154)
	at android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:545)
	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1551)
17 INFO TEST-UNEXPECTED-FAIL | testMasterPassword | Exception caught - junit.framework.AssertionFailedError: Click can not be completed!
All of these seem to happen after the Watcher has been killed -- another case of bug 867360. See also bug 879089, another way that testMasterPassword can fail when the Watcher is not running.

I am a little concerned at the rate of incidence: I have not noticed this many Watcher-related failures in other tests. There seems to be something in this test that "tickles" the Watcher failure...or maybe this test is very sensitive to Watcher failures?

Hopefully we can get the Watcher fix deployed soon.
Assignee: nobody → gbrown
Depends on: 867360
I couldn’t reproduce this issue and the one from bug Bug878589 on my panda board but I've made some changes in the code that should minimize the possibility for them to appear.
On try server this patch looks green
https://tbpl.mozilla.org/?tree=Try&rev=9c9ed79cd32d
Attachment #758517 - Flags: review?(jmaher)
Comment on attachment 758517 [details] [diff] [review]
Intermittent testMasterPassword fix

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

::: mobile/android/base/tests/testMasterPassword.java.in
@@ +116,5 @@
>          mSolo.clickOnButton("OK");
>  
>          // Verify that the Master Password was disabled
> +        mSolo.searchText("Privacy & Security");
> +        mAsserter.ok(mSolo.waitForText("^Use master password$"), "Checking if Use master password is present", "Use master password is present");

I don't understand this.  Now we are searching for Privacy & Security, but we are still waiting for 'Use master password'

@@ +121,2 @@
>          mSolo.clickOnText("^Use master password$");
> +        mAsserter.ok(mSolo.waitForText("Create Master Password"), "Checking if the password is disabled", "The password is disabled");

I noticed you removed the ^ and $ from the regex.  Is there a reason for that?
Attachment #758517 - Flags: review?(jmaher) → review+
(In reply to Joel Maher (:jmaher) from comment #14)
> Comment on attachment 758517 [details] [diff] [review]
> Intermittent testMasterPassword fix
> 
> Review of attachment 758517 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mobile/android/base/tests/testMasterPassword.java.in
> @@ +116,5 @@
> >          mSolo.clickOnButton("OK");
> >  
> >          // Verify that the Master Password was disabled
> > +        mSolo.searchText("Privacy & Security");
> > +        mAsserter.ok(mSolo.waitForText("^Use master password$"), "Checking if Use master password is present", "Use master password is present");
> 
> I don't understand this.  Now we are searching for Privacy & Security, but
> we are still waiting for 'Use master password'

This will grantee that we are in the right section to access the Use master password option, also adds a waiting period witch helps for slow devices.
> 
> @@ +121,2 @@
> >          mSolo.clickOnText("^Use master password$");
> > +        mAsserter.ok(mSolo.waitForText("Create Master Password"), "Checking if the password is disabled", "The password is disabled");
> 
> I noticed you removed the ^ and $ from the regex.  Is there a reason for
> that?
 I had no particular reason for removing ^ and $ but this will not affect the patch since we don't need an precise search in this case.
Keywords: checkin-needed
This has conflicts with another fix that landed recently. Please rebase.
Keywords: checkin-needed
This should be ok now.
Attachment #758517 - Attachment is obsolete: true
Attachment #761422 - Flags: checkin?(ryanvm)
Comment on attachment 761422 [details] [diff] [review]
Intermittent testMasterPassword fix1

https://hg.mozilla.org/integration/mozilla-inbound/rev/a3d315019c47
Attachment #761422 - Flags: checkin?(ryanvm) → checkin+
https://hg.mozilla.org/mozilla-central/rev/a3d315019c47
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 24
(In reply to TinderboxPushlog Robot from comment #33)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
The logcat in Comment 33 has WATCHDOG KILLING SYSTEM PROCESS -- we need to resolve bug 883539 to solve this.
Depends on: 883539
Actually, the watchdog problem here happened after the test failure. 

Also, I do not see evidence of watcher failure (Comment 3).
No longer depends on: 883539
I still cannot think of an explanation for the failure in Comment 33. Thankfully it only happened once.
Assignee: gbrown → nobody
(In reply to TinderboxPushlog Robot from comment #54)

Watcher died:

07:24:47     INFO -  08-06 07:24:39.320 I/ActivityManager( 1403): No longer want com.mozilla.watcher (pid 1833): hidden #16
07:24:47     INFO -  08-06 07:24:39.328 W/ActivityManager( 1403): Scheduling restart of crashed service com.mozilla.watcher/.WatcherService in 5000ms
...
07:24:47     INFO -  08-06 07:24:41.914 W/InputDispatcher( 1403): Permission denied: injecting event from pid 4927 uid 10046 to window Window{41360b30 Keyguard paused=false} owned by uid 1000

Looks like the same cause for all the failures since 2013-08-02.

Deployment of the new watcher should fix this.
(In reply to Geoff Brown [:gbrown] from comment #55)
> Deployment of the new watcher should fix this.

Cool, thank you :-)
Depends on: 876715

        Attachment #761422 -
        Attachment is obsolete: true

        Attachment #789441 -
        Flags: review?

    
  

        Attachment #789441 -
        Flags: review? → review?(jmaher)

    
    

    
  
This patch updates the test using some new methods included in base-test. It should help reducing intermittent fails by better managing the waiting periods.
Looks good on try server.
https://tbpl.mozilla.org/?tree=Try&rev=d368e456278f&jobname=Android%204.0%20Panda%20try%20opt%20test%20robocop-2

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
  
Comment on attachment 789441 [details] [diff] [review]
MasterPassword.patch

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

> Looks good on try server.
> https://tbpl.mozilla.org/?tree=Try&rev=d368e456278f&jobname=Android%204.0%20Panda%20try%20opt%20test%20robocop-2

I can believe that it is no worse than current runs...but it is really hard to tell. I also note that some of the retries that initially look unrelated to testMasterPassword actually happened in testMasterPassword. eg. https://tbpl.mozilla.org/php/getParsedLog.php?id=26439339&tree=Try&full=1#error2

Showing improvement or even lack of regression in rc2 is really hard now. A technique that you might consider is to push twice to try: once with your patch and once without. Retrigger N times on each and compare successes/failures/retries.


Thanks for trying to improve the reliability of this test, but be aware that there may be factors beyond your control working against you! In particular, I think we need to get the new watcher deployed (since the existing one does not register as a foreground service correctly), and probably a new android image on the pandas (to avoid the deadlocks and system restarts).

::: mobile/android/base/tests/testMasterPassword.java.in
@@ +52,5 @@
>          mActions.sendKeys(password);
>          mSolo.clearEditText(1);
>          mSolo.clickOnEditText(1);
>          mActions.sendKeys(password);
> +        waitForEnabledText("^Cancel$");

I wrote waitForEnabledText to solve a specific problem in Settings (GeckoPreferences)...I did not really intend it as a waitForText replacement. It might be fine, but I'm not sure that it is as robust as waitForText. Also, be aware that waitForEnabledText waits for a maximum of 10 seconds; the waitForText default maximum is 20 seconds, I believe.

@@ +59,2 @@
>          mSolo.clickOnText("^Use master password$");
> +        mAsserter.ok(waitForText("^Create Master Password$"), "Checking if no password was set if the action was canceled", "No password was set");

Can you clarify why you use waitForText sometimes and waitForEnabledText others? I'm just curious.

@@ +66,5 @@
>          mActions.sendKeys(password);
>  
>          // Verify that the input characters are converted to dots automatically
>          mAsserter.ok(waitForText("."), "waiting to convert the letters in dots", "The letters are converted in dots");
> +        clickOnButton("OK");

What is the advantage of BaseTest.clickOnButton here?

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
  
Comment on attachment 789441 [details] [diff] [review]
MasterPassword.patch

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

please address :gbrown's comments, this patch looks safe, but I would like to know why we switched to waitForEnabledText and stopped using the mSolo versions of many methods.

        Attachment #789441 -
        Flags: review?(jmaher) → review-

    
    

    
  
These failures seem to stop around the time testMasterPassword.java.in was updated in bug 905591.

    
    

    
  
Closing inactive keywords:intermittent-failure bugs where the TBPLbot has previously commented and the test isn't marked as disabled; filter on orange-cleanup-201401.
Status: REOPENED → RESOLVED
Closed: 11 years ago → 10 years ago
Resolution: --- → WORKSFORME
Product: Firefox for Android → Firefox for Android Graveyard

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: