Fix unit tests for multisite #1327

westonruter · 2024-07-01T20:11:50Z

Summary

Relevant technical choices

westonruter · 2024-07-02T21:11:14Z

Interesting. There are PHPUnit test failures in most PHP versions related to Image Placeholders. I don't get this error locally. Instead I get a fatal error:

PHP Fatal error: Cannot declare class Perflab_Server_Timing_Metric, because the name is already in use in /var/www/html/wp-content/plugins/performance/plugins/performance-lab/includes/server-timing/class-perflab-server-timing-metric.php on line 14

mukeshpanchal27 · 2024-07-03T08:38:49Z

@westonruter The current trunk doesn't work for multisite unit tests; it shows No tests executed! for all plugins. Check https://github.com/WordPress/performance/actions/runs/9749428903/job/26906551079.

Additionally, I debugged the unit test failures for the current PR and found that tiff and bmp are not part of the supported mime types for multisite. However, support for these has been added in WP 6.6, as seen in changeset 58400. These changes are already in trunk, so https://github.com/WordPress/performance/actions/runs/9750444816/job/26909889640?pr=1327 has passed ✅.

Additionally, I occasionally encounter a PHP Fatal error: Cannot declare class Perflab_Server_Timing_Metric error when running the unit tests. However, this issue is intermittent and I cannot consistently reproduce it.

mukeshpanchal27 · 2024-07-03T10:35:52Z

If we want to solve the unit tests issue then we have to skip test_get_dominant_color_invalid test for multisite.

$mime_type = wp_check_filetype( $image_path )['type'];
// Old WP does not support ".tiff" and ".bmp" so return false.
if ( ! $mime_type ) {
	$this->markTestSkipped( "Mime type is not supported." );
}
if ( ! wp_image_editor_supports( array( 'mime_type' => $mime_type ) ) ) {
	$this->markTestSkipped( "Mime type $mime_type is not supported." );
}

swissspidy · 2024-07-03T11:51:39Z

The current trunk doesn't work for multisite unit tests; it shows No tests executed! for all plugins

That's why this PR was opened, to fix this :-)

If we want to solve the unit tests issue then we have to skip test_get_dominant_color_invalid test for multisite.

Sounds reasonable 👍

westonruter · 2024-07-03T17:12:51Z

If we want to solve the unit tests issue then we have to skip test_get_dominant_color_invalid test for multisite.

@mukeshpanchal27 Would you amend this PR with your suggestion?

github-actions · 2024-07-04T04:28:48Z

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.

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

Co-authored-by: westonruter <westonruter@git.wordpress.org>
Co-authored-by: mukeshpanchal27 <mukesh27@git.wordpress.org>
Co-authored-by: thelovekesh <thelovekesh@git.wordpress.org>
Co-authored-by: swissspidy <swissspidy@git.wordpress.org>

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

composer.json

bin/generate-multisite-phpunit.php

.gitignore

tools/phpcs/phpcs.ruleset.xml

thelovekesh · 2024-07-04T08:03:05Z

plugins/dominant-color-images/tests/data/class-testcase.php

+		$mime_type = wp_check_filetype( $image_path )['type'];
+		// Old WP does not support ".tiff" and ".bmp" so return false.
+		if ( ! $mime_type ) {
+			$this->markTestSkipped( 'Mime type is not supported.' );
+		}
+		if ( ! wp_image_editor_supports( array( 'mime_type' => $mime_type ) ) ) {
+			$this->markTestSkipped( "Mime type $mime_type is not supported." );
+		}


This was discovered previously as well when I added support for running PHPUnit tests using a custom setup.

As a fix, I didn't skip these tests rather I added tiff and bmp in supported mime types in multisite.

/** * Setup before class. */ public static function wpSetUpBeforeClass() { // Setup site options if it's a multisite network. if ( is_multisite() ) { $site_exts = explode( ' ', get_site_option( 'upload_filetypes', 'jpg jpeg png gif' ) ); // Add `tiff` and `bmp` to the list of allowed file types. // These are removed by default in multisite. $site_exts[] = 'tiff'; $site_exts[] = 'bmp'; update_site_option( 'upload_filetypes', implode( ' ', $site_exts ) ); } }

See https://github.com/WordPress/performance/pull/1065/files#diff-670d6c64aed0f0b20a7a5971711c80e3d2b739a57e12b41044ee268010f01a13

I'm not in favor of adding support in multisite as it's a hacky solution. If WordPress itself doesn't support it, we should skip those tests to accurately reflect the lack of support in multisites.

As of WordPress 6.6, support for these features has been added in core, as seen in changeset 58400. These changes are already in trunk, so the tests are passing as seen in this run ✅.

From 6.6 onwards, the tests will work without needing to be skipped.

I wouldn't call it a hacky solution, as WordPress allows this extensibility of adding more mime types. Also, since tests are running with these MIME types on a single site, testing them on a multisite setup looks good to me. However, it was just a suggestion, and I'm perfectly fine with either approach.

Yes, we can go with either approach.

I think the unit tests for dominant-color-images need to be revisited. The valid and invalid MIME type tests seem to be skipped somehow, as they are meant to test functionality based on the MIME type. We should ensure these tests are correctly executed to verify the implementation accurately.

Single site skipped tests

There were 24 skipped tests:

1) Test_Dominant_Color_Image_Editor_GD::test_get_dominant_color_invalid with data set "tiff" ('/var/www/html/wp-content/plug...e.tiff') Mime type image/tiff is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:184 2) Test_Dominant_Color_Image_Editor_GD::test_get_dominant_color_invalid with data set "bmp" ('/var/www/html/wp-content/plug...ge.bmp') Mime type image/bmp is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:184 3) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "animated_gif" ('/var/www/html/wp-content/plug...ed.gif', array('874e4e', '864e4e', 'df7f7f'), true) Mime type image/gif is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:1[56](https://github.com/WordPress/performance/actions/runs/9812767245/job/27097470441?pr=1327#step:9:57) 4) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "red_jpg" ('/var/www/html/wp-content/plug...ed.jpg', array('ff0000', 'fe0000'), false) Mime type image/jpeg is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:156 5) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "green_jpg" ('/var/www/html/wp-content/plug...en.jpg', array('00ff00', '00ff01', '02ff01'), false) Mime type image/jpeg is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:156 6) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "white_jpg" ('/var/www/html/wp-content/plug...te.jpg', array('ffffff'), false) Mime type image/jpeg is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:156 7) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "red_gif" ('/var/www/html/wp-content/plug...ed.gif', array('ff0000'), false) Mime type image/gif is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:156 8) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "green_gif" ('/var/www/html/wp-content/plug...en.gif', array('00ff00'), false) Mime type image/gif is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:156 9) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "white_gif" ('/var/www/html/wp-content/plug...te.gif', array('ffffff'), false) Mime type image/gif is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:156 10) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "trans_gif" ('/var/www/html/wp-content/plug...ns.gif', array('5a5a5a', '020202'), true) Mime type image/gif is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:156 11) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "red_png" ('/var/www/html/wp-content/plug...ed.png', array('ff0000'), false) Mime type image/png is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:156 12) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "green_png" ('/var/www/html/wp-content/plug...en.png', array('00ff00'), false) Mime type image/png is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:156 13) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "white_png" ('/var/www/html/wp-content/plug...te.png', array('ffffff'), false) Mime type image/png is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:156 14) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "trans_png" ('/var/www/html/wp-content/plug...ns.png', array('000000'), true) Mime type image/png is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:156 15) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "red_webp" ('/var/www/html/wp-content/plug...d.webp', array('ff0000'), false) Mime type image/webp is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:156 16) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "green_webp" ('/var/www/html/wp-content/plug...n.webp', array('00ff00'), false) Mime type image/webp is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:156 17) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "white_webp" ('/var/www/html/wp-content/plug...e.webp', array('ffffff'), false) Mime type image/webp is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:156 18) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "trans_webp" ('/var/www/html/wp-content/plug...s.webp', array('000000'), true) Mime type image/webp is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:156 19) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "balloons_webp" ('/var/www/html/wp-content/plug...s.webp', array('c1bbb9', 'c0bbb9', 'c0bab8', 'c3bdbd', 'bfbab8'), false) Mime type image/webp is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:156 20) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "half_opaque" ('/var/www/html/wp-content/plug...ue.png', array('7e7e7e'), true) Mime type image/png is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:156 21) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_invalid with data set "tiff" ('/var/www/html/wp-content/plug...e.tiff') Mime type image/tiff is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:1[84](https://github.com/WordPress/performance/actions/runs/9812767245/job/27097470441?pr=1327#step:9:85) 22) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_invalid with data set "bmp" ('/var/www/html/wp-content/plug...ge.bmp') Mime type image/bmp is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:184 23) Test_Dominant_Color::test_get_dominant_color_invalid with data set "tiff" ('/var/www/html/wp-content/plug...e.tiff') Mime type image/tiff is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:184 24) Test_Dominant_Color::test_get_dominant_color_invalid with data set "bmp" ('/var/www/html/wp-content/plug...ge.bmp') Mime type image/bmp is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:184

Multisite site skipped tests

There were 24 skipped tests:

1) Test_Dominant_Color_Image_Editor_GD::test_get_dominant_color_invalid with data set "tiff" ('/var/www/html/wp-content/plug...e.tiff') Mime type is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:181 2) Test_Dominant_Color_Image_Editor_GD::test_get_dominant_color_invalid with data set "bmp" ('/var/www/html/wp-content/plug...ge.bmp') Mime type is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:181 3) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "animated_gif" ('/var/www/html/wp-content/plug...ed.gif', array('874e4e', '864e4e', 'df7f7f'), true) Mime type image/gif is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:1[56](https://github.com/WordPress/performance/actions/runs/9812767245/job/27097470441?pr=1327#step:10:57) 4) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "red_jpg" ('/var/www/html/wp-content/plug...ed.jpg', array('ff0000', 'fe0000'), false) Mime type image/jpeg is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:156 5) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "green_jpg" ('/var/www/html/wp-content/plug...en.jpg', array('00ff00', '00ff01', '02ff01'), false) Mime type image/jpeg is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:156 6) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "white_jpg" ('/var/www/html/wp-content/plug...te.jpg', array('ffffff'), false) Mime type image/jpeg is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:156 7) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "red_gif" ('/var/www/html/wp-content/plug...ed.gif', array('ff0000'), false) Mime type image/gif is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:156 8) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "green_gif" ('/var/www/html/wp-content/plug...en.gif', array('00ff00'), false) Mime type image/gif is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:156 9) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "white_gif" ('/var/www/html/wp-content/plug...te.gif', array('ffffff'), false) Mime type image/gif is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:156 10) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_valid with data set "trans_gif" ('/var/www/html/wp-content/plug...ns.gif', array('5a5a5a', '020202'), true) Mime type image/gif is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:156 21) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_invalid with data set "tiff" ('/var/www/html/wp-content/plug...e.tiff') Mime type is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:1[81](https://github.com/WordPress/performance/actions/runs/9812767245/job/27097470441?pr=1327#step:10:82) 22) Test_Dominant_Color_Image_Editor_Imagick::test_get_dominant_color_invalid with data set "bmp" ('/var/www/html/wp-content/plug...ge.bmp') Mime type is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:181 23) Test_Dominant_Color::test_get_dominant_color_invalid with data set "tiff" ('/var/www/html/wp-content/plug...e.tiff') Mime type is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:181 24) Test_Dominant_Color::test_get_dominant_color_invalid with data set "bmp" ('/var/www/html/wp-content/plug...ge.bmp') Mime type is not supported. /var/www/html/wp-content/plugins/performance/plugins/dominant-color-images/tests/data/class-testcase.php:181

Co-authored-by: Lovekesh Kumar <lovekesh.kumar@rtcamp.com>

westonruter · 2024-07-09T01:49:18Z

I just got this error again when running non-multisite tests locally:

> phpunit '--verbose' '--testsuite' 'performance-lab'
Installing...
Running as single site... To run multisite, use -c tests/phpunit/multisite.xml
[09-Jul-2024 01:45:59 UTC] PHP Fatal error:  Cannot declare class Perflab_Server_Timing_Metric, because the name is already in use in /var/www/html/wp-content/plugins/performance/plugins/performance-lab/includes/server-timing/class-perflab-server-timing-metric.php on line 14
Script phpunit handling the test event returned with error code 255
Script @test --verbose --testsuite performance-lab was called via test:performance-lab
Script @test:performance-lab was called via test:plugins

It's happening each time I do npm run test-php.

swissspidy · 2024-07-09T09:09:49Z

Maybe it's a wp-env thing? PL is loaded in wp-env by default. And then the bootstrap.php loads the plugin again, so that could explain the double-inclusion of that class. Alternatively, maybe related to the object-cache drop-in? That one loadsclass-perflab-server-timing-metric.php too.

When I run the tests locally without wp-env, I don't get such an error.

thelovekesh · 2024-07-09T09:13:38Z

Alternatively, maybe related to the object-cache drop-in? That one loadsclass-perflab-server-timing-metric.php too.

Yes, it is possible if the test cases don't delete the drop-in. @westonruter and I discussed this a while ago. It seems to be a side effect that occurs after running the test cases. If we halt the server timing test cases, we might be able to replicate this issue.

Generate phpunit-multisite.xml on the fly

9124db7

westonruter added the [Type] Bug An existing feature is broken label Jul 1, 2024

westonruter added this to the performance-lab n.e.x.t milestone Jul 1, 2024

westonruter mentioned this pull request Jul 1, 2024

Running multisite tests results in no tests being executed #1326

Closed

Skip invalid mime type support tests

936bcb3

mukeshpanchal27 marked this pull request as ready for review July 4, 2024 04:28

mukeshpanchal27 requested review from pbearne, spacedmonkey, mukeshpanchal27, felixarntz and thelovekesh as code owners July 4, 2024 04:28

mukeshpanchal27 requested review from joemcgill and swissspidy July 4, 2024 04:29

thelovekesh reviewed Jul 4, 2024

View reviewed changes

composer.json Outdated Show resolved Hide resolved

bin/generate-multisite-phpunit.php Outdated Show resolved Hide resolved

.gitignore Outdated Show resolved Hide resolved

tools/phpcs/phpcs.ruleset.xml Outdated Show resolved Hide resolved

thelovekesh reviewed Jul 4, 2024

View reviewed changes

Use WP_MULTISITE=1 environment var instead of separate config

eef6f62

Co-authored-by: Lovekesh Kumar <lovekesh.kumar@rtcamp.com>

westonruter force-pushed the fix/phpunit-multisite branch from 7dbe3ad to eef6f62 Compare July 5, 2024 19:24

swissspidy approved these changes Jul 8, 2024

View reviewed changes

thelovekesh approved these changes Jul 8, 2024

View reviewed changes

mukeshpanchal27 approved these changes Jul 8, 2024

View reviewed changes

swissspidy merged commit 351f577 into trunk Jul 8, 2024
19 checks passed

swissspidy deleted the fix/phpunit-multisite branch July 8, 2024 15:45

westonruter changed the title ~~Generate phpunit-multisite.xml on the fly~~ Fix unit tests for multisite Jul 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix unit tests for multisite #1327

Fix unit tests for multisite #1327

westonruter commented Jul 1, 2024

westonruter commented Jul 2, 2024 •

edited

Loading

mukeshpanchal27 commented Jul 3, 2024

mukeshpanchal27 commented Jul 3, 2024

swissspidy commented Jul 3, 2024

westonruter commented Jul 3, 2024

github-actions bot commented Jul 4, 2024 •

edited

Loading

thelovekesh Jul 4, 2024

mukeshpanchal27 Jul 5, 2024

thelovekesh Jul 5, 2024

mukeshpanchal27 Jul 8, 2024

westonruter commented Jul 9, 2024

swissspidy commented Jul 9, 2024

thelovekesh commented Jul 9, 2024

Fix unit tests for multisite #1327

Fix unit tests for multisite #1327

Conversation

westonruter commented Jul 1, 2024

Summary

Relevant technical choices

westonruter commented Jul 2, 2024 • edited Loading

mukeshpanchal27 commented Jul 3, 2024

mukeshpanchal27 commented Jul 3, 2024

swissspidy commented Jul 3, 2024

westonruter commented Jul 3, 2024

github-actions bot commented Jul 4, 2024 • edited Loading

thelovekesh Jul 4, 2024

Choose a reason for hiding this comment

mukeshpanchal27 Jul 5, 2024

Choose a reason for hiding this comment

thelovekesh Jul 5, 2024

Choose a reason for hiding this comment

mukeshpanchal27 Jul 8, 2024

Choose a reason for hiding this comment

There were 24 skipped tests:

There were 24 skipped tests:

westonruter commented Jul 9, 2024

swissspidy commented Jul 9, 2024

thelovekesh commented Jul 9, 2024

westonruter commented Jul 2, 2024 •

edited

Loading

github-actions bot commented Jul 4, 2024 •

edited

Loading