926394 - [Robocop] Create a new test for Top Sites

Status: RESOLVED INCOMPLETE

(Firefox for Android Graveyard :: General, defect)

Description

[Adrian Tamas (:AdrianT)]

The Most Visited page test was removed at some point because of the new re-design of about:home. This bug will cover the progress on creating a new test to cover the the Top Sites Page.

Comment 1

[Adrian Tamas (:AdrianT)]

Attachment: topSitesPartialPatch.patch

Comment 2

[Paul Feher]

Attachment: topSites.patch

This test cover the the New Top Sites page by initially creating a browsing history and populating the list
- checks the Top Sites grid context-menu functionality(Open in new tab, Open in Private Tab, Pin/Unpin and Edit options)
- editing the Top Sites grid by adding/removing new entries and check how this affects the position of items in the grid
- checks that the tab state is kept after the edit action, and accesses a page from Top Sites grid

The patch looks good on Try:
https://tbpl.mozilla.org/?tree=Try&rev=ef433f331960

Comment 3

[Geoff Brown [:gbrown]]

Comment on attachment 8341576 [details] [diff] [review]
topSites.patch

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

r- for reflection. Otherwise this is looking good.

::: mobile/android/base/tests/DatabaseHelper.java
@@ +54,5 @@
> +        try {
> +            ContentResolver resolver = mActivity.getContentResolver();
> +            ClassLoader classLoader = mActivity.getClassLoader();
> +            Class browserDB = classLoader.loadClass("org.mozilla.gecko.db.BrowserDB");
> +            Method updateVisitedHistory = browserDB.getMethod("updateVisitedHistory", ContentResolver.class, String.class);

We are trying to not use reflection now. You should be able to call methods on BrowserDB.updateVisitedHistory(), just like addBookmark is called above, in addOrUpdateMobileBookmark.

::: mobile/android/base/tests/testTopSitesPage.java
@@ +18,5 @@
> +public class testTopSitesPage extends AboutHomeTest {
> +
> +    private static String BLANK_PAGE_01, BLANK_PAGE_02;
> +    private int TopSitesNum;
> +    private int GridPosition;

TopSitesNum and GridPosition can be local variables in testTopSitesPage() -- no need to make them member variables. (If you need member variables, name them mWhatever please.)

@@ +153,5 @@
> +    // Loads a site by tapping on the site view in the Top Sites Grid section
> +    protected void loadTopSite(String url) {
> +        View topSite = getGridViewTopSite(url);
> +        if (topSite != null) {
> +            mSolo.clickOnView(topSite);

Nice.

@@ +185,5 @@
> +        return null;
> +    }
> +    protected View getGridViewTopSite(String url) {
> +        View topSitesView = getTopSitesGridView();
> +        ViewGroup topSitesViewGroup = (ViewGroup) topSitesView;

What if getTopSitesGridView returns null? I prefer an explicit assertion rather than a NullPointerException.

@@ +191,5 @@
> +            View topSiteView = topSitesViewGroup.getChildAt(i);
> +            try {
> +                ClassLoader classLoader = getActivity().getClassLoader();
> +                Class topSiteItemView= classLoader.loadClass("org.mozilla.gecko.home.TopSitesGridItemView");
> +                Method getUrl = topSiteItemView.getMethod("getUrl", (Class[]) null);

Try not to use reflection here.

@@ +213,5 @@
> +            View topSiteView = topSitesViewGroup.getChildAt(i);
> +            try {
> +                ClassLoader classLoader = getActivity().getClassLoader();
> +                Class topSiteItemView= classLoader.loadClass("org.mozilla.gecko.home.TopSitesGridItemView");
> +                Method getUrl = topSiteItemView.getMethod("getUrl", (Class[]) null);

Try not to use reflection here.

@@ +216,5 @@
> +                Class topSiteItemView= classLoader.loadClass("org.mozilla.gecko.home.TopSitesGridItemView");
> +                Method getUrl = topSiteItemView.getMethod("getUrl", (Class[]) null);
> +                Object topSiteItem = topSiteView;
> +                String topSiteUrl = (String)getUrl.invoke(topSiteItem, (Object []) null);
> +                if (url.equals(topSiteUrl)) {

Most of this code is the same as getGridViewTopSite. Why not do something like:

View topSiteView = getGridViewTopSite(url);
if (topSiteView)
    return TopSitesGridItemView.isPinned(topSiteView);

?

Comment 4

[Paul Feher]

Attachment: topSites.patch V2

Made all the requested changes but now this is blocked by Bug 948913

Comment 5

[Geoff Brown [:gbrown]]

Comment on attachment 8345850 [details] [diff] [review]
topSites.patch V2

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

Thanks Paul. This looks good.

I am reluctant to r+ with bug 948913 outstanding though -- just r? me again when that is resolved.

::: mobile/android/base/tests/DatabaseHelper.java
@@ +60,5 @@
> +            date = (long) System.currentTimeMillis();
> +        }
> +            final ContentResolver resolver = mActivity.getContentResolver();
> +            BrowserDB.updateHistoryEntry(resolver, url, title, date, visits);
> +            mAsserter.ok(true, "Updating a new History item", "Updating the history item with the url = " + url);

nit - spacing is off here.

::: mobile/android/base/tests/robocop.ini
@@ +59,5 @@
>  [testSessionOOMRestore]
>  [testSettingsMenuItems]
>  # [testShareLink] # see bug 915897
>  [testSystemPages]
> +[testTopSitesPage]

oops - If you insert this here, the skip-if below will apply to testTopSitesPage rather than testSystemPages. Add [testTopSitesPage] 2 lines further down.

Comment 6

[Paul Feher]

Attachment: TopSitesPages.patch

Comment 7

[BMO Automation]

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.