841377 - Robocop: Extend the testShareLink test to cover opening the share popup from context menues

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Adrian Tamas (:AdrianT)]

Attachment: share link test

This patch extends the testShareLink test to cover opening the share popups from context menus:
1) the urlbar context menu
2) a link's context menu
3) top sites item context menu
4) bookmark item context menu
5) history item context menu

The patch also removes the separate tests for the share items when opened from the app menu since the changes to it have been reverted and now for honeycomb and newer we also get a pop-up generated by Android and not a submenu with the options (bug 809790)

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 713913 [details] [diff] [review]
share link test

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

this runs fine for me locally.  If you could answer the question about the loop below, I would appreciate that.

::: mobile/android/base/tests/testShareLink.java.in
@@ +62,5 @@
> +
> +        // Test share popup in Top Sites
> +        ListView tslist = null;
> +        do {
> +           tslist = getTopSitesList();

nit: 4 space indent.

@@ +63,5 @@
> +        // Test share popup in Top Sites
> +        ListView tslist = null;
> +        do {
> +           tslist = getTopSitesList();
> +        } while (tslist == null); // Sometime the list is not correctly returned. Make sure it's not null

can this loop forever?  it seems dangerous to have a while (tslist == null) condition.

Comment 2

[Geoff Brown [:gbrown]]

Comment on attachment 713913 [details] [diff] [review]
share link test

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

::: mobile/android/base/tests/testShareLink.java.in
@@ +178,5 @@
>          mAsserter.ok(success,"Got the displayed share options?", "Got the share options list");
>          return list;
>      }
> +
> +    private ListView getTopSitesList() {

Please use BaseTest.getAllPagesList instead.

Comment 3

[Adrian Tamas (:AdrianT)]

Attachment: Share Link test v1.1

Removed the getTopSitesList method and used the getAllPagesList method indicated by Geoff. Also the method does not seem to return null so I removed the problematic do...while. I don't believe it would have presented any issues since in 2-3 tries it would have returned the list but this new method does not seem to fail at all in my tests. Keeping the (view != null) check just to be sure although now I did not find the test failing there anymore.

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 714319 [details] [diff] [review]
Share Link test v1.1

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

thanks!  I will retest and work on landing this.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/fe3b9c524a1c