Closed Bug 716937 Opened 8 years ago Closed 8 years ago

Robotium testBookmark is failing

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 12

People

(Reporter: kats, Assigned: gbrown)

Details

(Whiteboard: not-fennec-11)

Attachments

(2 files)

After running the tests on a recent m-c, testBookmark and testNewTab are failing for me. The testBookmark test is wrong, so that's an easy fix. Still looking into testNewTab.
Attachment #587357 - Flags: review?(jmaher)
> +    protected final Activity openAwesomeBar() {
> +        Element awesomebar = mDriver.findElement(mActivity, "awesome_bar");
> +        return getActivityFromClick(awesomebar);
>     }

I find "openAwesomeBar" a little awkward; can we name it something like getActivityFromAwesomeBarClick or perhaps clickOnAwesomeBar?
clickOnAwesomeBar is fine with me.
Comment on attachment 587357 [details] [diff] [review]
Fix testBookmark to open the awesome bar without verifying url

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

I could go with a name change on openAwesomebar.
Attachment #587357 - Flags: review?(jmaher) → review+
Ok, I'll fix that when I land it.
https://hg.mozilla.org/mozilla-central/rev/0fdae7529dfb
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 12
The landed patch may have improved behavior, but I am still seeing problems in testBookmark, notably a hang after the History tab is selected.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee: bugmail.mozilla → gbrown
After the History tab is selected, the attempt to select a history item often fails, because the history list has not been populated yet. 

A short pause between selecting the History tab and selecting a History item fixes the problem...but I am reluctant to propose that as a solution.

I would like to wait for a Gecko event here, but there appears to be no such event associated with populating the history list.
Using a single fixed pause might be wrong, but creating a util the waits for a UI situation would work fine. We can't create events for every darn thing in the world

In this case, I would suggest a waitFor(testFn, callbackFn) pattern. The waitFor would call the testFn every 50ms until either it returned true or 2 seconds elapsed. If the test did not timeout, we'd call the callbackFn to continue the tests. In JS we did something like:

 waitFor(function() { return list.getItem(index) != null }, continueWithNextStep)

I see robotium already uses the waitForXxx idea a lot. Can we make something like this:
 waitFor(new Action.onWaitListener() { return list.getCount() > 0 });

?
mfinkle - thanks for the tip! That's essentially what I'm doing here: waiting for confirmation that there's something in the list before proceeding. I'm leveraging Robotium functionality (clickInList) that takes care of some of the waiting / looping. We still need to loop over clickInList attempts, but success comes in just a few attempts. 

Tested on Galaxy S and Galaxy Tab.
Attachment #589257 - Flags: review?(jmaher)
Comment on attachment 589257 [details] [diff] [review]
make testBookmark more reliable: wait for history list before selecting

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

thanks, this looks a lot more robust!

::: mobile/android/base/tests/testBookmark.java.in
@@ +29,1 @@
>  

this changes from testing the keys to testing a tap on the screen.  Most people navigate via taps on android vs keys, so this is safe to change IMO.  If we are changing that, do we want to change the two keys for RIGHT to tap on the history tab?  Maybe something to consider as a followup bug.
Attachment #589257 - Flags: review?(jmaher) → review+
(In reply to Joel Maher (:jmaher) from comment #11)

> this changes from testing the keys to testing a tap on the screen.  Most
> people navigate via taps on android vs keys, so this is safe to change IMO. 
> If we are changing that, do we want to change the two keys for RIGHT to tap
> on the history tab?  Maybe something to consider as a followup bug.

Good point. I wanted to restrict this change to the part that was failing (DOWN, DOWN, ENTER). I'll file a separate bug for the RIGHT, RIGHT.
Keywords: checkin-needed
nice.  We should put an easter egg in for UP, UP, DOWN, DOWN, LEFT, RIGHT, LEFT, RIGHT, B, A, ENTER, ENTER :)
(In reply to Joel Maher (:jmaher) from comment #13)
> nice.  We should put an easter egg in for UP, UP, DOWN, DOWN, LEFT, RIGHT,
> LEFT, RIGHT, B, A, ENTER, ENTER :)

That would be awesome.
https://hg.mozilla.org/mozilla-central/rev/6553d699ca34
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Whiteboard: not-fennec-11
You need to log in before you can comment on or make changes to this bug.