Note: There are a few cases of duplicates in user autocompletion which are being worked on.

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

RESOLVED FIXED in Thunderbird 17.0

Status

MailNews Core
Import
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

Trunk
Thunderbird 17.0
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Created attachment 479722 [details] [diff] [review]
Original Test Cases from bug 79709

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
Created attachment 651407 [details] [diff] [review]
Updated patch

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-
Created attachment 653326 [details] [diff] [review]
Updated patch v2

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
Last Resolved: 5 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.