Closed Bug 841377 Opened 7 years ago Closed 7 years ago

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

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 22

People

(Reporter: AdrianT, Assigned: AdrianT)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch share link test (obsolete) — Splinter Review
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)
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.
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
Closed: 7 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.