Skip to content

Avoid forcing Imagick-only editor when Imagick lacks core format support#2522

Closed
Copilot wants to merge 3 commits into
trunkfrom
copilot/fix-dominant-color-images-tests
Closed

Avoid forcing Imagick-only editor when Imagick lacks core format support#2522
Copilot wants to merge 3 commits into
trunkfrom
copilot/fix-dominant-color-images-tests

Conversation

Copilot AI commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Fixes #2521

dominant-color-images tests were skipping many MIME-type cases in wp-env because the Imagick test setup always forced WP_Image_Editor_Imagick, even in environments where Imagick is loaded but reports no usable image formats. This created broad, noisy skips unrelated to the code under test.

  • Test setup guard for actual format capability
    • In Test_Dominant_Color_Image_Editor_Imagick::set_up(), the Imagick-only editor filter is now applied only when Imagick supports all required formats (JPEG, PNG, GIF, WEBP).
  • Graceful fallback behavior
    • If required formats are missing, the test no longer forces Imagick; WordPress can use whichever editor is actually supported in the environment.
  • Defensive handling of format introspection
    • Imagick::queryFormats() is wrapped in try/catch; if querying fails, the test is skipped with a clear reason instead of failing setup unexpectedly.
$required_formats = array( 'JPEG', 'PNG', 'GIF', 'WEBP' );
try {
	$supported_formats = Imagick::queryFormats();
} catch ( Exception $exception ) {
	$this->markTestSkipped( sprintf( 'Unable to query Imagick formats: %s', $exception->getMessage() ) );
}

$missing_formats = array_diff( $required_formats, $supported_formats );
if ( 0 === count( $missing_formats ) ) {
	add_filter( 'wp_image_editors', /* force WP_Image_Editor_Imagick */ );
}

Copilot AI linked an issue Jun 6, 2026 that may be closed by this pull request
@westonruter westonruter added the [Plugin] Image Placeholders Issues for the Image Placeholders plugin (formerly Dominant Color Images) label Jun 6, 2026
Copilot AI and others added 2 commits June 6, 2026 09:59
Co-authored-by: westonruter <134745+westonruter@users.noreply.github.com>
Co-authored-by: westonruter <134745+westonruter@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix issue with skipped tests for dominant-color-images plugin Avoid forcing Imagick-only editor when Imagick lacks core format support Jun 6, 2026
Copilot AI requested a review from westonruter June 6, 2026 10:04
@westonruter westonruter added the [Type] Bug An existing feature is broken label Jun 6, 2026
@westonruter westonruter marked this pull request as ready for review June 6, 2026 10:15
@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @Copilot.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Unlinked contributors: Copilot.

Co-authored-by: b1ink0 <b1ink0@git.wordpress.org>
Co-authored-by: westonruter <westonruter@git.wordpress.org>
Co-authored-by: thelovekesh <thelovekesh@git.wordpress.org>
Co-authored-by: nickchomey <nickchomey@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@westonruter westonruter added the no milestone PRs that do not have a defined milestone for release label Jun 6, 2026
@codecov

codecov Bot commented Jun 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.29%. Comparing base (b45a9d5) to head (e708b8f).

Additional details and impacted files
@@           Coverage Diff           @@
##            trunk    #2522   +/-   ##
=======================================
  Coverage   69.29%   69.29%           
=======================================
  Files          90       90           
  Lines        7723     7723           
=======================================
  Hits         5352     5352           
  Misses       2371     2371           
Flag Coverage Δ
multisite 69.29% <ø> (ø)
single 35.76% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@b1ink0

b1ink0 commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

@westonruter this is related to an issue I faced while testing the transparent AVIF issue with the Imagick editor #2245 (comment).

We use the tests-cli Docker image for testing. We could potentially switch to the tests-wordpress Docker image, which has the support. We could try this if tests-wordpress is not too heavy to run.

@westonruter

westonruter commented Jun 6, 2026

Copy link
Copy Markdown
Member

We use the tests-cli Docker image for testing. We could potentially switch to the tests-wordpress Docker image, which has the support. We could try this if tests-wordpress is not too heavy to run.

@b1ink0 Interesting. Well, are we actually using this tests-cli image anymore as of #2505?

@b1ink0

b1ink0 commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Ah that PR got merged I will update the #2245 and see if there is still issue of unsupported formats.

@b1ink0

b1ink0 commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

@westonruter Okay, I tested this and confirmed that the cli image still does not support multiple image formats when using Imagick.

For webp-uploads, 35 tests are skipped when using cli, while only 4 tests are skipped when using the wordpress image:

- "test-php:webp-uploads": "wp-env --config=.wp-env.test.json run cli --env-cwd=/var/www/html/wp-content/plugins/performance composer test:webp-uploads",
+ "test-php:webp-uploads": "wp-env --config=.wp-env.test.json run wordpress --env-cwd=/var/www/html/wp-content/plugins/performance composer test:webp-uploads",

I also tested dominant-color-images, where 24 tests are skipped with cli and only 2 are skipped when using the wordpress image (tested on trunk).

Given the difference in test coverage, I think we should use the wordpress image for the webp-uploads and dominant-color-images test suites (workflow files will also need to be updated).

@westonruter

Copy link
Copy Markdown
Member

@b1ink0 ok, great! Could you open a PR to test that?

@westonruter westonruter closed this Jun 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no milestone PRs that do not have a defined milestone for release [Plugin] Image Placeholders Issues for the Image Placeholders plugin (formerly Dominant Color Images) [Type] Bug An existing feature is broken

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Many tests being skipped for dominant-color-images plugin

3 participants