In bug 79709 we wrote some unit tests for importing vcards and cvs files. However due to the current threadsafe assertions (bug 142123), these couldn't be committed due to breaking developer's test runs. This bug should look at getting these test cases landed.

This patch just updates the original test cases to the current format, now that the assertion bug is fixed these run and pass fine.

Comment on attachment 651407 [details] [diff] [review] Updated patch Review of attachment 651407 [details] [diff] [review]: ----------------------------------------------------------------- Good work, Mark! Just a few minor points / questions - nothing major. Thanks, -Mike ::: mailnews/import/test/unit/resources/basic_vcard_addressbook.vcf @@ +1,2 @@ > +BEGIN:VCARD > +VERSION:3.0 We don't actually support vCard >= 2.1 .... so this might be a bit deceptive. ::: mailnews/import/test/unit/resources/import_helper.js @@ +161,5 @@ > + this.setFieldMap(this.getDefaultFieldMap(true)); > + } else if (endsWith(this.mFile.leafName.toLowerCase(), ".vcf")) { > + this.mSupportedAttributes = supportedAttributes; > + }; > + Trailing whitespace ::: mailnews/import/test/unit/test_csv_import.js @@ +11,5 @@ > +function run_test() > +{ > + // Due to the import code using nsIAbManager off the main thread, we need > + // to ensure that it is initialized before we start the main test. > + var abMgr = Cc["@mozilla.org/abmanager;1"].getService(Ci.nsIAbManager); Can't use MailServices.ab? @@ +13,5 @@ > + // Due to the import code using nsIAbManager off the main thread, we need > + // to ensure that it is initialized before we start the main test. > + var abMgr = Cc["@mozilla.org/abmanager;1"].getService(Ci.nsIAbManager); > + > + var file = do_get_file("resources/basic_csv_addressbook.csv"); let over var ::: mailnews/import/test/unit/test_vcard_import.js @@ +11,5 @@ > +function run_test() > +{ > + // Due to the import code using nsIAbManager off the main thread, we need > + // to ensure that it is initialized before we start the main test. > + var abMgr = Cc["@mozilla.org/abmanager;1"].getService(Ci.nsIAbManager); let over var. Also, I guess there's no ability to use MailServices.ab? @@ +15,5 @@ > + var abMgr = Cc["@mozilla.org/abmanager;1"].getService(Ci.nsIAbManager); > + var file; > + > + // test regular import (e.g. from file exported by another mail client) > + file = do_get_file("resources/basic_vcard_addressbook.vcf"); Might as well merge this with line 16, and do let file = do_get_file("resources/basic_vcard_addressbook.vcf");

The only reason not to use MailServices was it wasn't there at the time the tests were originally written ;-)

Comment on attachment 653326 [details] [diff] [review] Updated patch v2 Review of attachment 653326 [details] [diff] [review]: ----------------------------------------------------------------- 3 tiny nits - other than that, this looks good. Thanks Mark! ::: mailnews/import/test/unit/resources/basic_vcard_addressbook.vcf @@ +4,5 @@ > +N:Doe;John;;; > +EMAIL;TYPE=INTERNET:john.doe@genericemail.com > +END:VCARD > +BEGIN:VCARD > +VERSION:3.0 2.1 here ::: mailnews/import/test/unit/resources/emptylines_vcard_addressbook.vcf @@ +1,4 @@ > + > + > +BEGIN:VCARD > +VERSION:3.0 2.1 here @@ +8,5 @@ > +END:VCARD > + > + > +BEGIN:VCARD > +VERSION:3.0 2.1 here

Checked in: https://hg.mozilla.org/comm-central/rev/15bfe7e9d763