1235599 - Add tests for LocalURLMetadata

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Testing, defect)

Description

[Andrzej Hunt :ahunt]

We currently have no tests for the site metadata storage (preview tiles, tile colors, and touch icons as of Bug 826400) - we should add tests for this.

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/db/LocalURLMetadata.java

This is especially important because of the caching at:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/db/LocalURLMetadata.java#114

Comment 1

[Andrzej Hunt :ahunt]

Attachment: Bug 1235599 - Make UrlMetadataTable.CONTENT_URI public to allow using in tests

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

Comment 2

[Andrzej Hunt :ahunt]

Attachment: Bug 1235599 - Add basic UrlMetadata tests

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

Comment 3

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

Comment on attachment 8760409 [details]
Bug 1235599 - Make UrlMetadataTable.CONTENT_URI public to allow using in tests

https://reviewboard.mozilla.org/r/57976/#review54930

Comment 4

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

Comment on attachment 8760410 [details]
Bug 1235599 - Add basic UrlMetadata tests

https://reviewboard.mozilla.org/r/57978/#review55544

::: mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testBrowserProvider.java:1523
(Diff revision 1)
> +    private class TestInsertUrlMetadata extends TestCase {
> +        @Override
> +        public void test() throws Exception {
> +            testInsertionViaContentProvider();
> +            testInsertionViaUrlMetadata();
> +            testRetrievalViaUrlMetadata();

You should add a comment in here somewhere that this test relies on the results of the previous tests.

::: mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testBrowserProvider.java:1543
(Diff revision 1)
> +                    new String[] {url1}
> +            );
> +
> +            final Cursor c = getUrlMetadataByUrl(url1);
> +            try {
> +                mAsserter.is(c.moveToFirst(), true, "URL metadata inserted via Content Provider not found");

It'd more explicit to check `Cursor.getCount`

::: mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testBrowserProvider.java:1564
(Diff revision 1)
> +
> +            getTestProfile().getDB().getURLMetadata().save(mResolver, data);
> +
> +            final Cursor c = getUrlMetadataByUrl(url2);
> +            try {
> +                mAsserter.is(c.moveToFirst(), true, "URL metadata inserted via UrlMetadata not found");

Same.

::: mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testBrowserProvider.java:1571
(Diff revision 1)
> +            // LocalURLMetadata has some caching of results: we need to test that this caching
> +            // doesn't prevent us from accessing data that might not have been loaded into the cache.

nit: How do we do that? And how do we test around the cache? Are we trying to? Elaborate.

::: mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testBrowserProvider.java:1601
(Diff revision 1)
> +            mAsserter.is(urlData.containsKey(URLMetadataTable.TILE_COLOR_COLUMN), true, "touchIcon column missing in UrlMetadata results");
> +
> +
> +            // 3: retrieve all columns for both URLs
> +            results = metadata.getForURLs(mResolver,
> +                    Arrays.asList(new String[] {

`Arrays.asList(url1, url2)` might just work, I'm not sure. I've seen sebastian use syntax like that before.

::: mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testBrowserProvider.java:1611
(Diff revision 1)
> +            mAsserter.is(results.containsKey(url1), true, "URL 1 not found in results");
> +            mAsserter.is(results.containsKey(url2), true, "URL 2 not found in results");
> +
> +            urlData = results.get(url1);
> +            mAsserter.is(urlData.containsKey(URLMetadataTable.TILE_IMAGE_URL_COLUMN), true, "touchIcon column missing in UrlMetadata results");
> +            mAsserter.is(urlData.containsKey(URLMetadataTable.TILE_COLOR_COLUMN), true, "touchIcon column missing in UrlMetadata results");
> +            mAsserter.is(urlData.containsKey(URLMetadataTable.TOUCH_ICON_COLUMN), true, "touchIcon column missing in UrlMetadata results");
> +
> +            urlData = results.get(url2);
> +            mAsserter.is(urlData.containsKey(URLMetadataTable.TILE_IMAGE_URL_COLUMN), true, "touchIcon column missing in UrlMetadata results");
> +            mAsserter.is(urlData.containsKey(URLMetadataTable.TILE_COLOR_COLUMN), true, "touchIcon column missing in UrlMetadata results");
> +            mAsserter.is(urlData.containsKey(URLMetadataTable.TOUCH_ICON_COLUMN), true, "touchIcon column missing in UrlMetadata results");

nit: you could do this in a loop

`for (final String url : new String[] { url1, url2 }) {`

Or re-using the list you made above.

Comment 5

[Andrzej Hunt :ahunt]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8c2c19a40541

Comment 6

[Andrzej Hunt :ahunt]

https://hg.mozilla.org/integration/fx-team/rev/98220ccf6c93d6530c73e44f5511c23420fbe95e
Bug 1235599 - Make UrlMetadataTable.CONTENT_URI public to allow using in tests r=mcomella

https://hg.mozilla.org/integration/fx-team/rev/bc55f3804d80c29e66abf8352faa705c6dca1d41
Bug 1235599 - Add basic UrlMetadata tests r=mcomella

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/98220ccf6c93
https://hg.mozilla.org/mozilla-central/rev/bc55f3804d80