889673 - Adapt Contacts Unit Tests for Android

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Shane Tully (:stully:]

The contacts unit tests need adapting for the Android contacts API implementation.

This mainly includes:

* Removing the explict import of the fallback ContactService.jsm.
* Ensure that name field is always consistent with the prefix, given name, family name, and suffix fields.
* Ignoring the implicitly added default category.
* Ignoring failures of the getRevision() function.
* Ignoring tests of the updated and published contact properties.
* Changing a few comparisons of the name field to arrays rather than string.

Comment 1

[Shane Tully (:stully:]

Attachment: Adapt contacts unit tests for Android

Tests pass on B2G (obviously they fail on Android until the contacts API for Android is landed). https://tbpl.mozilla.org/?tree=Try&rev=76e7c1341749

Comment 2

[Gregor Wagner [:gwagner]]

Comment on attachment 770577 [details] [diff] [review]
Adapt contacts unit tests for Android

I am currently not in the office and have a big review backlog. I will be back on Monday. Maybe Reuben can do an initial pass.

Comment 3

[Shane Tully (:stully:]

No problem. Sorry for dumping all these contacts related reviews on you!

Comment 4

[Reuben Morais [:reuben]]

Comment on attachment 770577 [details] [diff] [review]
Adapt contacts unit tests for Android

Review of attachment 770577 [details] [diff] [review]:
-----------------------------------------------------------------

Patch looks OK, but we need to figure out how to run them on Android *and* Desktop :) review- because of that.

Any reason why you're not simply checking the platform before you load ContactService/PermissionPromptHelper?

Comment 5

[Shane Tully (:stully:]

I'm not sure what you mean by run them on Android and desktop. As far as I know, desktop would use the fallback contacts (the same as B2G). This patch is only to adapt the unit tests for Android.

I'm also not sure where the PermissionPromptHelper comes into play here. The patch removes the import of PermissionPromptHelper because it should already be loaded elsewhere since the contacts API implmentation relies on it, not the unit tests.

Comment 6

[Shane Tully (:stully:]

Attachment: Adapt contacts unit tests for Android

Re-added imports of ContactService.jsm and PermissionPromptHelper.jsm.

Comment 7

[Reuben Morais [:reuben]]

Comment on attachment 771122 [details] [diff] [review]
Adapt contacts unit tests for Android

Review of attachment 771122 [details] [diff] [review]:
-----------------------------------------------------------------

r=reuben with the key test added back or explained :)

::: dom/contacts/tests/test_contacts_basics.html
@@ -84,5 @@
>    nickname: "nicktest",
>    tel: [{type: ["work"], value: "123456", carrier: "testCarrier"} , {type: ["home", "fax"], value: "[phone]"}, {type: ["home"], value: "[phone]"}],
>    adr: adr1,
>    email: [{type: ["work"], value: "[e-mail]"}],
> -  key: "4343JEGJGERNBEGOI34G3WGVERBERB"

Why did you remove this?

Comment 8

[Shane Tully (:stully:]

I removed it because it seems like prop1 is a minimal set of properties, whereas prop2 is a full contact and the key field isn't exactly a "core" field. I was actually the one that added it in bug 807688, but looking at it again now, it seems like it shouldn't be there. It can stay though if you want.

Comment 9

[Shane Tully (:stully:]

Attachment: Adapt contacts unit tests for Android

Updated for changes in bug 892497

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/570a4dafc5db

Comment 11

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 778118 [details] [diff] [review]
Adapt contacts unit tests for Android

I'm assuming my review is no longer needed.  Ask again if I'm wrong.