Investigate failing Imagick tests on PHP 8.3 and 8.4

[johnbillion]

Background: #63876

Fixing the self-signed MySQL certificate issue in the PHP 8.3 and 8.4 container images that are based on Debian Trixie surfaces two errors and two failures in the Imagick tests.

​Failing GitHub Actions workflow run can be found here.

There were 2 errors:

1) Tests_Image_Editor_Imagick::test_image_max_bit_depth
ImagickException: no encode delegate for this image format `AVIF' @ error/constitute.c/WriteImage/1412

/var/www/tests/phpunit/tests/image/editorImagick.php:726
/var/www/vendor/bin/phpunit:122

2) Test_Image_Resize_Imagick::test_resize_avif
TypeError: getimagesize(): Argument #1 ($filename) must be of type string, WP_Error given

/var/www/src/wp-includes/media.php:5736
/var/www/tests/phpunit/tests/image/resize.php:107
/var/www/vendor/bin/phpunit:122

There were 2 failures:

1) Tests_Image_Editor_Imagick::test_resizes_are_small_for_16bit_images with data set "test8" ('/var/www/tests/phpunit/includ...t8.png')
The resized image file size is not smaller than the original file size.
Failed asserting that 99853 is less than 45231.

/var/www/tests/phpunit/tests/image/editorImagick.php:794
/var/www/vendor/bin/phpunit:122

2) Tests_Image_Editor_Imagick::test_png_color_type_is_preserved_after_resize with data set "test8_color_type_3" ('/var/www/tests/phpunit/includ...t8.png', 3)
The PNG original color type should be preserved after resize for /var/www/tests/phpunit/includes/../data/images/png-tests/test8.png.
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'3'
+'4'

/var/www/tests/phpunit/tests/image/editorImagick.php:848
/var/www/vendor/bin/phpunit:122

In addition a risky test was flagged which also covers AVIF handling. It may or may not be related and should be investigated.

There was 1 risky test:

1) Tests_Media::test_quality_with_avif_conversion_file_sizes
This test did not perform any assertions

/var/www/tests/phpunit/tests/media.php:5473

/var/www/vendor/bin/phpunit:122

This temporarily disables failing Imagick tests prior to investigation.

Adds some missing WP_Error assertions too which means an error will be a failure instead.

Branches from #9746

[SirLouen]

@johnbillion for the first two, it seems that AVIF is supported in the new ImageMagick version, but no is not being added as a delegate
​https://www.imagemagick.org/discourse-server/viewtopic.php?p=161544&sid=fc7993f28ef8acbc600eb4e4996d8133#p161544

Wonder if it was not skipping these two tests before, and never been tested.

About the other two tests, looks like a new version in ImageMagick. I wrote those tests some months ago and I remember we were having trouble with that topic because Imagick was inconsistent among versions.
Do you remember, @siliconforks? I will take a look tomorrow to see those inconsistencies with the color types, they were a headache (wonder if they have changed the codes in the new ImageMagick version).

For the last one skipped, the same as the first two. It seems that its not being skipped, but probably the sizes_to_compare array is empty leading into 0 tests.

[jorbin]

Noting that the version of ImageMagick on the passing tests is 6.9.11-60 while the version in the failing version is 7.1.1-43. ​https://imagemagick.org/script/porting.php#imv7 has some information on the move from 6 to 7 and might help identify some things that could help here.

[siliconforks]

Replying to SirLouen:

About the other two tests, looks like a new version in ImageMagick. I wrote those tests some months ago and I remember we were having trouble with that topic because Imagick was inconsistent among versions.
Do you remember, @siliconforks? I will take a look tomorrow to see those inconsistencies with the color types, they were a headache (wonder if they have changed the codes in the new ImageMagick version).

Well, the problem is presumably with ImageMagick version 7 (as noted in this comment), but I've never used that version so I don't know what the problem is.

I just ran all of the above tests on a machine with Ubuntu 24.04, which has PHP 8.3 but with ImageMagick 6.9, and all 5 tests are passing.

[SirLouen]

@johnbillion this is my proposal for now

1. Keeping back ImageMagick 7 for now

​https://github.com/WordPress/wpdev-docker-images/pull/184/

2. Create a new ticket to solve all the ImageMagick 7. I thought it was a single problem, but this is widespread. This may take time, there are major breaking changes that might need time in several places.

Trac ticket: https://core.trac.wordpress.org/ticket/63932

[siliconforks]

​PR #9775 is intended as a suggested (partial) alternative to ​PR #9749 for the PNG-related failing tests.

This PR has the following advantages:

1. It checks the ImageMagick version rather than the PHP version because it is really ImageMagick version 7 that is the problem.

2. It only skips the one data file that is causing test failures (​test8.png). The other data files should work fine with ImageMagick 7.

Note that this PR addresses only the PNG-related failing tests and not the AVIF-related ones, because I don't think the AVIF failures are caused by ImageMagick 7 (I suspect that the problem with those is something else).

[SirLouen]

Replying to siliconforks:

1. It checks the ImageMagick version rather than the PHP version because it is really ImageMagick version 7 that is the problem

I've been trying to figure out the changes regarding alpha and greyscale in IM7 but have not been able to get to a nice conclusion. Do you think its totally impossible now to compress greyscale alpha-1 images?

[siliconforks]

Replying to SirLouen:

I've been trying to figure out the changes regarding alpha and greyscale in IM7 but have not been able to get to a nice conclusion. Do you think its totally impossible now to compress greyscale alpha-1 images?

I'm not sure - ImageMagick 7 changed a lot of things so it's difficult to tell whether it's still possible to get the information needed from the public API.

I've created a post in the ImageMagick discussions in an attempt to get some clarification on this:

​https://github.com/ImageMagick/ImageMagick/discussions/8348

I think I prefer the approach used in #9749 since it makes use of an ​official annotation in PHPUnit .

The problem with ​PR #9749 is that it will skip the tests test_resizes_are_small_for_16bit_images and test_png_color_type_is_preserved_after_resize on a system with PHP 8.3 or 8.4 and ImageMagick 6. But there's no need for this - the tests pass on that system. (I've tested this on Ubuntu 24.04 LTS, which has PHP 8.3 and ImageMagick 6.)

I would guess that the reverse problem can happen too (although I have not tested this) - you could have a system with PHP 8.2 (or an earlier version) and ImageMagick 7, and then the tests would fail.

The problem with ​https://github.com/WordPress/wordpress-develop/pull/9749 is that it will skip the tests test_resizes_are_small_for_16bit_images and test_png_color_type_is_preserved_after_resize on a system with PHP 8.3 or 8.4 and ImageMagick 6. But there's no need for this - the tests pass on that system. (I've tested this on Ubuntu 24.04 LTS, which has PHP 8.3 and ImageMagick 6.)

I know it's good to test as many combinations as possible, but are there actually any documented differences between how ImageMagick 6.x works on PHP 7.2-8.0 that would be inconsistent with PHP >= 8.1?

I would guess that the reverse problem can happen too (although I have not tested this) - you could have a system with PHP 8.2 (or an earlier version) and ImageMagick 7, and then the tests would fail.

The commit that changed the underlying base image for the official PHP images was made to PHP 8.1-8.5. The 8.1 & 8.2 containers have not yet been rebuilt and published because that only happens when an update is released for that version of PHP. 8.1 & 8.2 are currently only receiving security updates, so it's not known when (or if) a new build will be published. But when that does happen, the base image will then be the same as the 8.3 & 8.4 ones currently with ImageMagick 7.

I know it's good to test as many combinations as possible, but are there actually any documented differences between how ImageMagick 6.x works on PHP 7.2-8.0 that would be inconsistent with PHP >= 8.1?

Probably not.

The commit that changed the underlying base image for the official PHP images was made to PHP 8.1-8.5. The 8.1 & 8.2 containers have not yet been rebuilt and published because that only happens when an update is released for that version of PHP. 8.1 & 8.2 are currently only receiving security updates, so it's not known when (or if) a new build will be published. But when that does happen, the base image will then be the same as the 8.3 & 8.4 ones currently with ImageMagick 7.

That's basically the sort of situation my PR is intended to address. If you have a system with PHP 8.1 or 8.2 and ImageMagick 6, but you later update ImageMagick to version 7, the code will detect that (because it just checks the ImageMagick version) and skip the problematic tests.

I've merged this into #9749, thanks. Happy to use an exclusion which is more targeted, and hopefully it won't be in place for long.

[johnbillion]

In 60736:

Media: Temporarily disable failing tests when Imagick 7 is in use, pending further investigation.

The updated PHP 8.4 and 8.3 containers are running Imagick 7 which is producing some test failures for AVIFs and PNGs with 1-bit transparency. This requires further investigation, possibly accompanied by more comprehensive testing across Imagick versions, so these tests are disabled for now.

Additional missing assertions have also been added which ensure an unexpected WP_Error instance correctly fails the test and is not passed to an image processing function.

Props johnbillion, siliconforks, desrosj, jorbin.

See #63932

https://core.trac.wordpress.org/changeset/60736

[johnbillion]

In 60737:

Build/Test Tools: Configure PHPUnit to fail on risky tests.

There should be no need for a risky test to go unseen. It usually signifies a problem in the associated test that needs to be addressed.

See #63167, #63932

[desrosj]

[60736] needs to be backported along with [60735].

[desrosj]

In 60744:

Build/Test Tools: Fix tooling and PHPUnit test issues.

This fixes the PHPUnit test suite for the 6.8 branch by:

• Fixing issues with the env:install script when using newer PHP images.
• Temporarily disables failing tests with ImageMagick 7 is in use.

Backports [60735] and [60736] to the 6.8 branch.

See #63876, #63932.

[desrosj]

In 60745:

Build/Test Tools: Fix tooling and PHPUnit test issues.

This fixes the PHPUnit test suite for the 6.7 branch by:

• Fixing issues with the env:install script when using newer PHP images.
• Temporarily disables failing tests with ImageMagick 7 is in use.

Backports [60735] and [60736] to the 6.7 branch.

See #63876, #63932.

[desrosj]

In 60746:

Build/Test Tools: Fix tooling and PHPUnit test issues.

This fixes the PHPUnit test suite for the 6.6 branch by:

• Fixing issues with the env:install script when using newer PHP images.
• Temporarily disables failing tests with ImageMagick 7 is in use.

Backports [60735] and [60736] to the 6.6 branch.

See #63876, #63932.

[desrosj]

In 60747:

Build/Test Tools: Fix tooling and PHPUnit test issues.

This fixes the PHPUnit test suite for the 6.5 branch by:

• Fixing issues with the env:install script when using newer PHP images.
• Temporarily disables failing tests with ImageMagick 7 is in use.

Backports [60735] and [60736] to the 6.5 branch.

See #63876, #63932.

[desrosj]

In 60749:

Build/Test Tools: Fix tooling and PHPUnit test issues.

This fixes the PHPUnit test suite for the 6.4 branch by:

• Fixing issues with the env:install script when using newer PHP images.
• Temporarily disables failing tests with ImageMagick 7 is in use.

Backports [60735] and [60736] to the 6.4 branch.

See #63876, #63932.

[wildworks]

I'd like to check the status of this ticket. Are the remaining tasks to fix the tests on the trunk branch, specifically 6.9?

[SirLouen]

Replying to wildworks:

I'd like to check the status of this ticket. Are the remaining tasks to fix the tests on the trunk branch, specifically 6.9?

They are already merged in trunk and backported, not sure why it's still open
cc @desrosj

[siliconforks]

Replying to SirLouen:

Replying to wildworks:

I'd like to check the status of this ticket. Are the remaining tasks to fix the tests on the trunk branch, specifically 6.9?

They are already merged in trunk and backported, not sure why it's still open
cc @desrosj

The tests have been disabled for the specific platform that was having problems (with ImageMagick 7), so they are no longer failing, but the underlying issue is still there.

That's probably not something that's going to get resolved before WordPress 6.9 is released, though.

[wildworks]

That's probably not something that's going to get resolved before WordPress 6.9 is released, though.

I see, let's punt it to version 7.0. If there's nothing else to do, feel free to close it.