Investigate potentially failing Imagick PDF alpha channel test

[swissspidy]

The test \Tests_Image_Editor_Imagick::test_remove_pdf_alpha_channel_should_remove_the_alpha_channel_in_preview() was introduced in #39216 / [56271].

On some hosting providers, this test appears to be failing with errors such as this:

Tests_Image_Editor_Imagick::test_remove_pdf_alpha_channel_should_remove_the_alpha_channel_in_preview
The intermediate size could not be retrieved.
Failed asserting that false is of type "array".

/tmp/wp-test-runner/tests/phpunit/tests/image/editorImagick.php:680

Examples:

​https://make.wordpress.org/hosting/test-results/r57849/wpsabot-r57849/
​https://make.wordpress.org/hosting/test-results/r57848/wetopibot-r57848/

We should investigate this failure to see whether it's an issue in core or with the hosting provider.

The test mentions "Ghostscript version >= 9.14", so maybe it's just a matter of skipping the test if the installed Ghostscript version (gs --version I think) is older than that.

[swissspidy]

cc @antpb @javiercasares

[swissspidy]

tl;dr from the above Slack discussion:

\WP_Image_Editor_Imagick::remove_pdf_alpha_channel is currently checking the ImageMagick version, but it looks like we need to check the Imagick version via phpversion( 'imagick' ).

So maybe even as simple as version_compare( phpversion( 'imagick' ), '9.14', '>=' ) would work.

[antpb]

I've attached a patch. I'm pretty certain the version check is checking ImageMagick and not the correct imagick version. From the php docs it says the function being used currently only returns the ImageMagick version: ​https://www.php.net/manual/en/imagick.getversion.php

Imagick::getVersion — Returns the ImageMagick API version

We should just directly do the phpversion() call to the module to get the version. We could use some testing from hosts seeing the test errors.

Prior art also: ​https://github.com/WordPress/wordpress-develop/blob/4f9b520d3e2531c9ffb9ee5c06a2e2a7e34f9c52/src/wp-admin/includes/class-wp-debug-data.php#L540

For manual testing to ensure this patch doesnt result in a regression please repeat the report here to ensure it is still fixed with this new version check: https://core.trac.wordpress.org/ticket/39216

[joedolson]

This looks good to me; it'll definitely help to be checking the right package! ;)

[antpb]

In 58415:

Media: Use version_compare() for Imagick version check when removing alpha.

Previously remove_pdf_alpha_channel() used Imagick::getVersion() to validate the environment is capable of handling alpha. This was the incorrect function to use to check the module version as it will only provide the ImageMagick API version. This patch adjusts to instead use phpversion() as this is the correct method to get the Imagick version needed to determine alpha compatibility. This fixes a number of host tests that have been correctly failing on subsets of environments. Serendipidously, sometimes the API version was high enough to avoid shining light on this problem.

Props swissspidy, joedolson, antpb.
Fixes #60798.

[antpb]

In 58417:

Media: Revert r58415 Use version_compare() for Imagick version check when removing alpha.
This commit is a clean revert of r58415 as tests seem to now be failing with this change. Likely the test needs an update as well to accomodate the new successful version check.
Follow-up to [60798].
Props jorbin, hellofromTonya.
See #60798.

[oglekler]

Moving this ticket to the next milestone due to RC1 coming.

[sippis]

We just started setting up our automated test reporter and ran into this issue. See ​https://make.wordpress.org/hosting/test-results/r58902/evermadebot-r58902/

Read the history and looked into the reverted patch.

<?php phpversion( 'imagick' )

reports 3.7.0 which seems to be the latest stable as per ​PECL so checking against 9.14 feels a bit odd to me. Maybe I'm missing something?

[swissspidy]

3.7.0 is just the version of the imagick extension, whereas 9.14 is the version of the ImageMagick library used by imagick. Two different things.

If we indeed need to check imagick version, then yes 9.14 is the wrong version to compare to there.

[chaion07]

Thanks @swissspidy for reporting this. As we move closer to the RC1 for 6.7 in a matter of days we want to possibly bump the milestone following the feedback received from a recent bug-scrub session. We'll revisit this Ticket in a few days.

Cheers!