Closed Bug 846296 Opened 8 years ago Closed 8 years ago

Robocop: Add test for 'Add Search Engine' feature

Categories

(Firefox for Android :: General, defect)

22 Branch
ARM
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 23

People

(Reporter: xti, Assigned: AdrianT)

References

Details

Attachments

(1 file, 2 obsolete files)

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

It will test if a search engine is correctly added through a test page with a form field via the context menu.

Test patch integrates the following testcase:
https://moztrap.mozilla.org/manage/case/1054/
Attached patch Add Seardh Engine test (obsolete) — Splinter Review
Try run: https://tbpl.mozilla.org/?tree=Try&rev=38533e4e0ce9

I don't have the results for the tryrun yet but the test runs without any issues locally on an LG Optimus 2X (Android 2.2) and the Samsung Galaxy Tab 2 (Android 4.1)
Attachment #741872 - Flags: review?(jmaher)
Assignee: nobody → adrian.tamas
Attached patch Add Seardh Engine test v2 (obsolete) — Splinter Review
Redone the patch to adres the panda fails because on pandas when the pop-up is triggred the vkb is opened and the buttons can't be clicked until the keyboard is closed. Added OS specific action for 4.x to cover the issue on pandas.

Try run: https://tbpl.mozilla.org/?tree=Try&rev=2d6d2fceee33
Attachment #741872 - Attachment is obsolete: true
Attachment #741872 - Flags: review?(jmaher)
Attachment #743048 - Flags: review?(jmaher)
Comment on attachment 743048 [details] [diff] [review]
Add Seardh Engine test v2

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

Do we need to add robocop_search.html to a makefile?

This is simple and pretty decent.  Can you link to a try server run so we can see this works well.

::: mobile/android/base/tests/testAddSearchEngine.java.in
@@ +20,5 @@
> +    }
> +
> +    public void testAddSearchEngine() {
> +        int height,width;
> +        final int initialNrOfSearchengines;

initialNumSearchEngines

@@ +45,5 @@
> +            mAsserter.dumpLog("Something went wrong and the context menu was not opened. Trying again");
> +            mSolo.clickLongOnScreen(width,height);
> +        }
> +        mAsserter.ok(waitForText("Add Search Engine"), "Waiting for the context menu to be opened", "The context menu was opened");
> +

we end up waiting for text twice here, I would like this to be a bit more efficient.

@@ +62,5 @@
> +        // Check that the number of search results has increased
> +        mAsserter.is(getNrOfSearchEngines() ,initialNrOfSearchengines + 1 , "The number of search results has increased");
> +    }
> +
> +    public int getNrOfSearchEngines() {

getNumSearchEngines()

@@ +82,5 @@
> +             } else {
> +                 searchEngineCount = 0;
> +             }
> +        }
> +        mAsserter.isnot(searchEngineCount, 0, "There should be search engines displayed when text is entered");

is there any chance that there could really be zero search engines?  If so, I would like to set the adapter==null case to -1 instead.
Attachment #743048 - Flags: review?(jmaher) → review+
Attachment #743048 - Attachment is obsolete: true
Attachment #743530 - Flags: review?(jmaher)
(In reply to Joel Maher (:jmaher) from comment #3)
> Comment on attachment 743048 [details] [diff] [review]
> Add Seardh Engine test v2
> 
> Review of attachment 743048 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Do we need to add robocop_search.html to a makefile?
> 

Any file with the .html extension added in a patch is added in the Robocop package and can be accessed after the patch is constructed via the standard getAbsoluteUrl method

> This is simple and pretty decent.  Can you link to a try server run so we
> can see this works well.
> 
The run for the old version can be found in Comment 2

> @@ +45,5 @@
> > +            mAsserter.dumpLog("Something went wrong and the context menu was not opened. Trying again");
> > +            mSolo.clickLongOnScreen(width,height);
> > +        }
> > +        mAsserter.ok(waitForText("Add Search Engine"), "Waiting for the context menu to be opened", "The context menu was opened");
> > +
> 
> we end up waiting for text twice here, I would like this to be a bit more
> efficient.

clickLongOnScreen does not always work as it should (known Robotium issue). We would end up waiting for text only if the first click did not work as expected and the wait would exit when the text is found. From what I have seen the second tap always works if the first has not worked as intended. I can't use searchText here since the context menu is not instantly opened.

> 
> @@ +82,5 @@
> > +             } else {
> > +                 searchEngineCount = 0;
> > +             }
> > +        }
> > +        mAsserter.isnot(searchEngineCount, 0, "There should be search engines displayed when text is entered");
> 
> is there any chance that there could really be zero search engines?  If so,
> I would like to set the adapter==null case to -1 instead.

We would always have at least 4 search engines - the 4 default - but I changed it -1 just to be on the safe side.

The try run is a pass but we still have a few "2400 seconds without output" errors and a few panda restarts. 
Tryrun for the previous patch: https://tbpl.mozilla.org/?tree=Try&rev=2d6d2fceee33.
Tryrun for this version (just some string renames but I added a new run to be sure of the results): https://tbpl.mozilla.org/?tree=Try&rev=f35f4a11b328
Comment on attachment 743530 [details] [diff] [review]
Add Seardh Engine test v2.1

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

with the much greener robocop tests, this is a good sign.
Attachment #743530 - Flags: review?(jmaher) → review+
Depends on: 869277
Adrian, can you look at this failure in bug 869277.
I will look into it as soon as possible
https://hg.mozilla.org/mozilla-central/rev/b24fa214730b
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 23
You need to log in before you can comment on or make changes to this bug.