mangstadt / ez-vcard

A vCard parser library for Java
Other
399 stars 92 forks source link

Added round-trip tests #49

Closed bgorven closed 4 years ago

bgorven commented 8 years ago

Continuing the discussion from https://github.com/bgorven/ez-vcard/commit/15813248e5b833cefdc6fb53892a529ccf42a938#commitcomment-16689669

Why did you make copies of the .vcf files from the "ezcard.io.text" package? Why not use the existing files?

The vcards look different after converting them to jcard and then back. I did it this way initially just to see if doing an exact string comparison of the converted files would work.

One benefit of having output files in the repo can be that if the output format changes at any time, you can see that in your repo. But it would be just as easy to create the files in a temp dir, which would keep the repo a lot tidier.

codecov-io commented 8 years ago

Current coverage is 80.62%

Merging #49 into master will increase coverage by +0.18% as of 7850ee6

@@            master     #49   diff @@
======================================
  Files          210     210       
  Stmts         8366    8366       
  Branches      1498    1498       
  Methods                          
======================================
+ Hit           6730    6745    +15
  Partial        236     236       
+ Missed        1400    1385    -15

Review entire Coverage Diff as of 7850ee6

Powered by Codecov. Updated on successful CI builds.

bgorven commented 8 years ago

To elaborate on the second commit message, the outlook samples weren't working because, for instance,

NOTE;ENCODING=QUOTED-PRINTABLE:This is the note field!!=0D=0ASecond line=0D=0A=0D=0AThird line is empty=0D=

in the original sample was being converted to

["note",{},"text","This is the note field!!\r\nSecond line\r\n\r\nThird line is empty\r\n"]

in the jcard, which was then being written as

NOTE:This is the note field!!\nSecond line\n\nThird line is empty\n

when it was converted back to vcard format on Linux.

This was causing the test to fail when it would read the second vcard in, convert it to jcard, and do an exact string comparison with the original jcard.

mangstadt commented 8 years ago

Thank you for noticing the issue with the newlines. I understand why you need to make copies of the vCard samples now.

bgorven commented 8 years ago

I've refactored this to make it easier to add round-trip tests to other formats, and added tests to roundtrip from xcard to vcard v4.0 and from xcard to vcard v3.0 as proof of concept.

I've added a note next to each excluded test case; apart from the character encoding issues, there are a couple of things causing some of the tests to fail that aren't necessarily bugs in the library; the vcards are semantically identical but not textually identical, e.g. <parameters><type><text>JPEG... when encoded one way or <parameters><type><text>jpeg... when encoded another.

I've added a way to compare the deserialized objects using VCard.equals, which doesn't have the same issues, but for the tests that did fail it's not as easy to see why (compared to string comparisons, where you can doubleclick the failure message in Eclipse for a diff).

bgorven commented 8 years ago

There were a couple of issues relating to xml serialization: the outlook-2003 file was serialized with an &#12; entity that is apparently not valid xml, as it causes a SAX exception when trying to read that xcard in, and if you enable indentation (change getTargetWriter to return new XCardWriter(sw, 2)), the formatting of some properties gets read back in as the property text. For instance:

    <x-phonetic-first-name>
      <unknown>Grregg</unknown>
    </x-phonetic-first-name>

after converting to vcard and back, becomes

    <x-phonetic-first-name>
      <unknown>\n      Grregg\n    </unknown>
    </x-phonetic-first-name>
mangstadt commented 4 years ago

Closing due to age.

Sorry this got neglected. I wasn't comfortable merging it due to the large number of complex changes it involved.