899182 - [fig] Re-implement robocop testBookmark.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

[Adrian Tamas (:AdrianT)]

Attachment: testBookmark rewrite

Added a few new methods in AboutHomeTest to get bookmarks/top bookmarks and to check if bookmarks/top bookmarks are displayed. Not all are used in testBookmarks, but they will be needed in testBookmarksTab. The only issue I had was that getting the view for the bookmark in getDisplayedBookmark(String url) with getView(i,null, bookmarksTabList) caused the clickOnView and longClickOnView not to work for some reason. I get the view with getChildAt but I make sure the view is scrolled to the bottom first so this should not create issues.

Added the new waitForAboutHomeTab method to wait for the tab to be rendered when changing tabs. waitForEnabledText no longer works reliably for me locally on the tablets with the recent changes in about:home

BaseTest.toggleBookmark does not work for me on tablets. Also there were some changes to the Navigation.Bookmark() method at some point which led to it not working anymore. Corrected the device/os verification and now it works.

I re-wrote the testBookmark test, covering the original tests and also adding tests for the Top Bookmarks section.

Comment 2

[Adrian Tamas (:AdrianT)]

Attachment: testBookmark fix v2

removed the openAboutHomeTab fix which was moved to bug 906662

Comment 3

[Lucas Rocha (:lucasr)]

Comment on attachment 792200 [details] [diff] [review]
testBookmark fix v2

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

Needs a rebase to fetch lists by tag instead of using reflection.

Comment 4

[Adrian Tamas (:AdrianT)]

Attachment: testBookmark.patch v3

Added tags to views in bug 916107, rewrote all methods to work with the tags and only left the getTopBookmark to use reflections since I need the getURl method. Also changed the Navigation.Bookmark method which did not seem to work on pandas because it has the 7"tablet UI which has the bookmark button in the menu and not on the UI.

Comment 5

[Adrian Tamas (:AdrianT)]

Attachment: testBookmark.patch v4

Changes the patch to not rely on a tagged Top Bookmarks view and also to work with the recently integrated DatabaseHelper and StringHelper classes

Comment 6

[Adrian Tamas (:AdrianT)]

Attachment: testBookmark.patch v4.1

The try run for the last version had some intermittent fails because the bookmarks page content was not rendered in time. Added a wait until the bookmark page content is displayed and pushed a new tryrun: https://tbpl.mozilla.org/?tree=Try&rev=3ba54df36ad2

Comment 7

[Adrian Tamas (:AdrianT)]

Comment on attachment 805345 [details] [diff] [review]
testBookmark.patch v4.1

removing the review flag per the chat on irc since the patch will be modified to remove the tests for Top Bookmarks

Comment 8

[Adrian Tamas (:AdrianT)]

Attachment: testBookmark.patch

Removed the top bookmarks part of the test because of the about:home changes. This works locally but a tryrun is pointless now until bug 919516 is fixed

Comment 9

[Lucas Rocha (:lucasr)]

Comment on attachment 808579 [details] [diff] [review]
testBookmark.patch

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

Looks good.

::: mobile/android/base/tests/AboutHomeTest.java.in
@@ +102,5 @@
> +        ListAdapter adapter = bookmarksTabList.getAdapter();
> +        if (adapter != null) {
> +            for (int i = 0; i < adapter.getCount(); i++ ) {
> +                // I am unable to click the view taken with getView for some reason so getting the child at i
> +                bookmarksTabList.smoothScrollToPosition(i);

This is likely to cause intermittent due to timing issues from the smooth scrolling, no? Not against it, just wondering.

Comment 10

[Adrian Tamas (:AdrianT)]

(In reply to Lucas Rocha (:lucasr) from comment #9)
> Comment on attachment 808579 [details] [diff] [review]
> testBookmark.patch
> 
> Review of attachment 808579 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good.
> 
> ::: mobile/android/base/tests/AboutHomeTest.java.in
> @@ +102,5 @@
> > +        ListAdapter adapter = bookmarksTabList.getAdapter();
> > +        if (adapter != null) {
> > +            for (int i = 0; i < adapter.getCount(); i++ ) {
> > +                // I am unable to click the view taken with getView for some reason so getting the child at i
> > +                bookmarksTabList.smoothScrollToPosition(i);
> 
> This is likely to cause intermittent due to timing issues from the smooth
> scrolling, no? Not against it, just wondering.

I haven't had any issues with it. Usually the scroll is very short for since we don't have long bookmark lists tests.

Comment 11

[Lucas Rocha (:lucasr)]

Adrian, are you just waiting for someone to push your patch?

Comment 12

[Adrian Tamas (:AdrianT)]

Until bug 919516 is fixed this will be permaorange on pandas. This is why I didn't even bother to do a tryserver run with this. Once bug 919516 is fixed I can do a run with the patch and if everything is ok we can push it.

Comment 13

[Adrian Tamas (:AdrianT)]

Tryrun with the patch for bug 919516 added: https://tbpl.mozilla.org/?tree=Try&rev=7b424127b7c9

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/35b839744801

Comment 15

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/35b839744801

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4bb7318a5a31

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Needs bug 919516 first, duh.
https://hg.mozilla.org/releases/mozilla-aurora/rev/6c1ea36a42e6

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ed7ca8db1123