manuelbl / SwissQRBill.NET

.NET library for Swiss QR bill payment slips (aka QR-Rechnung)
MIT License
88 stars 33 forks source link

Convert tests to use Verify.ImageMagick for QR-bills verification #35

Closed 0xced closed 3 years ago

0xced commented 3 years ago

Using Assert.Equal on byte arrays is fine as long as the tests pass. As soon as one test fails, it becomes unusable because of its useless message:

Xunit.Sdk.EqualException Assert.Equal() Failure Expected: Byte[] [37, 80, 68, 70, 45, ...] Actual: Byte[] [37, 80, 68, 70, 45, ...]

Something has changed in the way compression works in .NET Core 2 and .NET Core 3 and all PDF tests were failing because of the byte-by-byte comparison of PDF files.

Using Verify.ImageMagick instead is much better as it performs a real image comparison (with ImageMagick under the hood).

Note that all tests are now passing on both Windows and macOS (I haven't run them on Linux).

manuelbl commented 3 years ago

Thanks a lot for the contribution. I've opened a "v3" branch and merged most of it.

I don't use the ImageMagick comparison for PDF files as it requires Ghostscript to be installed. That prerequisite causes more pain than the image comparison solves. On further investigation of the PDF differences it also turned out that most of them were caused by the changes in number formatting ("-0" for very small negative numbers).

I didn't merge the "PNG fix" either as I couldn't reproduce the problem. GraphicsUnit.Point is a resolution independent unit. And as the image resolution is explicitly set, it should just work fine. Or do you have a test case for reproducing the problem?

0xced commented 3 years ago

I didn't merge the "PNG fix" either as I couldn't reproduce the problem. GraphicsUnit.Point is a resolution independent unit. And as the image resolution is explicitly set, it should just work fine. Or do you have a test case for reproducing the problem?

Sorry, I failed to explain properly why this was needed. This fix is required on macOS (and maybe also Linux) where the System.Drawing.Common package uses libgdiplus under the hood which is probably buggy with regard to the GraphicsUnit. By the way this is one of the reason why System.Drawing.Common will be deprecated: nobody wants to maintain libgdiplus.

Without the PNG fix on macOS: PNGCanvasTest PngBillQrBill received-macos

With the PNG fix on macOS: PNGCanvasTest PngBillQrBill received-macos-with-fix

By the way, it would be nice to setup GitHub actions to automatically run the tests on all supported platforms: Linux + macOS + Windows. I have already done this for several projects and I can configure it on this repository too.

0xced commented 3 years ago

I don't use the ImageMagick comparison for PDF files as it requires Ghostscript to be installed. That prerequisite causes more pain than the image comparison solves. On further investigation of the PDF differences it also turned out that most of them were caused by the changes in number formatting ("-0" for very small negative numbers).

There's also another issue because of compressed streams. I have opened #36 which explains the issue of byte-by-byte comparison and adresses it by converting the PDF documents to PNG without the Ghostscript requirement.

0xced commented 3 years ago

Closing since all remaining issues have been addressed.

PDF comparison in #36 and PNG rendering on Linux and macOS in #39.