[fig] Re-implement robocop testBookmark.java.in

RESOLVED FIXED in Firefox 26

Status

()

defect
P1
normal
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: capella, Assigned: AdrianT)

Tracking

unspecified
Firefox 27
All
Android
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox26 fixed, firefox27 fixed, fennec26+)

Details

Attachments

(1 attachment, 5 obsolete attachments)

(Reporter)

Description

6 years ago
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.
(Assignee)

Updated

6 years ago
Blocks: 895673
(Assignee)

Updated

6 years ago
QA Contact: adrian.tamas
(Assignee)

Comment 1

6 years ago
Posted patch testBookmark rewrite (obsolete) — Splinter Review
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.
Attachment #791274 - Flags: review?(lucasr.at.mozilla)
(Assignee)

Comment 2

6 years ago
Posted patch testBookmark fix v2 (obsolete) — Splinter Review
removed the openAboutHomeTab fix which was moved to bug 906662
Attachment #791274 - Attachment is obsolete: true
Attachment #791274 - Flags: review?(lucasr.at.mozilla)
Attachment #792200 - Flags: review?(lucasr.at.mozilla)
(Assignee)

Updated

6 years ago
Blocks: 899187
Assignee: nobody → adrian.tamas
tracking-fennec: --- → ?
Priority: -- → P1
Hardware: ARM → All
tracking-fennec: ? → 26+
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.
Attachment #792200 - Flags: review?(lucasr.at.mozilla) → review-
(Assignee)

Updated

6 years ago
Depends on: 916107
(Assignee)

Comment 4

6 years ago
Posted patch testBookmark.patch v3 (obsolete) — Splinter Review
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.
Attachment #792200 - Attachment is obsolete: true
Attachment #804460 - Flags: review?(lucasr.at.mozilla)
(Assignee)

Comment 5

6 years ago
Posted patch testBookmark.patch v4 (obsolete) — Splinter Review
Changes the patch to not rely on a tagged Top Bookmarks view and also to work with the recently integrated DatabaseHelper and StringHelper classes
Attachment #804460 - Attachment is obsolete: true
Attachment #804460 - Flags: review?(lucasr.at.mozilla)
Attachment #805258 - Flags: review?(lucasr.at.mozilla)
(Assignee)

Comment 6

6 years ago
Posted patch testBookmark.patch v4.1 (obsolete) — Splinter Review
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
Attachment #805258 - Attachment is obsolete: true
Attachment #805258 - Flags: review?(lucasr.at.mozilla)
Attachment #805345 - Flags: review?(lucasr.at.mozilla)
(Assignee)

Comment 7

6 years ago
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
Attachment #805345 - Flags: review?(lucasr.at.mozilla)
(Assignee)

Updated

6 years ago
Depends on: 919516
(Assignee)

Comment 8

6 years ago
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
Attachment #805345 - Attachment is obsolete: true
Attachment #808579 - Flags: review?(lucasr.at.mozilla)
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.
Attachment #808579 - Flags: review?(lucasr.at.mozilla) → review+
(Assignee)

Comment 10

6 years ago
(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.
Adrian, are you just waiting for someone to push your patch?
Flags: needinfo?(adrian.tamas)
(Assignee)

Comment 12

6 years ago
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.
Flags: needinfo?(adrian.tamas)
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/35b839744801
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/35b839744801
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 27
Needs bug 919516 first, duh.
https://hg.mozilla.org/releases/mozilla-aurora/rev/6c1ea36a42e6
Whiteboard: [don't uplift until bug 919516 has approval]
You need to log in before you can comment on or make changes to this bug.