899183 - [fig] Re-implement robocop testBookmarklets.java.in

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Mark Capella [:capella]]

Bug 896576 removed the getBookmarksList() logic from BaseTest.java.in and disabled this test (which depends on it).

This needs to be re-implemented when the new Testing |utility class to get things from about:home| concept is finalized.

Comment 1

[Lucas Rocha (:lucasr)]

Attachment: Update testBookmarklets for new about:home

Comment 2

[:Margaret Leibovic]

Comment on attachment 789598 [details] [diff] [review]
Update testBookmarklets for new about:home

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

r+ with comments addressed.

::: mobile/android/base/tests/testBookmarklets.java.in
@@ +39,5 @@
>          // add the bookmarklet to the database. there's currently no way to
>          // add this using the UI, so we go through the content provider.
>          addOrUpdateMobileBookmark(title, js);
>  
> +        // Test the share popup in the Bookmarks page

This comment looks wrong.

@@ +49,5 @@
> +        int width = mDriver.getGeckoWidth() / 2;
> +        int y = mDriver.getGeckoHeight() / 2;
> +
> +        // Scroll down so that the bookmarks list has more items on screen.
> +        mActions.drag(width, width, mDriver.getGeckoHeight() - 10, mDriver.getGeckoHeight() / 2);

Reviewing this patch actually prompted me to revisit and add a new comment to the patch in bug 897481. Same comment applies here.

And since this is copy/pasted, maybe it should be pulled into a method on AboutHomeTest.

@@ +56,1 @@
>          Boolean found = false;

Any reason that this is a Boolean instead of a boolean?

@@ +56,4 @@
>          Boolean found = false;
> +        for (int i = bookmarks.getHeaderViewsCount(); i < bookmarks.getAdapter().getCount(); i++) {
> +            Cursor c = (Cursor)bookmarks.getItemAtPosition(i);
> +            String turl = c.getString(c.getColumnIndexOrThrow("url"));

Can we come up with a better variable name than turl?

Comment 3

[Lucas Rocha (:lucasr)]

(In reply to :Margaret Leibovic from comment #2)
> Comment on attachment 789598 [details] [diff] [review]
> Update testBookmarklets for new about:home
> 
> Review of attachment 789598 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r+ with comments addressed.
> 
> ::: mobile/android/base/tests/testBookmarklets.java.in
> @@ +39,5 @@
> >          // add the bookmarklet to the database. there's currently no way to
> >          // add this using the UI, so we go through the content provider.
> >          addOrUpdateMobileBookmark(title, js);
> >  
> > +        // Test the share popup in the Bookmarks page
> 
> This comment looks wrong.

Fixed.
 
> @@ +49,5 @@
> > +        int width = mDriver.getGeckoWidth() / 2;
> > +        int y = mDriver.getGeckoHeight() / 2;
> > +
> > +        // Scroll down so that the bookmarks list has more items on screen.
> > +        mActions.drag(width, width, mDriver.getGeckoHeight() - 10, mDriver.getGeckoHeight() / 2);
> 
> Reviewing this patch actually prompted me to revisit and add a new comment
> to the patch in bug 897481. Same comment applies here.
> 
> And since this is copy/pasted, maybe it should be pulled into a method on
> AboutHomeTest.

Fixed.

> @@ +56,1 @@
> >          Boolean found = false;
> 
> Any reason that this is a Boolean instead of a boolean?

Nope, fixed.
 
> @@ +56,4 @@
> >          Boolean found = false;
> > +        for (int i = bookmarks.getHeaderViewsCount(); i < bookmarks.getAdapter().getCount(); i++) {
> > +            Cursor c = (Cursor)bookmarks.getItemAtPosition(i);
> > +            String turl = c.getString(c.getColumnIndexOrThrow("url"));
> 
> Can we come up with a better variable name than turl?

aUrl? Done.

Comment 4

[Lucas Rocha (:lucasr)]

All green: https://tbpl.mozilla.org/?tree=Try&rev=b08ff84d93f9
Pushed: https://hg.mozilla.org/integration/fx-team/rev/7848c8ff7608

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7848c8ff7608