1330280 - Refactor testActivityStreamContextMenu to match new code

Status: RESOLVED FIXED

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

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

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

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: Bug 1330280 - Refactor testActivityStreamContextMenu to use new Highlight/TopSite model classes.

Review commit: https://reviewboard.mozilla.org/r/104206/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/104206/

Comment 2

[Andrzej Hunt :ahunt]

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

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

Comment 3

[:Grisha Kruglov]

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 ;-)

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/104206/diff/1-2/

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/deb2014e1eb5
Refactor testActivityStreamContextMenu to use new Highlight/TopSite model classes. r=ahunt,Grisha

Comment 6

[Carsten Book [:Tomcat]]

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