incubator-callback-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Filip Maj <...@adobe.com>
Subject Re: Pull request for updated contacts tests
Date Thu, 15 Mar 2012 16:59:46 GMT
Merge fail on my part on that last one!

Thanks for the fix Becky. I've pushed up to the mobile-spec master.

On 3/15/12 7:40 AM, "Becky Gibson" <gibson.becky@gmail.com> wrote:

>One correction.  This works with MY version of the updated contacts.js
>that
>is on Fil's cordova-js becky branch - NOT the one that is in Fil's
>cordova-js ios branch.
>-becky
>
>On Thu, Mar 15, 2012 at 9:46 AM, Becky Gibson
><gibson.becky@gmail.com>wrote:
>
>> I was able to get Fil's updated contacts test running on iOS with the
>>code
>> in his cordova-js and cordova-ios branches for the unified iOS work.  It
>> just involved some changes to the tests.   The updates are posted here:
>> 
>>https://github.com/becka11y/incubator-cordova-mobile-spec/tree/fixContact
>>s
>>
>> The main change was to updated the global variable used to store the
>> contact to refer to the current contact after a save.  It is worth
>>noting
>> that when you call save on a Contact object, the Contact object that is
>> returned in the save success function does not exactly match the
>>original
>> contact.  For one, the id has been updated after a save.  Also, there
>>may
>> be other subtle differences due to translating some fields between the
>> device and the W3C Contact object.   For example,  as documented at
>> docs.phonegap.com, iOS will ignore any ContactName.formatted input (as
>> there is no field to save this in on iOS) but will return a
>> ContactName.formatted as the composite name from the iOS address book.
>>I
>> expect other platforms have similar quirks.  It does seem strange that
>>when
>> you call save on an object, you can no longer use that object but this
>>is
>> how Contacts have been working since they were added to PhoneGap.
>>
>> For this reason, I also removed the test to compare the original and
>>saved
>> contact.  We may want to do some other check to make sure the contact is
>> saved as expected.
>>
>> -becky
>>
>>
>> On Thu, Mar 15, 2012 at 7:20 AM, Becky Gibson
>><gibson.becky@gmail.com>wrote:
>>
>>> Hmm,  they all worked on the cordova-js rework branch when I pushed
>>>them.
>>>  Will take a look.
>>>
>>> -becky
>>>
>>>
>>> On Wed, Mar 14, 2012 at 6:27 PM, Filip Maj <fil@adobe.com> wrote:
>>>
>>>> Hey Becky and everyone else,
>>>>
>>>> I made a few tweaks to your pull request and have merged it in. I made
>>>> the
>>>> tests a bit more atomic and added some setup/teardown methods for
>>>> cleaning
>>>> up the global contact objects created (so we don't persist contacts
>>>>into
>>>> the device address book, or at least try not to).
>>>>
>>>> I am about to push up the improved tests to the mobile-spec repo. All
>>>>of
>>>> the new tests pass on Android (hurray!). Some of the test are failing
>>>>on
>>>> iOS though (at least the cordova-js rework for iOS).
>>>>
>>>> On 3/2/12 4:26 PM, "Filip Maj" <fil@adobe.com> wrote:
>>>>
>>>> >Looks good, thanks for the tests. Some of them are failing on my
>>>>Android
>>>> >device but: that's what tests are for!
>>>> >
>>>> >I've got a couple comments re: use of Qunit APIs which I think are
>>>> >incorrect but I'll add those to the pull request.
>>>> >
>>>> >On 3/2/12 2:33 PM, "Becky Gibson" <gibson.becky@gmail.com> wrote:
>>>> >
>>>> >>I've updated the contacts test to test saving, updating and
>>>>removing a
>>>> >>contact.  This also check for proper treatment of dates. These tests
>>>> run
>>>> >>on
>>>> >>the current iOS implementation.  I'd like to have it tested on other
>>>> >>implementations before I commit.   Here is the pull request against
>>>>the
>>>> >>github repo:
>>>> >>https://github.com/apache/incubator-cordova-mobile-spec/pull/1
>>>> >>
>>>> >>Pull Request comments:
>>>> >>
>>>> >>Added tests to save, update and remove a contact.
>>>> >>
>>>> >>Save and update verify Date is saved, updated and retrieved
>>>>properly.
>>>> >>Test
>>>> >>modifying a note, removing an email address.
>>>> >>
>>>> >>These new tests work with the current iOS implementation. NOTE -
>>>>these
>>>> >>tests now ADD and REMOVE a contact. You may want to run on a
>>>>simulator
>>>> >>first so you don't risk losing contacts from an actual device!!!!
>>>>Also,
>>>> >>the
>>>> >>test to remove an invalid contact uses and id of 999 (it was
>>>>previously
>>>> >>99
>>>> >>and I made it larger since I think it is conceivable that a device
>>>> could
>>>> >>have a valid contact with id 99). I think even 999 is risky since
>>>> >>different
>>>> >>devices may use different conventions for assigning ids. There is
>>>>now a
>>>> >>test that uses an id for a contact that was removed to verify that
>>>> trying
>>>> >>to remove a contact with an invalid ID returns the proper error.
As
>>>> long
>>>> >>as
>>>> >>the remove works, this test should be valid and we could get rid
of
>>>>the
>>>> >>removal test with the arbitrary id of 999.
>>>> >>
>>>> >>Please test and let me know of any issues before I commit.
>>>> >>
>>>> >>These contacts tests could be HUGE in order to test all of the
>>>> >>permutations
>>>> >>for contacts. There are other plugins that could use more testing
>>>>love
>>>> as
>>>> >>well.  Not sure if we want to make the automated tests that large?
>>>> >>
>>>> >>thanks,
>>>> >>
>>>> >>-becky
>>>> >
>>>>
>>>>
>>>
>>


Mime
View raw message