Tag the BrowserSearch ListView and the TopBookmarksView in order for UI tests to access them

RESOLVED FIXED in Firefox 27

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: AdrianT, Assigned: AdrianT)

Tracking

Trunk
Firefox 27
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Posted patch addTags.patch (obsolete) — Splinter Review
Adding tags to the BrowserSearch ListView and the TopBookmarksView in order to sort views after tags in UI tests.

Tryrun with the patch for testBookmark on top: https://tbpl.mozilla.org/?tree=Try&rev=d3cb0f41d276
Attachment #804457 - Flags: review?(lucasr.at.mozilla)
Comment on attachment 804457 [details] [diff] [review]
addTags.patch

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

Wouldn't a findbyid be enough for the top_bookmarks? Just wondering.
All the findViewById use an int and I don't have access from tests to R.java. Another solution might be to get the view as a FennecNativeElement and then cast it as a View and try to use it like that although I'm not very sure if that will work. This is the simplest solution in this case but I can look into this further next week if tagging the view is a problem.
(In reply to Adrian Tamas (:AdrianT) from comment #2)
> All the findViewById use an int and I don't have access from tests to
> R.java. Another solution might be to get the view as a FennecNativeElement
> and then cast it as a View and try to use it like that although I'm not very
> sure if that will work. This is the simplest solution in this case but I can
> look into this further next week if tagging the view is a problem.

Doesn't the new Robotium API support findViewById(String id)?
(In reply to Lucas Rocha (:lucasr) from comment #3)
> (In reply to Adrian Tamas (:AdrianT) from comment #2)
> > All the findViewById use an int and I don't have access from tests to
> > R.java. Another solution might be to get the view as a FennecNativeElement
> > and then cast it as a View and try to use it like that although I'm not very
> > sure if that will work. This is the simplest solution in this case but I can
> > look into this further next week if tagging the view is a problem.
> 
> Doesn't the new Robotium API support findViewById(String id)?
Only the new UI testing api that you worked on in bug 910859 has this method. We could wait on that to be integrated but I don't think we should. We could leave this as it is and remove the Top Bookmarks tagging when that is integrated. Another way to approach this is for me to add the findViewById method in this patch.
(In reply to Adrian Tamas (:AdrianT) from comment #4)
> (In reply to Lucas Rocha (:lucasr) from comment #3)
> > (In reply to Adrian Tamas (:AdrianT) from comment #2)
> > > All the findViewById use an int and I don't have access from tests to
> > > R.java. Another solution might be to get the view as a FennecNativeElement
> > > and then cast it as a View and try to use it like that although I'm not very
> > > sure if that will work. This is the simplest solution in this case but I can
> > > look into this further next week if tagging the view is a problem.
> > 
> > Doesn't the new Robotium API support findViewById(String id)?
> Only the new UI testing api that you worked on in bug 910859 has this
> method. We could wait on that to be integrated but I don't think we should.
> We could leave this as it is and remove the Top Bookmarks tagging when that
> is integrated. Another way to approach this is for me to add the
> findViewById method in this patch.

I actually mean Robotium's built-in getView(String id) API. See here:
http://robotium.googlecode.com/svn/doc/com/jayway/android/robotium/solo/Solo.html#getView%28java.lang.String%29
Posted patch searchViewTagging.patch (obsolete) — Splinter Review
Sorry I didn't look into this new method from Robotium 4.2 but unfortunately it seems that the top bookmarks view does not have an id. I can check if the view is a GridView for top bookmarks since it's the only grid view in bookmarks and I updated the testBookmark test to use that.
Attachment #804457 - Attachment is obsolete: true
Attachment #804457 - Flags: review?(lucasr.at.mozilla)
Attachment #805257 - Flags: review?(lucasr.at.mozilla)
Comment on attachment 805257 [details] [diff] [review]
searchViewTagging.patch

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

Looks good. Just needs the renames done.

::: mobile/android/base/home/HomePager.java
@@ +49,5 @@
>      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";
> +    static final String LIST_TAG_SEARCH = "search";

LIST_TAG_SEARCH -> LIST_TAG_BROWSER_SEARCH
search -> browser_search
Attachment #805257 - Flags: review?(lucasr.at.mozilla) → feedback+
Changed the tag and name as suggested
Attachment #805257 - Attachment is obsolete: true
Attachment #808441 - Flags: review?(lucasr.at.mozilla)
Comment on attachment 808441 [details] [diff] [review]
tagBrowserSearch.patch

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

Nice.
Attachment #808441 - Flags: review?(lucasr.at.mozilla) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/317e5d5c822b
Assignee: nobody → adrian.tamas
Status: NEW → RESOLVED
Closed: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 27
You need to log in before you can comment on or make changes to this bug.