Add test for _parseJID in XMPP

The same test file could also test normalize and normalizeFullJid and check all three functions are consistent with each other.

Comment on attachment 8626871 [details] [diff] [review] rev 2 - add test for _parseJID and normalization methods Review of attachment 8626871 [details] [diff] [review]: ----------------------------------------------------------------- This looks pretty good! A few other comments: * Please name the file "test_parseJidAndNormalization.js" (i.e. a capital "J"). * Please add a comment above _parseJid that explains what it's doing. I know you didn't write this, but the other two of these three similar functions have comments and I'd like to see one there too. ::: chat/protocols/xmpp/test/test_parsejidAndNormalization.js @@ +6,5 @@ > +let xmpp = {}; > +Services.scriptloader.loadSubScript("resource:///components/xmpp.js", xmpp); > + > +function testParseJID() { > + const testData = { Nit: Use 2 space indentation instead of tabs. @@ +51,5 @@ > + }; > + > + for (let currentJID in testData) { > + equal(xmpp.XMPPAccount.prototype.normalize(currentJID), > + testData[currentJID]); Nit: Line up testData with xmpp on the line above it. (This applies to a few places in this file.)

Comment on attachment 8626871 [details] [diff] [review] rev 2 - add test for _parseJID and normalization methods Review of attachment 8626871 [details] [diff] [review]: ----------------------------------------------------------------- To avoid duplication and to make adding additional test cases easy in the future, you could use the same test data for all three tests.

Doing a quick search for jid test cases I discovered these links which might be helpful: http://stackoverflow.com/questions/3514342/validating-an-xmpp-jid-with-python https://github.com/otalk/xmpp-jid I haven't fully read them but it seems likely the current implementations of parseJid/normalize/normalizeFullJid don't always match RFC 6122, which is surprisingly complicated. Probably worth checking * that parseJid doesn't fail for more unusual jids (e.g. with unicode characters) * whether normalize and normalizeFullJid need rewriting in order to really ensure the resulting normalized jids can be used as unique identifiers for comparisons. The second link does seem to include some tests, which should be worth a look.

(In reply to aleth [:aleth] from comment #7) > http://stackoverflow.com/questions/3514342/validating-an-xmpp-jid-with-python > https://github.com/otalk/xmpp-jid Note I'm not suggesting we use that code (or even need to), just that it might be helpful in figuring out if we are spec compliant.

(In reply to aleth [:aleth] from comment #7) > Probably worth checking > * that parseJid doesn't fail for more unusual jids (e.g. with unicode > characters) > * whether normalize and normalizeFullJid need rewriting in order to really > ensure the resulting normalized jids can be used as unique identifiers for > comparisons. I will extend test cases in next patch to be sure of everything (parseJid, normalize and normalizeFullJid).

Comment on attachment 8629109 [details] [diff] [review] rev 4 - add test for _parseJID and normalization methods Review of attachment 8629109 [details] [diff] [review]: ----------------------------------------------------------------- Looks pretty reasonable to me. Minor nit: change `testData` to `TEST_DATA` since it's a constant. r+ with that change! Also, if any of the elements in that test data are derived from a specific bug or issue, it might make sense to add a comment about that above that specific test-case.

Updating milestone from 1.6 to 42.