Refactor testActivityStreamContextMenu to match new code

RESOLVED FIXED in Firefox 53

Status

()

Firefox for Android
Testing
P1
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: sebastian, Assigned: sebastian)

Tracking

unspecified
Firefox 53
All
Android
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

(Whiteboard: [MobileAS])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

a year ago
Follow-up from bug 1328937. There's some refactoring needed to make testActivityStreamContextMenu work with the new code. The test is disabled for now.
(Assignee)

Updated

a year ago
Iteration: --- → 1.13
Priority: -- → P1
Whiteboard: [MobileAS]
Comment hidden (mozreview-request)

Comment 2

a year ago
mozreview-review
Comment on attachment 8826226 [details]
Bug 1330280 - Refactor testActivityStreamContextMenu to use new Highlight/TopSite model classes.

https://reviewboard.mozilla.org/r/104206/#review105208
Attachment #8826226 - Flags: review?(ahunt) → review+

Comment 3

a year ago
mozreview-review
Comment on attachment 8826226 [details]
Bug 1330280 - Refactor testActivityStreamContextMenu to use new Highlight/TopSite model classes.

https://reviewboard.mozilla.org/r/104206/#review105396

Nice! I like that you enumerated the possible cases. Reminds me of my digital circuits class.

::: mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testActivityStreamContextMenu.java:306
(Diff revision 1)
>          mAsserter.is(item.getTitle(), expectedTitle, "Title does not match expected title");
>      }
> +
> +
> +    private Highlight findHighlightByUrl(String url) {
> +        final Cursor cursor = db.getHighlights(getActivity(), 20).loadInBackground();

This cursor is never closed.

::: mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testActivityStreamContextMenu.java:313
(Diff revision 1)
> +        mAsserter.isnot(cursor, null, "Highlights cursor is not null");
> +        mAsserter.ok(cursor.getCount() > 0, "Highlights cursor has entries", null);
> +
> +        mAsserter.dumpLog("Database returned " + cursor.getCount() + " highlights");
> +
> +        cursor.moveToFirst();

You probably want to assert that moveToFirst succeeded, since you're using `do/while`. But I guess you're asserting that getCount>0, so this _shouldn't_ fail ;-)
Attachment #8826226 - Flags: review?(gkruglov) → review+
Comment hidden (mozreview-request)

Comment 5

a year ago
Pushed by s.kaspari@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/deb2014e1eb5
Refactor testActivityStreamContextMenu to use new Highlight/TopSite model classes. r=ahunt,Grisha

Comment 6

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/deb2014e1eb5
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
You need to log in before you can comment on or make changes to this bug.