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

RESOLVED FIXED in Firefox 22

Status

()

Firefox for Android
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: AdrianT, Assigned: AdrianT)

Tracking

Trunk
Firefox 22
ARM
Android
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Created attachment 713913 [details] [diff] [review]
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)
Attachment #713913 - Flags: review?(jmaher)
(Assignee)

Updated

5 years ago
Blocks: 744859
Depends on: 826261
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.
Attachment #713913 - Flags: review?(jmaher) → review+
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.
(Assignee)

Comment 3

5 years ago
Created attachment 714319 [details] [diff] [review]
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.
Attachment #713913 - Attachment is obsolete: true
Attachment #714319 - Flags: review?(jmaher)
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.
Attachment #714319 - Flags: review?(jmaher) → review+
https://hg.mozilla.org/mozilla-central/rev/fe3b9c524a1c
Assignee: nobody → adrian.tamas
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 22
You need to log in before you can comment on or make changes to this bug.