Closed Bug 1330280 Opened 3 years ago Closed 3 years ago

Refactor testActivityStreamContextMenu to match new code

Categories

(Firefox for Android :: Testing, defect, P1)

All
Android
defect

Tracking

()

RESOLVED FIXED
Firefox 53
Iteration:
1.13
Tracking Status
firefox53 --- fixed

People

(Reporter: sebastian, Assigned: sebastian)

References

Details

(Whiteboard: [MobileAS])

Attachments

(1 file)

Follow-up from bug 1328937. There's some refactoring needed to make testActivityStreamContextMenu work with the new code. The test is disabled for now.
Iteration: --- → 1.13
Priority: -- → P1
Whiteboard: [MobileAS]
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 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+
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
https://hg.mozilla.org/mozilla-central/rev/deb2014e1eb5
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
You need to log in before you can comment on or make changes to this bug.