Closed
Bug 417065
Opened 17 years ago
Closed 17 years ago
Add some tests for the more complex nsIAbCard functions
Categories
(MailNews Core :: Address Book, defect)
MailNews Core
Address Book
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: standard8, Assigned: standard8)
References
Details
Attachments
(1 file, 3 obsolete files)
8.14 KB,
patch
|
neil
:
review+
neil
:
superreview+
|
Details | Diff | Splinter Review |
Looking at the review for bug 413260, I think it would help if we had at least some minimum tests for convertToBase64EncodedXML, convertToXMLPrintData and convertToEscapedVCard.
The attached tests basically just take a fully-filled in nsIAbCard and checks the results for them.
Yes, I did just copy and paste from what it currently does, but I don't think that's too much of a problem. We can expand them later and think about standards then, I'm just looking to get something in so that we can check we don't do major regressions.
Doing these functions also gives us the advantage is these functions should cover pretty much all the current properties that we use on ab cards.
Attachment #302872 -
Flags: superreview?(neil)
Attachment #302872 -
Flags: review?(neil)
Assignee | ||
Comment 1•17 years ago
|
||
This one has better comments at the top of it that I was meant to have saved before I generated the previous patch...
Attachment #302872 -
Attachment is obsolete: true
Attachment #302878 -
Flags: superreview?(neil)
Attachment #302878 -
Flags: review?(neil)
Attachment #302872 -
Flags: superreview?(neil)
Attachment #302872 -
Flags: review?(neil)
Comment 2•17 years ago
|
||
Comment on attachment 302878 [details] [diff] [review]
The fix v2
>+ var testAB = do_get_file("mailnews/addrbook/test/unit/data/cardForEmail.mab");
This is an existing file, right?
>+ var rdf = Components.classes["@mozilla.org/rdf/rdf-service;1"]
>+ .getService(Components.interfaces.nsIRDFService);
>+ do_check_true(rdf != null);
getService never returns null (it will throw on error)
>+ var AB = rdf.GetResource(kPABData.URI)
>+ .QueryInterface(Components.interfaces.nsIAbDirectory);
>+ do_check_true(AB != null);
As will QueryInterface. But you could do_check_true(AB instanceof ...)
>+ // If QI failes, just set tempCard to null.
Nit: fails
>+ try {
>+ tempCard = childCards.getNext().QueryInterface(Components.interfaces.nsIAbCard);
>+ }
>+ catch (ex) {
>+ tempCard = null;
>+ }
>+ // We want the one with this email...
>+ if (tempCard && tempCard.primaryEmail == "PrimaryEmail1@test.invalid")
Again, easier to use tempCard instanceof nsIAbCard &&
>+ do_check_true(fullCard != childCards);
This makes no sense.
>+ do_check_eq(fullCard.convertToXMLPrintData(),
This isn't an ideal test, depending on what it is you're actually testing...
>+ do_check_eq(fullCard.convertToBase64EncodedXML(),
>+ "PD94bWwgdmVyc2lvbj0iMS4wIj8+Cjw/eG1sLXN0eWxlc2hlZXQgdHlwZT0idGV4dC9jc3MiIGhyZWY9ImNocm9tZTovL21lc3Nlbmdlci9jb250ZW50L2FkZHJlc3Nib29rL3ByaW50LmNzcyI/Pgo8ZGlyZWN0b3J5Pgo8dGl0bGUgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzE5OTkveGh0bWwiPkFkZHJlc3MgQm9vazwvdGl0bGU+CjxHZW5lcmF0ZWROYW1lPgpEaXNwbGF5TmFtZTE8L0dlbmVyYXRlZE5hbWU+Cjx0YWJsZT48dHI+PHRkPjxzZWN0aW9uPjxsYWJlbHJvdz48bGFiZWw+RGlzcGxheSBOYW1lOiA8L2xhYmVsPjxEaXNwbGF5TmFtZT5EaXNwbGF5TmFtZTE8L0Rpc3BsYXlOYW1lPjwvbGFiZWxyb3c+PGxhYmVscm93PjxsYWJlbD5OaWNrbmFtZTogPC9sYWJlbD48Tmlja05hbWU+Tmlja05hbWUxPC9OaWNrTmFtZT48L2xhYmVscm93PjxQcmltYXJ5RW1haWw+UHJpbWFyeUVtYWlsMUB0ZXN0LmludmFsaWQ8L1ByaW1hcnlFbWFpbD48U2Vjb25kRW1haWw+U2Vjb25kRW1haWwxQHRlc3QuaW52YWxpZDwvU2Vjb25kRW1haWw+PGxhYmVscm93PjxsYWJlbD5TY3JlZW4gTmFtZTogPC9sYWJlbD48X0FpbVNjcmVlbk5hbWU+U2NyZWVuTmFtZTE8L19BaW1TY3JlZW5OYW1lPjwvbGFiZWxyb3c+PC9zZWN0aW9uPjwvdGQ+PC90cj48dHI+PHRkPjxzZWN0aW9uPjxzZWN0aW9udGl0bGU+UGhvbmU8L3NlY3Rpb250aXRsZT48bGFiZWxyb3c+PGxhYmVsPldvcms6IDwvbGFiZWw+PFdvcmtQaG9uZT5Xb3JrUGhvbmUxPC9Xb3JrUGhvbmU+PC9sYWJlbHJvdz48bGFiZWxyb3c+PGxhYmVsPkhvbWU6IDwvbGFiZWw+PEhvbWVQaG9uZT5Ib21lUGhvbmUxPC9Ib21lUGhvbmU+PC9sYWJlbHJvdz48bGFiZWxyb3c+PGxhYmVsPkZheDogPC9sYWJlbD48RmF4TnVtYmVyPkZheE51bWJlcjE8L0ZheE51bWJlcj48L2xhYmVscm93PjxsYWJlbHJvdz48bGFiZWw+UGFnZXI6IDwvbGFiZWw+PFBhZ2VyTnVtYmVyPlBhZ2VyTnVtYmVyMTwvUGFnZXJOdW1iZXI+PC9sYWJlbHJvdz48bGFiZWxyb3c+PGxhYmVsPk1vYmlsZTogPC9sYWJlbD48Q2VsbHVsYXJOdW1iZXI+Q2VsbHVsYXJOdW1iZXIxPC9DZWxsdWxhck51bWJlcj48L2xhYmVscm93Pjwvc2VjdGlvbj48c2VjdGlvbj48c2VjdGlvbnRpdGxlPk90aGVyPC9zZWN0aW9udGl0bGU+PGxhYmVscm93PjxsYWJlbD5DdXN0b20gMTogPC9sYWJlbD48Q3VzdG9tMT5DdXN0b20xMTwvQ3VzdG9tMT48L2xhYmVscm93PjxsYWJlbHJvdz48bGFiZWw+Q3VzdG9tIDI6IDwvbGFiZWw+PEN1c3RvbTI+Q3VzdG9tMjE8L0N1c3RvbTI+PC9sYWJlbHJvdz48bGFiZWxyb3c+PGxhYmVsPkN1c3RvbSAzOiA8L2xhYmVsPjxDdXN0b20zPkN1c3RvbTMxPC9DdXN0b20zPjwvbGFiZWxyb3c+PGxhYmVscm93PjxsYWJlbD5DdXN0b20gNDogPC9sYWJlbD48Q3VzdG9tND5DdXN0b200MTwvQ3VzdG9tND48L2xhYmVscm93PjxOb3Rlcz5Ob3RlczE8L05vdGVzPjwvc2VjdGlvbj48L3RkPjx0ZD48c2VjdGlvbj48c2VjdGlvbnRpdGxlPkhvbWU8L3NlY3Rpb250aXRsZT48SG9tZUFkZHJlc3M+SG9tZUFkZHJlc3MxMTwvSG9tZUFkZHJlc3M+PEhvbWVBZGRyZXNzMj5Ib21lQWRkcmVzczIxPC9Ib21lQWRkcmVzczI+PEhvbWVDaXR5PkhvbWVDaXR5MTwvSG9tZUNpdHk+LCA8SG9tZVN0YXRlPkhvbWVTdGF0ZTE8L0hvbWVTdGF0ZT4gPEhvbWVaaXBDb2RlPkhvbWVaaXBDb2RlMTwvSG9tZVppcENvZGU+PEhvbWVDb3VudHJ5PkhvbWVDb3VudHJ5MTwvSG9tZUNvdW50cnk+PFdlYlBhZ2UyPmh0dHA6Ly9XZWJQYWdlMTE8L1dlYlBhZ2UyPjwvc2VjdGlvbj48c2VjdGlvbj48c2VjdGlvbnRpdGxlPldvcms8L3NlY3Rpb250aXRsZT48Sm9iVGl0bGU+Sm9iVGl0bGUxPC9Kb2JUaXRsZT48RGVwYXJ0bWVudD5EZXBhcnRtZW50MTwvRGVwYXJ0bWVudD48Q29tcGFueT5Pcmdhbml6YXRpb24xPC9Db21wYW55PjxXb3JrQWRkcmVzcz5Xb3JrQWRkcmVzczE8L1dvcmtBZGRyZXNzPjxXb3JrQWRkcmVzczI+V29ya0FkZHJlc3MyMTwvV29ya0FkZHJlc3MyPjxXb3JrQ2l0eT5Xb3JrQ2l0eTE8L1dvcmtDaXR5PiwgPFdvcmtTdGF0ZT5Xb3JrU3RhdGUxPC9Xb3JrU3RhdGU+IDxXb3JrWmlwQ29kZT5Xb3JrWmlwQ29kZTE8L1dvcmtaaXBDb2RlPjxXb3JrQ291bnRyeT5Xb3JrQ291bnRyeTE8L1dvcmtDb3VudHJ5PjxXZWJQYWdlMT5odHRwOi8vV2ViUGFnZTIxPC9XZWJQYWdlMT48L3NlY3Rpb24+PC90ZD48L3RyPjwvdGFibGU+PC9kaXJlY3Rvcnk+Cg==");
Is this simply the base64 of the last test? If it is, why not just compare the base64 encoded/decoded versions?
Attachment #302878 -
Flags: superreview?(neil)
Attachment #302878 -
Flags: superreview-
Attachment #302878 -
Flags: review?(neil)
Assignee | ||
Comment 3•17 years ago
|
||
(In reply to comment #2)
> (From update of attachment 302878 [details] [diff] [review])
> >+ var testAB = do_get_file("mailnews/addrbook/test/unit/data/cardForEmail.mab");
> This is an existing file, right?
Yes
> >+ do_check_eq(fullCard.convertToXMLPrintData(),
> This isn't an ideal test, depending on what it is you're actually testing...
Here (and the two tests below it) I just wanted to check all the fields we currently support to ensure that when we're doing big changes we've got at least a basic test coverage. I think these functions need redoing anyway, but that's another bug.
> >+ do_check_eq(fullCard.convertToBase64EncodedXML(),
> >+ "PD94bWwgdmVyc2lvbj0iMS4wIj8+Cjw/eG1sLXN0eWxlc2hlZXQgdHlwZT0idGV4dC9jc3MiIGhyZWY9ImNocm9tZTovL21lc3Nlbmdlci9jb250ZW50L2FkZHJlc3Nib29rL3ByaW50LmNzcyI/Pgo8ZGlyZWN0b3J5Pgo8dGl0bGUgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzE5OTkveGh0bWwiPkFkZHJlc3MgQm9vazwvdGl0bGU+CjxHZW5lcmF0ZWROYW1lPgpEaXNwbGF5TmFtZTE8L0dlbmVyYXRlZE5hbWU+Cjx0YWJsZT48dHI+PHRkPjxzZWN0aW9uPjxsYWJlbHJvdz48bGFiZWw+RGlzcGxheSBOYW1lOiA8L2xhYmVsPjxEaXNwbGF5TmFtZT5EaXNwbGF5TmFtZTE8L0Rpc3BsYXlOYW1lPjwvbGFiZWxyb3c+PGxhYmVscm93PjxsYWJlbD5OaWNrbmFtZTogPC9sYWJlbD48Tmlja05hbWU+Tmlja05hbWUxPC9OaWNrTmFtZT48L2xhYmVscm93PjxQcmltYXJ5RW1haWw+UHJpbWFyeUVtYWlsMUB0ZXN0LmludmFsaWQ8L1ByaW1hcnlFbWFpbD48U2Vjb25kRW1haWw+U2Vjb25kRW1haWwxQHRlc3QuaW52YWxpZDwvU2Vjb25kRW1haWw+PGxhYmVscm93PjxsYWJlbD5TY3JlZW4gTmFtZTogPC9sYWJlbD48X0FpbVNjcmVlbk5hbWU+U2NyZWVuTmFtZTE8L19BaW1TY3JlZW5OYW1lPjwvbGFiZWxyb3c+PC9zZWN0aW9uPjwvdGQ+PC90cj48dHI+PHRkPjxzZWN0aW9uPjxzZWN0aW9udGl0bGU+UGhvbmU8L3NlY3Rpb250aXRsZT48bGFiZWxyb3c+PGxhYmVsPldvcms6IDwvbGFiZWw+PFdvcmtQaG9uZT5Xb3JrUGhvbmUxPC9Xb3JrUGhvbmU+PC9sYWJlbHJvdz48bGFiZWxyb3c+PGxhYmVsPkhvbWU6IDwvbGFiZWw+PEhvbWVQaG9uZT5Ib21lUGhvbmUxPC9Ib21lUGhvbmU+PC9sYWJlbHJvdz48bGFiZWxyb3c+PGxhYmVsPkZheDogPC9sYWJlbD48RmF4TnVtYmVyPkZheE51bWJlcjE8L0ZheE51bWJlcj48L2xhYmVscm93PjxsYWJlbHJvdz48bGFiZWw+UGFnZXI6IDwvbGFiZWw+PFBhZ2VyTnVtYmVyPlBhZ2VyTnVtYmVyMTwvUGFnZXJOdW1iZXI+PC9sYWJlbHJvdz48bGFiZWxyb3c+PGxhYmVsPk1vYmlsZTogPC9sYWJlbD48Q2VsbHVsYXJOdW1iZXI+Q2VsbHVsYXJOdW1iZXIxPC9DZWxsdWxhck51bWJlcj48L2xhYmVscm93Pjwvc2VjdGlvbj48c2VjdGlvbj48c2VjdGlvbnRpdGxlPk90aGVyPC9zZWN0aW9udGl0bGU+PGxhYmVscm93PjxsYWJlbD5DdXN0b20gMTogPC9sYWJlbD48Q3VzdG9tMT5DdXN0b20xMTwvQ3VzdG9tMT48L2xhYmVscm93PjxsYWJlbHJvdz48bGFiZWw+Q3VzdG9tIDI6IDwvbGFiZWw+PEN1c3RvbTI+Q3VzdG9tMjE8L0N1c3RvbTI+PC9sYWJlbHJvdz48bGFiZWxyb3c+PGxhYmVsPkN1c3RvbSAzOiA8L2xhYmVsPjxDdXN0b20zPkN1c3RvbTMxPC9DdXN0b20zPjwvbGFiZWxyb3c+PGxhYmVscm93PjxsYWJlbD5DdXN0b20gNDogPC9sYWJlbD48Q3VzdG9tND5DdXN0b200MTwvQ3VzdG9tND48L2xhYmVscm93PjxOb3Rlcz5Ob3RlczE8L05vdGVzPjwvc2VjdGlvbj48L3RkPjx0ZD48c2VjdGlvbj48c2VjdGlvbnRpdGxlPkhvbWU8L3NlY3Rpb250aXRsZT48SG9tZUFkZHJlc3M+SG9tZUFkZHJlc3MxMTwvSG9tZUFkZHJlc3M+PEhvbWVBZGRyZXNzMj5Ib21lQWRkcmVzczIxPC9Ib21lQWRkcmVzczI+PEhvbWVDaXR5PkhvbWVDaXR5MTwvSG9tZUNpdHk+LCA8SG9tZVN0YXRlPkhvbWVTdGF0ZTE8L0hvbWVTdGF0ZT4gPEhvbWVaaXBDb2RlPkhvbWVaaXBDb2RlMTwvSG9tZVppcENvZGU+PEhvbWVDb3VudHJ5PkhvbWVDb3VudHJ5MTwvSG9tZUNvdW50cnk+PFdlYlBhZ2UyPmh0dHA6Ly9XZWJQYWdlMTE8L1dlYlBhZ2UyPjwvc2VjdGlvbj48c2VjdGlvbj48c2VjdGlvbnRpdGxlPldvcms8L3NlY3Rpb250aXRsZT48Sm9iVGl0bGU+Sm9iVGl0bGUxPC9Kb2JUaXRsZT48RGVwYXJ0bWVudD5EZXBhcnRtZW50MTwvRGVwYXJ0bWVudD48Q29tcGFueT5Pcmdhbml6YXRpb24xPC9Db21wYW55PjxXb3JrQWRkcmVzcz5Xb3JrQWRkcmVzczE8L1dvcmtBZGRyZXNzPjxXb3JrQWRkcmVzczI+V29ya0FkZHJlc3MyMTwvV29ya0FkZHJlc3MyPjxXb3JrQ2l0eT5Xb3JrQ2l0eTE8L1dvcmtDaXR5PiwgPFdvcmtTdGF0ZT5Xb3JrU3RhdGUxPC9Xb3JrU3RhdGU+IDxXb3JrWmlwQ29kZT5Xb3JrWmlwQ29kZTE8L1dvcmtaaXBDb2RlPjxXb3JrQ291bnRyeT5Xb3JrQ291bnRyeTE8L1dvcmtDb3VudHJ5PjxXZWJQYWdlMT5odHRwOi8vV2ViUGFnZTIxPC9XZWJQYWdlMT48L3NlY3Rpb24+PC90ZD48L3RyPjwvdGFibGU+PC9kaXJlY3Rvcnk+Cg==");
> Is this simply the base64 of the last test? If it is, why not just compare the
> base64 encoded/decoded versions?
Suggestions please, because btoa doesn't work, neither does window.btoa in xpcshell tests :-(
Comment 4•17 years ago
|
||
(In reply to comment #3)
>Suggestions please, because btoa doesn't work, neither does window.btoa in
>xpcshell tests :-(
Not to worry, but ask around on IRC perhaps just in case there's a way?
Assignee | ||
Comment 5•17 years ago
|
||
Updated version, no easy way found for btoa, other xpcshell tests have worked around it in this way as well unfortunately :-(
Attachment #302878 -
Attachment is obsolete: true
Attachment #307091 -
Flags: superreview?(neil)
Attachment #307091 -
Flags: review?(neil)
Comment 6•17 years ago
|
||
I get that shutdown crash in this test too :-(
Assignee | ||
Comment 7•17 years ago
|
||
This should fix the windows crashes...
Attachment #307091 -
Attachment is obsolete: true
Attachment #308487 -
Flags: superreview?(neil)
Attachment #308487 -
Flags: review?(neil)
Attachment #307091 -
Flags: superreview?(neil)
Attachment #307091 -
Flags: review?(neil)
Comment 8•17 years ago
|
||
Comment on attachment 308487 [details] [diff] [review]
The fix v4
I double-checked against test_collection.js and it calls gc before cleanup, which stops the crash here too. r+sr with that (and its comment) included.
Attachment #308487 -
Flags: superreview?(neil)
Attachment #308487 -
Flags: superreview+
Attachment #308487 -
Flags: review?(neil)
Attachment #308487 -
Flags: review+
Assignee | ||
Comment 9•17 years ago
|
||
Patch checked in -> fixed.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•17 years ago
|
Flags: in-testsuite+
Updated•17 years ago
|
Product: Core → MailNews Core
You need to log in
before you can comment on or make changes to this bug.
Description
•