-
Notifications
You must be signed in to change notification settings - Fork 254
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tests: qpdf 2064 (compare images) FAILED #20
Comments
I run the compare images tests every time I release, and they passed for me with 5.0.1. Once in a while I have seen false failures of compare images tests for people who have old versions of libtiff. What version is your tiffcmp? This is also dependent on the version of ghostscript. Some versions of ghostscript might not be able to properly convert some weird PDF files. That said, I've been running these tests regularly for some time, so recent versions of both packages should work. A few additional comments. For autobuilders, I recommend adding the Ideally, to look into this, I would want to see the actual and expected images as well as the actual and expect PDF files. It doesn't look like the test suite captures these, but it wouldn't be hard to make it do so. In Is that enough information for you to go on? Finally, I would say that it is extremely likely that this failure indicates a problem with the test suite or with the underlying tools, not with qpdf itself. There have been no changes in the content preservation parts of qpdf for a very long time, and in the history of qpdf, there has never been a bug reported about qpdf failing to preserve the contents of a page assuming all the objects are properly copied. The fact that these tests can be fragile dependent on external tools and test parts of the code that are very stable and that they are more or less just an extra layer of checks since all the real content preservation is tested directly are the reasons that these tests are disabled by default. Even so, it would be interesting to see why these are failing. I don't recall having seen cases of only one compare images test failing before. The particular file with inline images in it that is in that test suite looks like it may have some errors in that may be visually depicted by some readers, so I may have to remove that particular compare images test. Thanks. |
Currently the installed versions of app-text/ghostscript is 9.10 and media-libs/tiff 4.0.3 New build log with --enable-show-failed-test-output can be found here. And the tif files: http://lasseb.tihlde.org/exherbo/tif1_a.tif Not knowing perl i did a quick sed on qpdf.test copying both files regardles of the state before they where removed copy("tif1/a.tif", "../tif1_a.tif");
copy("tif2/a.tif", "../tif2_a.tif");
system("rm -rf tif1 tif2"); |
Also fails here (on Exherbo too, amd64). app-text/ghostscript v9.10 & media-libs/tiff 4.0.3-r1 @Bruners: What would you think of a patch to temporarily disable this test in Exherbo ? |
@nbraud it actually passes the test for me now. the test passes for most people (so far only me and you with fails) so i believe we should hear what @jberkenbilt has found out first. Please upload the tif files so that they can be investigated |
Okay, I know what the problem is: it's a bug in tiffcmp. The files here are multi-page tiffs and were generated using the tiff12nc device with ghostscript. When tiffcmp is used to compare the files, it complains that the 25 page has differences in the last pixel of each scanline. The issue here is that there are only 4 bits per sample, and there are an odd number of pixels in each line. When the data is uncompressed, the last half of the last byte of each row, which is not actually part of the image, differs sometimes probably at random, and this is confusing tiffcmp. The images are in fact identical in all ways that matter. This can be verified in two different ways: use tiffsplit to split the multi-page images into individual pages and then use tiffcmp separately on each page, or use ImageMagick's convert to convert the multi-page tiff files to pdf files, use pdfimages to extract the image data, and use plain old cmp to compare them. Both techniques show the image data is identical. I will report this bug to the tiff people, but without a patch (which I don't have time to create), it will not get fixed anytime soon, and even if it did, you wouldn't have the fix in your environment right away, so you will still need a workaround. I can suggest three approaches, which I will present in decreasing difficulty (hardest first):
I maintain qpdf for debian, and I don't run image comparison tests for the debian packaging, so I would recommend option 3 as the best option. The reason that it is safe to skip image comparison tests if you're just rebuilding and not modifying the code is that there are direct tests for all the content preservation functionality, so there is no loss of test coverage by disabling the image comparison tests. They are additional tests to force something to actually process the content streams, which qpdf only does lexically, not symatically. The only thing anywhere in the qpdf codebase that potentially modifies the content stream is only used in qdf mode or when content stream normalization is enabled, and that's off by default and only turned on by people who want to manually debug or inspect PDF files. Even then, it's only changing whitespace except in the case of inline images where it's preserving whitespace. That said, there's basically no chance that something like a platform or compiler difference could cause qpdf to malfunction in a way that would make one of the image comparison tests fail without also making the direct tests that look at the actual data fail. If you were modifying the code in qpdf that works with inline images to, say, replace the image data with something, then running the image comparison tests would be important. For just packaging for a distribution, they're not important. Turning off the image comparison tests also makes the only external build dependencies of qpdf be zlib and pcre. Make sense? Let me know if I can answer any additional questions about this or provide additional clarification. |
Actually, I think image comparison tests are disabled by default. Any reason that you enabled them other than just being extra careful? |
It was enabled by @tgurr in the first version of the package. |
@jberkenbilt on Exherbo we run tests per default, but leave the option to the user to disable running tests (per package or completely system wide). When tests are disabled this also removes the need to install dependencies which are only required for running tests, so dependecy wise we have no problem. Because of this we also don't really bother how much disk space the tests need or how long they run. For the latter case we even have a special expensive_tests flag. As it helped to discover a special case/bug I tend to rather not remove the option to run the tests and go with your option 2, though only if you'll make that change upstream as we try to stay as close to upstream as possible and rather not carry distribution specific patches. |
I'm willing to implement option 2 or some other similar option that uses a different gs device that doesn't trip up on this tiffcmp bug. I don't remember how much disk space it takes to run the compare image tests, but I can tell you that those tests have been there for over 10 years, and taking up twice as much disk space as they did 10 years ago should not be a problem for anyone... I'll get this on the list for 5.0.2 which currently is planned to contain only trivial changes now including this and a one-line fix for recent clang on macos. |
If you want to run all tests, you should also enable the large file tests. They require about 15 GB and more than double the test time, but they cause qpdf to create files larger than 4 GB and do all sorts of validation to make sure that various recovery options and regular file reading/write operations work properly. The README file should provide details. |
For completeness, I'll mention that I finally got around to reporting a bug against libtiff about the comparison problems, and I referenced the tif files you mentioned above in the bug report. |
@jberkenbilt looks like this is still an issue (app-text/qpdf-5.1.2, app-text/ghostscript-9.15-r1, media-libs/tiff-4.0.3-r3)...
|
Trying to build qpdf 5.0.1 with tests enabled
Complete build logs can be found here:
If you need anything else from the build i will be happy to provide them :)
The text was updated successfully, but these errors were encountered: