search

brucemiller / LaTeXML

LaTeXML: a TeX and LaTeX to XML/HTML/ePub/MathML translator.
http://dlmf.nist.gov/LaTeXML/
Other
961 stars 101 forks source link

test if ImageMagick policy.xml allows PDF conversion #2195

Closed dginev closed 1 year ago

dginev commented 1 year ago

Fixes #1216 .

The consensus resolution was to try and alert LaTeXML users that the policy.xml of ImageMagick needs to be edited for fully featured image conversion.

This PR adds a unit test which detects the situation and warns during make test, without treating it as a Failure case, as image conversion is still considered optional. In the case where Image::Magick is properly installed and configured, the test checks that the created PNG matches an expected size.

Open to suggestions for making both the warning language, as well as the test conditions, more generally useful.

P.S. currently CI will run and skip the test, as we don't install the image-related dependencies.

brucemiller commented 1 year ago

So, the end result of this is that if image magic has the bad policy, you can't even install LaTeXML (from the perspective of the average user)? Even if you never intend to convert pdf images?

dginev commented 1 year ago

As I say in the description:

This PR adds a unit test which detects the situation and warns during make test, without treating it as a Failure case

The test never fails, it only prints a diagnostic message (and succeeds) in the case where the PDF-to-PNG conversion fails, while the dependency was installed.

brucemiller commented 1 year ago

Ah, OK, that's reasonable. There are somewhat similar concerns dealt with in Makefile.PL (eg. connecting to TeX, etc), but it's probably too early, and too painful, to do this test there?

dginev commented 1 year ago

Right, Makefile.PL is a little early (and feels a bit too custom a concern). The testing framework offers a nice abstraction for skipping and diagnostics that felt like a good fit.

And we can also think towards adding more unit testing of image conversion functionality in the long run by just extending that new test file, which felt like a nice bonus...

brucemiller commented 1 year ago

Well, it's ugly & horrible, but what choice...?

brucemiller commented 1 year ago

Hmm... just did a git pull, etc and got

t/003_unit_imagemagick.t .. 2/? 
#   Failed test 'PDF to PNG conversion did not produce an image of the expected size'
#   at t/003_unit_imagemagick.t line 23.
#          got: '2457'
#     expected: '2557'
# Looks like you failed 1 test of 2.
t/003_unit_imagemagick.t .. Dubious, test returned 1 (wstat 256, 0x100)

which isnt' quite what I was hoping for.

dginev commented 1 year ago

I would have hoped the size was reproducible... Sigh. I guess this condition should be a simple >0