Cristian Nicolae (:xti) Reporter
	Description • 11 years ago

This bug tracks the Robocop tests for the Show Search Suggestions feature on Firefox for Android.

The first one will test if the Show Search Suggestions preference is unchecked at start up in Settings Menu and then if the Show Suggestions notification is triggered in Awesomescreen. The notification is dismissed by tapping on the "Yes" button and the test will go back in Settings to check if the Show Search Suggestions preference is now checked. In the end, the test will verify if the Show Suggestions notification is not triggered again in Awesomescreen.

For the second test, there will be the same steps except the part where the notification is dismissed by tapping on the "No" button and the checkbox status according to this option.

These patches integrates the following testcases:
https://moztrap.mozilla.org/manage/case/2739/
https://moztrap.mozilla.org/manage/case/2740/

	Cristian Nicolae (:xti) Reporter
	Comment 1 • 11 years ago

Note: The following lines were changed since the test profile had the "Show Search Suggestions" pref enabled by default. A regular profile has it disabled at start-up.

524 options.extraPrefs.append('browser.search.suggest.enabled=false')
525 options.extraPrefs.append('browser.search.suggest.prompted=false')

	Geoff Brown [:gbrown]
	Comment 2 • 11 years ago

Comment on attachment 702373 [details] [diff] [review]
Search Suggestion opt-in prompt: Yes V1.0

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

::: mobile/android/base/tests/testBrowserSearchSuggestionsYes.java.in
@@ +32,5 @@
> +        // Verify how many checkboxes are visible on the screen
> +        mAsserter.ok(checkBoxes.size() > 0, "There are checkboxes displayed in the view", String.valueOf(checkBoxes.size()) + " are in view");
> +
> +        // Verify if "Show search suggestions" pref is unchecked       
> +        mAsserter.ok(!mSolo.isCheckBoxChecked(checkBoxes.size()-2), "Checkbox not checked", "Show search suggestions is not checked");

This seems like it might break easily: If a checkbox setting is removed, added, or re-arranged. Worse, if a setting is changed, there is a good chance that the test will continue to pass, even though it is examining the wrong checkbox!

@@ +44,5 @@
> +        mAsserter.ok(mSolo.searchText("Would you like to turn on Google search suggestions?"), "Notification is triggered", "Google search engine notification is triggered");
> +
> +        mSolo.clickOnText("Yes");
> +        mActions.sendSpecialKey(Actions.SpecialKey.BACK);
> +        mSolo.sleep(1000);

I would like to see a comment explaining why the pause is necessary.

::: testing/mochitest/runtestsremote.py
@@ +521,5 @@
>              fennec_ids = options.robocopIds
>          dm.pushFile(fennec_ids, os.path.join(deviceRoot, "fennec_ids.txt"))
>          options.extraPrefs.append('robocop.logfile="%s/robocop.log"' % deviceRoot)
> +        options.extraPrefs.append('browser.search.suggest.enabled=false')
> +        options.extraPrefs.append('browser.search.suggest.prompted=false')

Will this adversely affect testSearchSuggestions?

	Cristian Nicolae (:xti) Reporter
	Comment 4 • 11 years ago

(In reply to Geoff Brown [:gbrown] from comment #2)
> Comment on attachment 702373 [details] [diff] [review]
> Search Suggestion opt-in prompt: Yes V1.0
> 
> Review of attachment 702373 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This seems like it might break easily: If a checkbox setting is removed,
> added, or re-arranged. Worse, if a setting is changed, there is a good
> chance that the test will continue to pass, even though it is examining the
> wrong checkbox!

Indeed, if a checkbox is added/removed from Settings, then this line will be changed:
mAsserter.ok(!mSolo.isCheckBoxChecked(checkBoxes.size()-2), and there will be a checkBoxes.size()-x, according to the new changes.

Another way to do this would be to scroll down to the "Show search suggestions" pref and the code will be always: checkBoxes.size()-1, except for the 10" tablets (in portrait) where it will be always checkBoxes.size()-2 (when Settings menu is opened, this pref is already in view and also the next checkbox too, by default).

> > +        mSolo.sleep(1000);
> 
> I would like to see a comment explaining why the pause is necessary.

I noticed that sometimes mActions.sendSpecialKey(Actions.SpecialKey.MENU) (which follows after this pause) doesn't work immediately after the Awesomescreen is dismissed and it seems to work always with this pause here.

> ::: testing/mochitest/runtestsremote.py
> @@ +521,5 @@
> >              fennec_ids = options.robocopIds
> >          dm.pushFile(fennec_ids, os.path.join(deviceRoot, "fennec_ids.txt"))
> >          options.extraPrefs.append('robocop.logfile="%s/robocop.log"' % deviceRoot)
> > +        options.extraPrefs.append('browser.search.suggest.enabled=false')
> > +        options.extraPrefs.append('browser.search.suggest.prompted=false')
> 
> Will this adversely affect testSearchSuggestions?

The Search Suggestions notification is triggered only if this pref is disabled at start-up. Any other changes to this pref through the Settings Menu will result in a permanent disable of the Search Suggestions notification, so both tests will be invalid in this case.

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 5 • 11 years ago

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/tmpNHk1QSpidlog,XPCOM_MEM_BLOAT_LOG=/tmp/tmptgq4X7/runtests_leaks.log" am instrument -w -e deviceroot /mnt/sdcard/tests -e class org.mozilla.fennec_jmaher.tests.testBrowserSearchSuggestionsNo 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 | testBrowserSearchSuggestionsNo
2 INFO TEST-PASS | testBrowserSearchSuggestionsNo | Check for Show Search  Suggestions pref - Show search suggestions exists
3 INFO TEST-PASS | testBrowserSearchSuggestionsNo | There are checkboxes displayed in the view - 6 are in view
4 INFO TEST-PASS | testBrowserSearchSuggestionsNo | Checkbox not checked - Show search suggestions is not checked
5 INFO TEST-PASS | testBrowserSearchSuggestionsNo | Notification is triggered - Google search engine notification is triggered
Exception caught during test!
junit.framework.AssertionFailedError: The text: Settings 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.testBrowserSearchSuggestionsNo.testBrowserSearchSuggestionsNo(testBrowserSearchSuggestionsNo.java:53)
	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: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:520)
	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1447)
6 INFO TEST-UNEXPECTED-FAIL | testBrowserSearchSuggestionsNo | Exception caught - junit.framework.AssertionFailedError: The text: Settings is not found!
7 INFO TEST-END | testBrowserSearchSuggestionsNo | finished in 57172ms
8 INFO TEST-START | Shutdown
9 INFO Passed: 4
10 INFO Failed: 1
11 INFO Todo: 0
12 INFO SimpleTest FINISHED

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 6 • 11 years ago

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/tmpUCGLAtpidlog,XPCOM_MEM_BLOAT_LOG=/tmp/tmpMvzmkp/runtests_leaks.log" am instrument -w -e deviceroot /mnt/sdcard/tests -e class org.mozilla.fennec_jmaher.tests.testBrowserSearchSuggestionsYes 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 | testBrowserSearchSuggestionsYes
2 INFO TEST-PASS | testBrowserSearchSuggestionsYes | Check for Show Search  Suggestions pref - Show search suggestions exists
3 INFO TEST-PASS | testBrowserSearchSuggestionsYes | There are checkboxes displayed in the view - 6 are in view
4 INFO TEST-PASS | testBrowserSearchSuggestionsYes | Checkbox not checked - Show search suggestions is not checked
5 INFO TEST-UNEXPECTED-FAIL | testBrowserSearchSuggestionsYes | Notification is triggered - Google search engine notification is triggered
Exception caught during test!
junit.framework.AssertionFailedError: 5 INFO TEST-UNEXPECTED-FAIL | testBrowserSearchSuggestionsYes | Notification is triggered - Google search engine notification is triggered
	at junit.framework.Assert.fail(Assert.java:47)
	at org.mozilla.fennec_jmaher.FennecMochitestAssert._logMochitestResult(FennecMochitestAssert.java:107)
	at org.mozilla.fennec_jmaher.FennecMochitestAssert.ok(FennecMochitestAssert.java:136)
	at org.mozilla.fennec_jmaher.tests.testBrowserSearchSuggestionsYes.testBrowserSearchSuggestionsYes(testBrowserSearchSuggestionsYes.java:44)
	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: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:520)
	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1447)
6 INFO TEST-UNEXPECTED-FAIL | testBrowserSearchSuggestionsYes | Exception caught - junit.framework.AssertionFailedError: 5 INFO TEST-UNEXPECTED-FAIL | testBrowserSearchSuggestionsYes | Notification is triggered - Google search engine notification is triggered
7 INFO TEST-END | testBrowserSearchSuggestionsYes | finished in 48559ms
8 INFO TEST-START | Shutdown
9 INFO Passed: 3
10 INFO Failed: 2
11 INFO Todo: 0
12 INFO SimpleTest FINISHED

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 7 • 11 years ago

Comment on attachment 702373 [details] [diff] [review]
Search Suggestion opt-in prompt: Yes V1.0

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

::: mobile/android/base/tests/testBrowserSearchSuggestionsYes.java.in
@@ +31,5 @@
> +
> +        // Verify how many checkboxes are visible on the screen
> +        mAsserter.ok(checkBoxes.size() > 0, "There are checkboxes displayed in the view", String.valueOf(checkBoxes.size()) + " are in view");
> +
> +        // Verify if "Show search suggestions" pref is unchecked       

nit: whitespace after the above line.

@@ +44,5 @@
> +        mAsserter.ok(mSolo.searchText("Would you like to turn on Google search suggestions?"), "Notification is triggered", "Google search engine notification is triggered");
> +
> +        mSolo.clickOnText("Yes");
> +        mActions.sendSpecialKey(Actions.SpecialKey.BACK);
> +        mSolo.sleep(1000);

maybe a waitfortext instead of a sleep?

::: testing/mochitest/runtestsremote.py
@@ +521,5 @@
>              fennec_ids = options.robocopIds
>          dm.pushFile(fennec_ids, os.path.join(deviceRoot, "fennec_ids.txt"))
>          options.extraPrefs.append('robocop.logfile="%s/robocop.log"' % deviceRoot)
> +        options.extraPrefs.append('browser.search.suggest.enabled=false')
> +        options.extraPrefs.append('browser.search.suggest.prompted=false')

these should be turned off for good as we should not have any outside network access required.  This does appear to cause testSearchSuggestions to fail.  We should revisit testSearchSuggestions to see how valid it is.

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 8 • 11 years ago

Comment on attachment 702386 [details] [diff] [review]
Search Suggestion opt-in prompt: No V1.0

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

marking as r- since it fails and I would prefer to see this combined into one test case.

	Cristian Nicolae (:xti) Reporter
	Comment 10 • 11 years ago

testSearchSuggestions was modified due to the changes performed in runtestsremote.py and it's not failing anymore.

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 12 • 11 years ago

Comment on attachment 704475 [details] [diff] [review]
Search Suggestion opt-in prompt: Yes V2.0

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

this fails on android 2.3/tablet mode:
https://tbpl.mozilla.org/php/getParsedLog.php?id=19004095&tree=Try&full=1

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 15 • 11 years ago

Comment on attachment 706356 [details] [diff] [review]
Search Suggestion opt-in prompt: Yes V3.0

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

this passes on try server and locally!  congrats.  I am marking this a r- because of the long sleeps.  We need comments to justify why we have them, or find other solutions.

::: mobile/android/base/tests/testBrowserSearchSuggestionsYes.java.in
@@ +76,5 @@
> +            }
> +        }
> +
> +        // Verify if all Settings options are listed
> +        mAsserter.ok(true, "Settings List = ", options.toString());

this will never fail

@@ +79,5 @@
> +        // Verify if all Settings options are listed
> +        mAsserter.ok(true, "Settings List = ", options.toString());
> +
> +        // Verify if all checkboxes are listed
> +        mAsserter.ok(true, "CheckBox List = ", checkBoxText.toString());

this will never fail

@@ +126,5 @@
> +
> +    private ListView getSettingsList() {
> +
> +    selectMenuItem("Settings");
> +    mSolo.sleep(10000);

ouch, this is a long sleep.  For other settings menu work, I don't recall needing to do this.

::: mobile/android/base/tests/testSearchSuggestions.java.in
@@ +38,5 @@
> +        // Enable the "Show Search Suggestions" pref in Settings
> +        selectMenuItem("Settings");
> +        mSolo.searchText("Show search suggestions");
> +        mSolo.clickOnText("Show search suggestions");
> +        mSolo.sleep(1000);

can we do something else besides sleep?

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 16 • 11 years ago

Comment on attachment 706357 [details] [diff] [review]
Search Suggestion opt-in prompt: No V3.0

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

Can we share code between the Yes|No case here?  I think 99% of the code is the same.  A couple common routines would be very beneficial.  I understand why we need two test cases.

	Cristian Nicolae (:xti) Reporter
	Comment 19 • 11 years ago

(In reply to Joel Maher (:jmaher) from comment #16)
> Comment on attachment 706357 [details] [diff] [review]
> Search Suggestion opt-in prompt: No V3.0
> 
> Review of attachment 706357 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Can we share code between the Yes|No case here?  I think 99% of the code is
> the same.  A couple common routines would be very beneficial.  I understand
> why we need two test cases.

I moved the main common code to BaseTest. It should be fine now.

	Geoff Brown [:gbrown]
	Comment 20 • 11 years ago

Comment on attachment 709022 [details] [diff] [review]
Search Suggestion opt-in prompt: Yes V4.0

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

r- for the build failure.

::: mobile/android/base/tests/BaseTest.java.in
@@ +626,5 @@
> +        selectMenuItem("Settings");
> +        mSolo.waitForText("General");
> +        final ArrayList<ListView> views = mSolo.getCurrentListViews();
> +
> +        list = null;

Where is list declared? ** This does not compile for me. **

@@ +634,5 @@
> +                  list = view;
> +                  return true;
> +               }
> +               return false;
> +            }

Why is waitForTest used here? It looks like nothing can change for the duration of waitForTest. I would either move the getCurrentListViews() call inside test(), or remove the waitForTest().

@@ +644,5 @@
> +        ArrayList<String> checkBoxText = new ArrayList();
> +        View option, Opt;
> +        final ArrayList<CheckBox> checkBoxes =  new ArrayList();
> +        final ArrayList<String> options = new ArrayList();
> +        //final ArrayList<String> checkBoxText = new ArrayList();

delete the unused (commented out) line

::: mobile/android/base/tests/testBrowserSearchSuggestionsYes.java.in
@@ +23,5 @@
> +    }
> +
> +    public void testBrowserSearchSuggestionsYes() {
> +        int index;
> +        ArrayList<String> checkBoxText = new ArrayList();

nit: I think this would be easier to read if you changed "checkBoxText" -> "checkBoxTextList"

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 23 • 11 years ago

Comment on attachment 710191 [details] [diff] [review]
Search Suggestion opt-in prompt V4.1

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

a few small nits.  
this passes on the tegras in production, but the testSystemPages seems to fail a lot
r- for failing to pass on panda boards.
https://tbpl.mozilla.org/?tree=Try&rev=348fc13b7ba9&noignore=1


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/tmpvtF8Vopidlog,XPCOM_MEM_BLOAT_LOG=/mnt/sdcard/tests/profile/runtests_leaks.log" am instrument -w -e deviceroot /mnt/sdcard/tests -e class org.mozilla.fennec.tests.testBrowserSearchSuggestionsYes org.mozilla.roboexample.test/org.mozilla.fennec.FennecInstrumentationTestRunner'
Robocop derived process name: org.mozilla.fennec
INFO | automation.py | Application pid: 0
0 INFO SimpleTest START
1 INFO TEST-START | testBrowserSearchSuggestionsYes
2 INFO TEST-PASS | testBrowserSearchSuggestionsYes | Check for Show Search  Suggestions pref - Show search suggestions exists
3 INFO TEST-PASS | testBrowserSearchSuggestionsYes | Checkbox not checked - Show search suggestions is not checked
4 INFO TEST-PASS | testBrowserSearchSuggestionsYes | Notification is triggered - Google search engine notification is triggered
5 INFO TEST-UNEXPECTED-FAIL | testBrowserSearchSuggestionsYes | Checkbox checked - Show search suggestions is checked
Exception caught during test!
junit.framework.AssertionFailedError: 5 INFO TEST-UNEXPECTED-FAIL | testBrowserSearchSuggestionsYes | Checkbox checked - Show search suggestions is checked
	at junit.framework.Assert.fail(Assert.java:47)
	at org.mozilla.fennec.FennecMochitestAssert._logMochitestResult(FennecMochitestAssert.java:107)
	at org.mozilla.fennec.FennecMochitestAssert.ok(FennecMochitestAssert.java:136)
	at org.mozilla.fennec.tests.testBrowserSearchSuggestionsYes.testBrowserSearchSuggestionsYes(testBrowserSearchSuggestionsYes.java:62)
	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: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:545)
	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1551)
6 INFO TEST-UNEXPECTED-FAIL | testBrowserSearchSuggestionsYes | Exception caught - junit.framework.AssertionFailedError: 5 INFO TEST-UNEXPECTED-FAIL | testBrowserSearchSuggestionsYes | Checkbox checked - Show search suggestions is checked
7 INFO TEST-END | testBrowserSearchSuggestionsYes | finished in 23251ms
8 INFO TEST-START | Shutdown
9 INFO Passed: 3
10 INFO Failed: 2
11 INFO Todo: 0
12 INFO SimpleTest FINISHED
INFO | automation.py | Application ran for: 0:00:28.141330
INFO | automation.py | Reading PID log: /tmp/tmpvtF8Vopidlog
getting files in '/mnt/sdcard/tests/profile/minidumps/'
WARNING | automationutils.processLeakLog() | refcount logging is off, so leaks can't be detected!

::: mobile/android/base/tests/BaseTest.java.in
@@ +645,5 @@
>              }
>          }
>      }
> +
> +    public ListView getSettingsList() {      

nit: whitespace at the end here.

@@ +656,5 @@
> +        }
> +        return list;
> +    }
> +
> +    public ArrayList<String> getSettingsCheckBoxes() {

Can you add a comment above this as to why we would use this method or how it is used?

@@ +662,5 @@
> +        View option, Opt;
> +        final ArrayList<CheckBox> checkBoxes =  new ArrayList();
> +        final ArrayList<String> options = new ArrayList();
> +        ListView settingsList = getSettingsList();
> +        for (int i = 0; i < settingsList.getAdapter().getCount();i++) {

nit: space between ; and i++

@@ +668,5 @@
> +            if (settingsItem instanceof android.widget.LinearLayout) {
> +                ViewGroup settingsEntry = (ViewGroup)settingsItem;
> +                for (int j = 0; j < settingsEntry.getChildCount(); j++) {
> +                    View setting = settingsEntry.getChildAt(j);
> +                    if (setting instanceof android.widget.RelativeLayout){

nit: space between ){

::: mobile/android/base/tests/testSearchSuggestions.java.in
@@ +40,5 @@
> +        mSolo.searchText("Show search suggestions");
> +        mSolo.clickOnText("Show search suggestions");
> +
> +        /* Adding the following line as a delay to be sure that "Show search suggestion"
> +        checkbox is cheked */

nit: /cheked/checked/, also remove the extra newline in the comment

::: testing/mochitest/runtestsremote.py
@@ +527,5 @@
>              fennec_ids = options.robocopIds
>          dm.pushFile(fennec_ids, os.path.join(deviceRoot, "fennec_ids.txt"))
>          options.extraPrefs.append('robocop.logfile="%s/robocop.log"' % deviceRoot)
> +        options.extraPrefs.append('browser.search.suggest.enabled=false')
> +        options.extraPrefs.append('browser.search.suggest.prompted=false')

nit: fix the indentation here.

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 24 • 11 years ago

Comment on attachment 710193 [details] [diff] [review]
Search Suggestion opt-in prompt Yes & No V4.1

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

just a couple nits.

::: mobile/android/base/tests/testBrowserSearchSuggestionsNo.java.in
@@ +30,5 @@
> +
> +        checkBoxTextList = getSettingsCheckBoxes();
> +
> +        // Verify if the "Show Search Suggestion" pref exists
> +        mAsserter.ok(mSolo.searchText("Show search suggestions"), "Check for Show Search  Suggestions pref", "Show search suggestions exists");

nit: extra white space between 'Search  Suggestions'.  Also please keep it lowercase except for the first word.

@@ +54,5 @@
> +        mActions.sendSpecialKey(Actions.SpecialKey.BACK);
> +        mSolo.waitForText("Enter Search or Address");
> +
> +        /* Go back to Settings and verify if "Show search suggestions" pref is still
> +        unchecked */

nit: make the comment fit on one line.

::: mobile/android/base/tests/testBrowserSearchSuggestionsYes.java.in
@@ +30,5 @@
> +
> +        checkBoxTextList = getSettingsCheckBoxes();
> +
> +        // Verify if the "Show Search Suggestion" pref exists
> +        mAsserter.ok(mSolo.searchText("Show search suggestions"), "Check for Show Search  Suggestions pref", "Show search suggestions exists");

nit: extra white space between 'Search  Suggestions'.  Also please keep it lowercase except for the first word.

	Cristian Nicolae (:xti) Reporter
	Comment 27 • 11 years ago

(In reply to Joel Maher (:jmaher) from comment #23)
> Comment on attachment 710191 [details] [diff] [review]
> Search Suggestion opt-in prompt V4.1
> 
> Review of attachment 710191 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> a few small nits.  
> this passes on the tegras in production, but the testSystemPages seems to
> fail a lot
> r- for failing to pass on panda boards.
> https://tbpl.mozilla.org/?tree=Try&rev=348fc13b7ba9&noignore=1
> 
These patches shouldn't affect the System Pages test.

> ::: testing/mochitest/runtestsremote.py
> @@ +527,5 @@
> >              fennec_ids = options.robocopIds
> >          dm.pushFile(fennec_ids, os.path.join(deviceRoot, "fennec_ids.txt"))
> >          options.extraPrefs.append('robocop.logfile="%s/robocop.log"' % deviceRoot)
> > +        options.extraPrefs.append('browser.search.suggest.enabled=false')
> > +        options.extraPrefs.append('browser.search.suggest.prompted=false')
> 
> nit: fix the indentation here.

I tried to fix it and I've got this error:

  File "_tests/testing/mochitest/runtestsremote.py", line 527
    fennec_ids = options.robocopIds
             ^
IndentationError: expected an indented block

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 28 • 11 years ago

Comment on attachment 710684 [details] [diff] [review]
Search Suggestion opt-in prompt: No V4.2

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

this works great on the tegras, but is failing on the panda boards:
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/tmpO8ZHKGpidlog,XPCOM_MEM_BLOAT_LOG=/tmp/tmpDRIE51/runtests_leaks.log" am instrument -w -e deviceroot /mnt/sdcard/tests -e class org.mozilla.fennec_jmaher.tests.testBrowserSearchSuggestionsNo 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 | testBrowserSearchSuggestionsNo
2 INFO TEST-PASS | testBrowserSearchSuggestionsNo | Check for Show search suggestions pref - Show search suggestions exists
3 INFO TEST-UNEXPECTED-FAIL | testBrowserSearchSuggestionsNo | Checkbox not checked - Show search suggestions is not checked
Exception caught during test!
junit.framework.AssertionFailedError: 3 INFO TEST-UNEXPECTED-FAIL | testBrowserSearchSuggestionsNo | Checkbox not checked - Show search suggestions is not checked
	at junit.framework.Assert.fail(Assert.java:47)
	at org.mozilla.fennec_jmaher.FennecMochitestAssert._logMochitestResult(FennecMochitestAssert.java:107)
	at org.mozilla.fennec_jmaher.FennecMochitestAssert.ok(FennecMochitestAssert.java:136)
	at org.mozilla.fennec_jmaher.tests.testBrowserSearchSuggestionsNo.testBrowserSearchSuggestionsNo(testBrowserSearchSuggestionsNo.java:41)
	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_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:545)
	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1551)
4 INFO TEST-UNEXPECTED-FAIL | testBrowserSearchSuggestionsNo | Exception caught - junit.framework.AssertionFailedError: 3 INFO TEST-UNEXPECTED-FAIL | testBrowserSearchSuggestionsNo | Checkbox not checked - Show search suggestions is not checked
5 INFO TEST-END | testBrowserSearchSuggestionsNo | finished in 9367ms
6 INFO TEST-START | Shutdown
7 INFO Passed: 1
8 INFO Failed: 2
9 INFO Todo: 0
10 INFO SimpleTest FINISHED
INFO | automation.py | Application ran for: 0:00:16.312130


the panda boards run ICS 4.0.4 in landscape mode.  I will have to do some screenshots to look at what is happening.

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 29 • 11 years ago

Comment on attachment 710683 [details] [diff] [review]
Search Suggestion opt-in prompt: Yes V4.2

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

this passes great on the tegras, but fails on the panda (ICS 4.0.4):
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/tmp_rGX7Jpidlog,XPCOM_MEM_BLOAT_LOG=/tmp/tmpQmx5op/runtests_leaks.log" am instrument -w -e deviceroot /mnt/sdcard/tests -e class org.mozilla.fennec_jmaher.tests.testBrowserSearchSuggestionsYes 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 | testBrowserSearchSuggestionsYes
2 INFO TEST-PASS | testBrowserSearchSuggestionsYes | Check for Show search suggestions pref - Show search suggestions exists
3 INFO TEST-UNEXPECTED-FAIL | testBrowserSearchSuggestionsYes | Checkbox not checked - Show search suggestions is not checked
Exception caught during test!
junit.framework.AssertionFailedError: 3 INFO TEST-UNEXPECTED-FAIL | testBrowserSearchSuggestionsYes | Checkbox not checked - Show search suggestions is not checked
	at junit.framework.Assert.fail(Assert.java:47)
	at org.mozilla.fennec_jmaher.FennecMochitestAssert._logMochitestResult(FennecMochitestAssert.java:107)
	at org.mozilla.fennec_jmaher.FennecMochitestAssert.ok(FennecMochitestAssert.java:136)
	at org.mozilla.fennec_jmaher.tests.testBrowserSearchSuggestionsYes.testBrowserSearchSuggestionsYes(testBrowserSearchSuggestionsYes.java:41)
	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_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:545)
	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1551)
4 INFO TEST-UNEXPECTED-FAIL | testBrowserSearchSuggestionsYes | Exception caught - junit.framework.AssertionFailedError: 3 INFO TEST-UNEXPECTED-FAIL | testBrowserSearchSuggestionsYes | Checkbox not checked - Show search suggestions is not checked
5 INFO TEST-END | testBrowserSearchSuggestionsYes | finished in 10043ms
6 INFO TEST-START | Shutdown
7 INFO Passed: 1
8 INFO Failed: 2
9 INFO Todo: 0
10 INFO SimpleTest FINISHED
INFO | automation.py | Application ran for: 0:00:17.657962

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 30 • 11 years ago

I believe the failure is that the "Show Search Suggestions" needs to be panned into view?  I don't see it on a screenshot.

	Cristian Nicolae (:xti) Reporter
	Comment 33 • 11 years ago

(In reply to Joel Maher (:jmaher) from comment #30)
> I believe the failure is that the "Show Search Suggestions" needs to be
> panned into view?  I don't see it on a screenshot.

I hope that this time will work fine. I added mSolo.searchForText when I try to verify the checkbox status for the 2nd time.

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 35 • 11 years ago

Comment on attachment 712380 [details] [diff] [review]
Search Suggestion opt-in prompt: Yes V4.3

this fails in the same way it did last time.  I would recommend getting try server access and going that route.

	Cristian Nicolae (:xti) Reporter
	Comment 36 • 11 years ago

These tests seems to fail on the Panda board and Samsung Galaxy Tab2 7" - both on ICS. However, the tests never fails on the tablet if I place it in portrait.

I've checked if all existing checkboxes are indexed in landscape mode and I got the same results as for portrait. Also on the Panda board I noticed in the screenshots that the Show search suggestions pref is uncheck, but for some reasons the results point out that the pref is checked.

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 39 • 11 years ago

Comment on attachment 719962 [details] [diff] [review]
Search Suggestion opt-in prompt: Yes V4.4

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

this fails on the tegra platform:
https://tbpl.mozilla.org/?tree=Try&rev=d7e4964d6a3d&noignore=1
also the testSearchSuggestionsNo patch is missing a } at the end of the file.

	Cristian Nicolae (:xti) Reporter
	Comment 40 • 11 years ago

I hope that this time will work fine both on tegra and panda boards.

	Adrian Tamas (:AdrianT)
	Comment 42 • 11 years ago

Try server run: https://tbpl.mozilla.org/?tree=Try&rev=34c157b12f00

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 43 • 11 years ago

Comment on attachment 730612 [details] [diff] [review]
Search Suggestion opt-in prompt: Yes V4.5

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

in general the code is good, the tests are failing both on tegra (unexpected-pass) and on the panda board (unexpected-fail)

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 44 • 11 years ago

Comment on attachment 730613 [details] [diff] [review]
Search Suggestion opt-in prompt: No V4.5

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

in general the code is good, the tests are failing both on tegra (unexpected-pass) and on the panda board (unexpected-fail)

	Geoff Brown [:gbrown]
	Comment 45 • 11 years ago

Comment on attachment 730612 [details] [diff] [review]
Search Suggestion opt-in prompt: Yes V4.5

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

::: mobile/android/base/tests/BaseTest.java.in
@@ +51,5 @@
>      protected String mBaseUrl;
>      protected String mRawBaseUrl;
>      private String mLogFile;
>      protected String mProfile;
> +    private ListView list;

I think this need not be a member variable.

Use mList (mSettingsList?) if a member variable is needed.

@@ +762,5 @@
>      }
> +
> +    public ListView getSettingsList() {
> +        selectMenuItem("Settings");
> +        mSolo.waitForText("General");

Use waitForText (from BaseTest) or check the return value from mSolo.waitForText.

::: mobile/android/base/tests/testBrowserSearchSuggestionsYes.java.in
@@ +52,5 @@
> +        mAsserter.ok(mSolo.searchText("Would you like to turn on Google search suggestions?"), "Notification is triggered", "Google search engine notification is triggered");
> +
> +        mSolo.clickOnText("Yes");
> +        mActions.sendSpecialKey(Actions.SpecialKey.BACK);
> +        mSolo.waitForText("Enter Search or Address");

waitForText or check return from mSolo.waitForText

@@ +67,5 @@
> +            }
> +        }
> +
> +        mActions.sendSpecialKey(Actions.SpecialKey.BACK);
> +        mSolo.waitForText("Enter Search or Address");

waitForText or check return from mSolo.waitForText

::: mobile/android/base/tests/testSearchSuggestions.java.in
@@ +53,5 @@
> +            }
> +        }
> +
> +        mActions.sendSpecialKey(Actions.SpecialKey.BACK);
> +        mSolo.waitForText("Enter Search or Address");

waitForText or check return from mSolo.waitForText

	Adrian Tamas (:AdrianT)
	Comment 46 • 11 years ago

Due to the issue with verifying if the checkbox is checked (bug 854872) and the instability of the isCheckBoxChecked method (sometimes works and sometimes not on ICS) I added an if to verify if the option was checked. It it is ok then it will be a PASS else it would be a KNOWN-FAIL. This also solves the issue without having to run checks for the device OS since on Tegra isCheckBoxChecked works as expected

Tryserver run: https://tbpl.mozilla.org/?tree=Try&rev=0c7ee4a48bc6

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 47 • 11 years ago

Comment on attachment 741223 [details] [diff] [review]
Show Search Suggestions

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

please get this up on try and retrigger the job that runs this test case (I assume rc2) about 10 times to look for consistency.

::: mobile/android/base/tests/testSearchSuggestions.java.in
@@ +42,5 @@
> +        mSolo.searchText("Show search suggestions");
> +        mSolo.clickOnText("Show search suggestions");
> +
> +        mActions.sendSpecialKey(Actions.SpecialKey.BACK);
> +        mSolo.waitForText("Enter Search or Address");

can we use waitForText from basetest here?

	Adrian Tamas (:AdrianT)
	Comment 48 • 11 years ago

Sorry I missed the waitForText there...

New try run: https://tbpl.mozilla.org/?tree=Try&rev=13cacf1b5704

	Adrian Tamas (:AdrianT)
	Comment 49 • 11 years ago

Changed the code in testSearchSuggestion since in some rare situations the search suggestions options was not checked when clicking on the option text. Now I enabled search suggestions at the beginning of the test by tapping on the "yes" button for the search suggestion dialog.

Tryrun: https://tbpl.mozilla.org/?tree=Try&rev=35817dbcbf7c. It looks good now on tegra but unfortunately on pandas there is still the "2400 seconds without output" issue. We should wait for the issue to be fixed gbrown has identified the cause and do a retest then.

	Adrian Tamas (:AdrianT)
	Comment 50 • 11 years ago

Tryserver run: https://tbpl.mozilla.org/?tree=Try&rev=74a3a5322550

Made the necessary changes to the test for pandas since we have the new settings menu UI on tablets.

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 51 • 11 years ago

Comment on attachment 749145 [details] [diff] [review]
Show Search Suggestions v3

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

this patch looks good.  The try server run doesn't look good right now, although those failures are not from this, we need to clean up our existing failures before adding new tests.

	Adrian Tamas (:AdrianT)
	Comment 52 • 11 years ago

Retry of the tryrun looks green. Unfortunatlt I am seeing a few restarts. Should we integrate or wait for the restart bugs to be fixed?

Tryrun: https://tbpl.mozilla.org/?tree=Try&rev=d4cf01d2b0dc

Also this run was before testImportFromAndroid was integrated. Should we retest and check that the integration does not influence the number of restarts?

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 53 • 11 years ago

I would like to see a try run after the testImportFromAndroid.  Just looking at that the overall failure rate looked too high, but much better.  I would need to see if the errors were installation/setup prior to the test or not, I will do that on the next try run!

	Adrian Tamas (:AdrianT)
	Comment 54 • 11 years ago

Try run after testImportFromAndroid was integrated: https://tbpl.mozilla.org/?tree=Try&rev=4d7f882df2a6. Only seeing a few restarts, a few known fails and a fail in testImportFromAndroid which has not happened before.

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 55 • 11 years ago

that try run looks a little too noisy for my taste.  I suspect a little bit of cleanup on other tests and maybe this will look nice and normal late next week.

	Adrian Tamas (:AdrianT)
	Comment 56 • 11 years ago

Added a method to get info about the search engines from the SearchEngine:Data event data. Not critical for this test but may be used in others like for example a rewrite of the AddSearchEngine test.

Tryrun: https://tbpl.mozilla.org/?tree=Try&rev=fb92b9581dad

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 57 • 11 years ago

Comment on attachment 761942 [details] [diff] [review]
Show Search Suggestions v4

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

This is still too noisy on try server.  I looked a lot of the blue runs and they happen inside of the test run itself, not in the setup/cleanup.

	Adrian Tamas (:AdrianT)
	Comment 58 • 11 years ago

Leaving this as it is until bug 879979 and bug 862793 are fixed. I don't know yet how the search suggestion will tie in with the new about:home and it would not make sense to continue work on this and then just remove the test later if it does not apply anymore

	Sylvestre Ledru [:Sylvestre]
	Comment 59 • 6 years ago

No assignee, updating the status.

	Sylvestre Ledru [:Sylvestre]
	Comment 60 • 6 years ago

No assignee, updating the status.

	BMO Automation
	Comment 61 • 3 years ago

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.