Closed Bug 903478 Opened 11 years ago Closed 11 years ago

[fig] Update testHistoryTab (now called testMostRecentPage) for new about:home

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 26

People

(Reporter: lucasr, Assigned: lucasr)

References

Details

Attachments

(3 files)

No description provided.
Attachment #788978 - Flags: review?(margaret.leibovic)
Attachment #788979 - Flags: review?(margaret.leibovic)
Attachment #788980 - Flags: review?(margaret.leibovic)
FYI: this is a straight port of the original testHistoryTab.
Comment on attachment 788978 [details] [diff] [review] (1/3) Tag all ListViews in the new about:home UI Review of attachment 788978 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/home/HomePager.java @@ +41,5 @@ > + static final String LIST_TAG_BOOKMARKS = "bookmarks"; > + static final String LIST_TAG_READING_LIST = "reading_list"; > + static final String LIST_TAG_MOST_VISITED = "most_visited"; > + static final String LIST_TAG_MOST_RECENT = "most_recent"; > + static final String LIST_TAG_LAST_TABS = "last_tabs"; Add a comment documenting what these are used for. Hopefully that will help us remember to add new tags for any new fragments we create in the future.
Attachment #788978 - Flags: review?(margaret.leibovic) → review+
Comment on attachment 788979 [details] [diff] [review] (2/3) Add findListViewWithTag() to BaseTest Review of attachment 788979 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/tests/BaseTest.java.in @@ +282,5 @@ > } > mAsserter.is(urlBarText, url, "Browser toolbar URL stayed the same"); > } > > + public final ListView findListViewWithTag(String tag) { Nit: add some documentation on top of this method. @@ +295,5 @@ > + } > + } > + > + return null; > + } This does seem generic enough to belong in BaseTest, but if it's only going to be used for about:home views, I wonder if it would be better in AboutHomeTest.
Attachment #788979 - Flags: review?(margaret.leibovic) → review+
Comment on attachment 788980 [details] [diff] [review] (3/3) Add testMostRecentPage for new about:home Review of attachment 788980 [details] [diff] [review]: ----------------------------------------------------------------- I know you're just porting over testHistoryTab, but it's still kinda painful to see some of this code being added back. Please be sure to file a follow-up on tidying this up, and perhaps add some comments in places where you know we want something to change. ::: mobile/android/base/tests/testMostRecentPage.java.in @@ +174,5 @@ > + } > + } > + > + private void testClick(final ListView listview, String url) { > + // waitForText(url); Can this just be removed? @@ +222,5 @@ > + uri = uri.buildUpon().appendQueryParameter("profile", "default") > + .appendQueryParameter("sync", "true").build(); > + resolver.delete(uri, "url = ?", new String[] { > + "http://mochi.test:8888/tests/robocop/robocop_blank_01.html" > + }); We should udpate this use the new deleteBookmark method in AboutHomeTest. It's okay to do that in a follow-up.
Attachment #788980 - Flags: review?(margaret.leibovic) → review+
(In reply to :Margaret Leibovic from comment #5) > Comment on attachment 788978 [details] [diff] [review] > (1/3) Tag all ListViews in the new about:home UI > > Review of attachment 788978 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: mobile/android/base/home/HomePager.java > @@ +41,5 @@ > > + static final String LIST_TAG_BOOKMARKS = "bookmarks"; > > + static final String LIST_TAG_READING_LIST = "reading_list"; > > + static final String LIST_TAG_MOST_VISITED = "most_visited"; > > + static final String LIST_TAG_MOST_RECENT = "most_recent"; > > + static final String LIST_TAG_LAST_TABS = "last_tabs"; > > Add a comment documenting what these are used for. Hopefully that will help > us remember to add new tags for any new fragments we create in the future. Done.
(In reply to :Margaret Leibovic from comment #6) > Comment on attachment 788979 [details] [diff] [review] > (2/3) Add findListViewWithTag() to BaseTest > > Review of attachment 788979 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: mobile/android/base/tests/BaseTest.java.in > @@ +282,5 @@ > > } > > mAsserter.is(urlBarText, url, "Browser toolbar URL stayed the same"); > > } > > > > + public final ListView findListViewWithTag(String tag) { > > Nit: add some documentation on top of this method. Done. > @@ +295,5 @@ > > + } > > + } > > + > > + return null; > > + } > > This does seem generic enough to belong in BaseTest, but if it's only going > to be used for about:home views, I wonder if it would be better in > AboutHomeTest. Fair enough. Moved to AboutHomeTest.
Pushed the first two patches in order to land the fix for bug 903448: http://hg.mozilla.org/projects/fig/rev/45ed1f18184f http://hg.mozilla.org/projects/fig/rev/fbbc87078c3c I'll push the last patch once the test goes green on try.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: