josch / img2pdf

Go to a project
Toggle navigation
Toggle navigation pinning
Issue #85 opened by

Some 0.4.0 test failures on Fedora

So I've bumped the Fedora package to 0.4.0. AFAICS, the test suite switched to pytest although a test shell script is still included.

However, I execute the tests now like this:

PYTHONPATH=src python3 -m pytest src/img2pdf_test.py

Some tests fail because of the 'endianess' vs. 'endianness' key difference in ImageMagick - I think this was previously fixed in test.sh. FWIW, ImageMagick on Fedora >= 31 uses 'endiannes'. I quick fixed this with:

$ cat test-byteorder.diff 
--- a/src/img2pdf_test.py 11:09:08.279377606 +0200
+++ b/src/img2pdf_test.py   2020-09-20 11:15:42.996704883 +0200
@@ -835,6 +835,12 @@
     yield tmp_palette8_png
     tmp_palette8_png.unlink()

+def get_byteorder(identify):
+    r = identify[0]["image"].get("endianess")
+    if r is None:
+       return identify[0]["image"].get("endianness")
+    return r
+

 @pytest.fixture(scope="session")
 def jpg_img(tmp_path_factory, tmp_normal_png):
$ patch -p1 < test-byteorder.diff
$ sed -i 's/assert identify\[0\]\["image"\]\.get("endianess")/assert get_byteorder(identify)/' src/img2pdf_test.py

Another problem is the location of the sRGB.icc some tests use now. On Fedora it's located elsewhere. Thus, I quick fixed (?) it with:

sed -i 's@"/usr/share/color/icc/sRGB.icc"@"/usr/share/color/icc/colord/sRGB.icc"@' src/img2pdf_test.py

Unfortunately, after there are still 3 failing tests (9 errors) - on x86_64:

Edited
  • josch
    @josch commented

    So I've bumped the Fedora package to 0.4.0. AFAICS, the test suite switched to pytest although a test shell script is still included.

    Yes, that's an oversight and has been fixed in c4fb1d88

    Some tests fail because of the 'endianess' vs. 'endianness' key difference in ImageMagick

    Yes, there is some difference in the -verbose output depending on the version which was fixed. Since I didn't want to have these kind of problems in the future, I switched from parsing -verbose which is meant to be human readable to the json output, which is meant to be machine readable and is "guaranteed not to change other than the possibility of additional attributes": https://github.com/ImageMagick/ImageMagick6/issues/90#issuecomment-668281074

    Unfortunately, there was another bug in the json output which was quickly fixed: https://github.com/ImageMagick/ImageMagick6/issues/90#issuecomment-682266441

    So the correct fix, is to query the json version and then choose to retrieve either endianess or endianness -- I still have to make a commit for that.

    The temporary fix for Debian is: https://sources.debian.org/src/img2pdf/0.4.0-1/debian/patches/endianess-endianness.patch/

    Another problem is the location of the sRGB.icc some tests use now.

    Yes, the tests should not rely on the icc profiles shipped by the OS. This is solved by creating our own ICC profiles in 11907242.

    Unfortunately, after there are still 3 failing tests

    At least the failures are now easier to read than they were before with the shell script. :)

    The problem with icc.png is probably, that the sRGB.icc shipped by fedora has a different checksum than the one shipped in Debian and thus the resulting in a different checksum. The problem is fixed by 11907242 as well, because no external icc profile is needed anymore.

    The next failure is the difference between depth and baseDepth which in Debian I fixed like this:

    https://sources.debian.org/src/img2pdf/0.4.0-1/debian/patches/imdepth.patch/

    The last failure is a wrong ghostscript rendering of the ccitt encoded pdf. I have no clue about that one. Maybe it's a ghostscript problem? Then it'll probably pop up in Debian soon as well.

  • Georg Sauthoff
    @gms commented

    Ok, I've added your imdepth.patch and I'm skipping test_png_icc and test_tiff_ccitt_nometa2 for this release.

    With this release I'm also adding pikepdf as hard dependency. I understand that pdfrw is deprecated and that pikepdf is the future.

    Previously, the fedora package didn't depend on pdfrw - which is also ok, because img2pdf also has built-in support for PDF writing.

    However, AFAICS, you are basically recommending pikepdf over the internal PDF engine because it offers additional features, such as better compression. Is this a fair summary?

  • josch
    @josch commented

    Yes, that's part of the reason. The additional features are stuff like being able to produce linearized pdfs (fast web view) or the better compression you mentioned. The internal PDF engine will never have these features. The test suite makes sure that both, the pikepdf as well as the internal engine produce pdf files that render equally. My reason to keep the internal engine nevertheless are:

    • pure Python (some platforms might not have extra modules available)
    • safe fallback if pdf libraries vanish (pdfrw already got abandoned)
    • forces the codebase to remain modular (maybe there will be better pdf libraries in the future)

    For distributions like Fedora or Debian, there is no reason not to add a hard dependency on pikepdf, because the library is readily available in the respective repos.