Closed Bug 600798 Opened 14 years ago Closed 12 years ago

Implement unit tests for importing vcards and csv files into address book

Categories

(MailNews Core :: Import, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 17.0

People

(Reporter: standard8, Assigned: standard8)

References

Details

Attachments

(1 file, 2 obsolete files)

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.
Blocks: 603265
Summary: Implement unit tests for importing vcards and cvs files into address book → Implement unit tests for importing vcards and csv files into address book
Attached patch Updated patch (obsolete) — Splinter Review
This patch just updates the original test cases to the current format, now that the assertion bug is fixed these run and pass fine.
Assignee: nobody → mbanner
Attachment #479722 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #651407 - Flags: review?(mconley)
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");
Attachment #651407 - Flags: review?(mconley) → review-
Attached patch Updated patch v2Splinter Review
The only reason not to use MailServices was it wasn't there at the time the tests were originally written ;-)
Attachment #651407 - Attachment is obsolete: true
Attachment #653326 - Flags: review?(mconley)
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
Attachment #653326 - Flags: review?(mconley) → review+
Checked in: https://hg.mozilla.org/comm-central/rev/15bfe7e9d763
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 17.0
You need to log in before you can comment on or make changes to this bug.