search

joelverhagen / NCsvPerf

A test bench for various .NET CSV parsing libraries
https://www.joelverhagen.com/blog/2020/12/fastest-net-csv-parsers
MIT License
69 stars 14 forks source link

Added benchmark for CommonLibrary.NET #24

Closed JoshClose closed 3 years ago

JoshClose commented 3 years ago

I don't get the failure locally.

image

joelverhagen commented 3 years ago

Works locally for me too. I'll merge and figured out what's up.

joelverhagen commented 3 years ago

Okay, it looks like the CI was running on ubuntu-latest and I presume we are both using Windows. Probably line ending issues.

JoshClose commented 3 years ago

Does that mean your tests need to specify the line ending? Libraries should be able to to handle any type of newline, but tests should also be consistent across platforms.

joelverhagen commented 3 years ago

Does that mean your tests need to specify the line ending?

Yes 😞. I had to make this change. Previously it was using Environment.NewLine which yielded \n on Ubuntu leading to the test failure.

https://github.com/joelverhagen/NCsvPerf/blob/f59ab5ba27d4b9e1c3a4f3a4477b02f0bb801b37/NCsvPerf/TestData/TestData.cs#L69-L72

Libraries should be able to to handle any type of newline, but tests should also be consistent across platforms.

I looked through the CommonLibrary.NET source code on CodePlex and found code detecting newline by explicitly checking for CR LF (\r then \n). This means it does not handle any type of newline.

As a side not the SEA.CommonLibrary.NET package is compiled with Debug configuration. I'm considering forking the source code onto GitHub myself and publishing my own copy with at least Release configuration and perhaps even a minimal change to handle files with just LF.

JoshClose commented 3 years ago

Nice. What a good open source citizen.

joelverhagen commented 3 years ago

Done.

GitHub | NuGet

LF is supported and it's built with Release configuration.

JoshClose commented 3 years ago

Awesome! In your NuGet description you might want to mention that your version supports LF and is in Release config.