838596 - Robocop: Add test for 'Master Password' feature

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Paul Feher]

This bug tracks the Robocop tests part of 'Master Password' feature on Firefox for Android.
This patch tests the Master Password feature first by enabling the password, then testing it on a login page and finally disabling the password.
This patch integrates the following test cases:
Clear private data will not clear the saved Master Password
Characters typed in Master Password field(s) are converted to dots
Disable Master Password
Confirm a wrong Master Password while saving new credentials to Password Manager
Confirm the correct Master Password while saving new credentials to Password Manager
Set Master Password
Disable Master Password using a wrong password
Master Password popup is triggered when a password is remembered
https://moztrap.mozilla.org/manage/cases/?filter-suite=79&pagenumber=2

Comment 1

[Paul Feher]

Attachment: MasterPassword V1

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 710693 [details] [diff] [review]
MasterPassword V1

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

overall the indentation level needs a lot of work.  This fails on the tegras both for me locally and on try server:

FIRE PROC: '"MOZ_CRASHREPORTER=1,XPCOM_DEBUG_BREAK=stack,MOZ_HIDE_RESULTS_TABLE=1,MOZ_CRASHREPORTER_NO_REPORT=1,NO_EM_RESTART=1,MOZ_PROCESS_LOG=/tmp/tmpilWlJRpidlog,XPCOM_MEM_BLOAT_LOG=/tmp/tmp6Nzof5/runtests_leaks.log" am instrument -w -e deviceroot /mnt/sdcard/tests -e class org.mozilla.fennec_jmaher.tests.testMasterPassword org.mozilla.roboexample.test/org.mozilla.fennec_jmaher.FennecInstrumentationTestRunner'
Robocop derived process name: org.mozilla.fennec_jmaher
INFO | automation.py | Application pid: 0
0 INFO SimpleTest START
1 INFO TEST-START | testMasterPassword
2 INFO TEST-PASS | testMasterPassword | Verify if the OK button is inactive - The OK button is inactive until both fields are filled
3 INFO TEST-PASS | testMasterPassword | Verify if the OK button is inactive - The OK button is inactive until the Confirm password field is filled
4 INFO TEST-PASS | testMasterPassword | Verify if the OK button is inactive - The OK button is inactive until until both fields contain the same password
5 INFO TEST-PASS | testMasterPassword | Verify if the OK button is inactive - The OK button is inactive until the Password field is filled
6 INFO TEST-PASS | testMasterPassword | Checking if no password was set if the action was canceled - No password was set
7 INFO TEST-PASS | testMasterPassword | waiting to convert the letters in dots - The letters are converted in dots
8 INFO TEST-PASS | testMasterPassword | Checking if the password is enabled - The password is enabled
9 INFO TEST-PASS | testMasterPassword | Awesomebar URL typed properly - http://mochi.test:8888/tests/robocop/robocop_login.html should equal http://mochi.test:8888/tests/robocop/robocop_login.html
10 INFO TEST-PASS | testMasterPassword | Doorhanger notification is hidden - false should equal false
11 INFO TEST-PASS | testMasterPassword | Awesomebar URL typed properly - http://mochi.test:8888/tests/robocop/robocop_login.html should equal http://mochi.test:8888/tests/robocop/robocop_login.html
Exception caught during test!
junit.framework.AssertionFailedError: The text: OK is not found!
	at junit.framework.Assert.fail(Assert.java:47)
	at junit.framework.Assert.assertTrue(Assert.java:20)
	at com.jayway.android.robotium.solo.Clicker.clickOnText(Clicker.java:291)
	at com.jayway.android.robotium.solo.Solo.clickOnText(Solo.java:823)
	at org.mozilla.fennec_jmaher.tests.testMasterPassword.verifyLoginPage(testMasterPassword.java:171)
	at org.mozilla.fennec_jmaher.tests.testMasterPassword.testMasterPassword(testMasterPassword.java:24)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:521)
	at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:204)
	at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:194)
	at android.test.ActivityInstrumentationTestCase2.runTest(ActivityInstrumentationTestCase2.java:186)
	at org.mozilla.fennec_jmaher.tests.BaseTest.runTest(BaseTest.java:135)
	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:520)
	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1447)
12 INFO TEST-UNEXPECTED-FAIL | testMasterPassword | Exception caught - junit.framework.AssertionFailedError: The text: OK is not found!
13 INFO TEST-END | testMasterPassword | finished in 117508ms
14 INFO TEST-START | Shutdown
15 INFO Passed: 10
16 INFO Failed: 1
17 INFO Todo: 0
18 INFO SimpleTest FINISHED
INFO | automation.py | Application ran for: 0:02:01.520218

::: mobile/android/base/tests/testMasterPassword.java.in
@@ +5,5 @@
> +
> +/* This patch tests the Master Password feature first by enabling the password,
> +then testing it on a login page and finally disabling the password */
> +public class testMasterPassword extends PixelTest {
> +	Device dev;

nit: 4 spaces only.

@@ +22,5 @@
> +
> +		enableMasterPassword(password, badPassword);
> +		verifyLoginPage(password, badPassword);
> +		disableMasterPassword(password, badPassword);
> +    }

nit: 4 space indent please.

Comment 3

[Paul Feher]

Attachment: MasterPassword V2

I've made the necessary changes.

Comment 4

[Paul Feher]

Attachment: MasterPassword V2.1

clearing some spaces

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 712896 [details] [diff] [review]
MasterPassword V2.1

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

this is a comprehensive test case and it covers this feature very well.  We should be doing 4 space indentation, not 8.  

r- for it failing on the tegras.  I am not sure why as I tried a few things to get this to click.

::: mobile/android/base/tests/testMasterPassword.java.in
@@ +29,5 @@
> +
> +		// Look for the 'Settings' menu if this device/OS uses it
> +		selectMenuItem("Settings");
> +		mSolo.waitForText("^Use master password$");
> +		mSolo.clickOnText("^Use master password$");

this never happens on the tegras (android 2.3 tablet mode).  I have verified via a screenshot that we are opening up the settings menu and this text is visible.

@@ +165,5 @@
> +
> +		mSolo.waitForText("Enter Search or Address");// Wait for the page to load
> +		for (String item:option) {
> +	 		if (!item.equals("Not Now")) {
> +				if (!item.equals("Never")) {

this logic is confusing.  I would do 3 methods "Remember", "Never", "NotNow" and put the logic in there.  You could get rid of this crazy if block and for stuff and just do:
verifyPasswordRemember(LOGIN_URL, password, badPassword);
verifyPasswordNever(LOGIN_URL, password, badPassword);
verifyPasswordNotNow(LOGIN_URL, password, badPassword);

Comment 6

[Geoff Brown [:gbrown] (pto Apr 11-18)]

(In reply to Joel Maher (:jmaher) from comment #5)
> Comment on attachment 712896 [details] [diff] [review]
> MasterPassword V2.1
> 
> Review of attachment 712896 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> this is a comprehensive test case and it covers this feature very well.  We
> should be doing 4 space indentation, not 8.  
> 
> r- for it failing on the tegras.  I am not sure why as I tried a few things
> to get this to click.
> 
> ::: mobile/android/base/tests/testMasterPassword.java.in
> @@ +29,5 @@
> > +
> > +		// Look for the 'Settings' menu if this device/OS uses it
> > +		selectMenuItem("Settings");
> > +		mSolo.waitForText("^Use master password$");
> > +		mSolo.clickOnText("^Use master password$");
> 
> this never happens on the tegras (android 2.3 tablet mode).  I have verified
> via a screenshot that we are opening up the settings menu and this text is
> visible.

Some ideas:
 - maybe the ^ or $ is a problem? (maybe there is a leading or trailing space in the menu?)
 - maybe we need to click on the check box rather than the text? I know clicking on the text works on some phones, but maybe that's not universal.

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

I removed the ^ and the $ from the waitForText as well as another run on the clickOnText.  It didn't help.  Maybe the checkbox is the solution,

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 712896 [details] [diff] [review]
MasterPassword V2.1

I retested on try server with the latest bits of everthing, lots of green with many retriggers.

Comment 9

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 712896 [details] [diff] [review]
MasterPassword V2.1

oh, I was wrong.  This patch doesn't have the robocop.ini bits added to it, so when I pushed again with the test activated I see the failure I indicated originally.

Here is a link to the try server push:
https://tbpl.mozilla.org/?tree=Try&rev=fce593720fc3&noignore=1

Comment 10

[Geoff Brown [:gbrown] (pto Apr 11-18)]

The patch on bug 837318 -- landing today -- might help here.

Comment 11

[Joel Maher ( :jmaher ) (UTC -8)]

I tried another try job this morning and it appears to still be problematic:
https://tbpl.mozilla.org/?tree=Try&rev=1a1577667848&noignore=1

Comment 12

[Paul Feher]

Attachment: MasterPassword V3

I think this patch solves the problem, looks ok on try-server
https://tbpl.mozilla.org/?tree=Try&rev=b0ca8e8c2b43

Comment 13

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 749863 [details] [diff] [review]
MasterPassword V3

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

this patch looks good with one nit.  The try server run has to many oranges and blues to land this.  I want to make sure we get this small backlog of tests landed in the next week or two.

::: mobile/android/base/tests/testMasterPassword.java.in
@@ +70,5 @@
> +        mActions.sendKeys(password);
> +        mSolo.clickOnEditText(1);
> +        mActions.sendKeys(password);
> +
> +        // Verify that the imput caracters ar convered to dots automaticaly

nit: // Verify that the input characters are converted to dots automatically

Comment 14

[Paul Feher]

Attachment: MasterPassword V4

I made the necessary adjustments and I removed the by-pass section in the code cause by Bug871464 since this was fixed. Now the try server run looks good, much greener.
https://tbpl.mozilla.org/?tree=Try&rev=d087a57c1027

Comment 15

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 756475 [details] [diff] [review]
MasterPassword V4

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

try server looks a lot better, this patch in general looks good.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e9193d5774f4