Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add standalone PHPUnit configs in plugins/ #1013

Closed
wants to merge 43 commits into from
Closed

Conversation

thelovekesh
Copy link
Member

@thelovekesh thelovekesh commented Feb 24, 2024

Summary

Fixes task 2 of #1012

Relevant technical choices

  • Add phpunit.xml.dist to standalone plugins.
  • Move test helpers to a new class.
  • Add tests bootstrap files to plugins.

Checklist

  • PR has either [Focus] or Infrastructure label.
  • PR has a [Type] label.
  • PR has a milestone or the no milestone label.

@thelovekesh thelovekesh added [Type] Enhancement A suggestion for improvement of an existing feature Infrastructure Issues for the overall performance plugin infrastructure no milestone PRs that do not have a defined milestone for release labels Feb 24, 2024
@thelovekesh thelovekesh changed the title Poc/plugins test Add standalone PHPUnit configs in plugins/ Feb 24, 2024
@thelovekesh
Copy link
Member Author

These tests are specifically to test image editors with GD extension loaded.

public function test_get_dominant_color_invalid( $image_path ) {
if ( ! extension_loaded( 'gd' ) || ! function_exists( 'gd_info' ) ) {
$this->markTestSkipped( 'The GD PHP extension is not loaded.' );
}
$attachment_id = self::factory()->attachment->create_upload_object( $image_path );
wp_maybe_generate_attachment_metadata( get_post( $attachment_id ) );
$dominant_color_data = dominant_color_get_dominant_color_data( $attachment_id );
$this->assertWPError( $dominant_color_data );
$this->assertStringContainsString( 'image_no_editor', $dominant_color_data->get_error_code() );
}

But if both Imagick and GD extensions are loaded, it's not guaranteed which editor will take effect since we are loading both

'WP_Image_Editor_GD' => 'Dominant_Color_Image_Editor_GD',
'WP_Image_Editor_Imagick' => 'Dominant_Color_Image_Editor_Imagick',

If any of the editor satisfies the implementation conditions, it will not check for others as per https://github.com/WordPress/wordpress-develop/blob/aea8a21c118cf268128693dc8b6e5016c4314f7b/src/wp-includes/media.php#L4159

In wp-env setup, it seems like Imagick extension not supporting BMP and TIFF formats but in this setup it does hence there is error in unit tests for dominant color images.

@thelovekesh
Copy link
Member Author

thelovekesh commented Feb 24, 2024

Seems like the Imagick extension in wp-env is not working properly. It's not loading any formats, while the new WP Tests setup is loading them properly. So it seems like we have some flakiness in tests due to this which are failing in a properly working environment.

PHP Info wp-env: Imagick extension
imagick

imagick module => enabled
imagick module version => 3.6.0
imagick classes => Imagick, ImagickDraw, ImagickPixel, ImagickPixelIterator, ImagickKernel
Imagick compiled with ImageMagick version => ImageMagick 7.1.1-26 Q16-HDRI x86_64 21914 https://imagemagick.org
Imagick using ImageMagick library version => ImageMagick 7.1.1-26 Q16-HDRI x86_64 21914 https://imagemagick.org
ImageMagick copyright => (C) 1999 ImageMagick Studio LLC
ImageMagick release date => 2024-01-07
ImageMagick number of supported formats:  => 0

Directive => Local Value => Master Value
imagick.allow_zero_dimension_images => 0 => 0
imagick.locale_fix => 0 => 0
imagick.progress_monitor => 0 => 0
imagick.set_single_thread => 1 => 1
imagick.shutdown_sleep_count => 10 => 10
imagick.skip_version_check => 0 => 0
PHP Info wp-tests CI: Imagick extension
imagick
imagick module => enabled
imagick module version => 3.7.0
imagick classes => Imagick, ImagickDraw, ImagickPixel, ImagickPixelIterator, ImagickKernel
Imagick compiled with ImageMagick version => ImageMagick 6.9.11-60 Q16 x86_64 2021-01-25 https://imagemagick.org/
Imagick using ImageMagick library version => ImageMagick 6.9.11-60 Q16 x86_64 2021-01-25 https://imagemagick.org/
ImageMagick copyright => (C) 1999-2021 ImageMagick Studio LLC
ImageMagick release date => 2021-01-25
ImageMagick number of supported formats:  => 247
ImageMagick supported formats => 3FR, 3G2, 3GP, AAI, AI, APNG, ART, ARW, AVI, AVIF, AVS, BGR, BGRA, BGRO, BIE, BMP, BMP2, BMP3, BRF, CAL, CALS, CANVAS, CAPTION, CIN, CIP, CLIP, CMYK, CMYKA, CR2, CR3, CRW, CUR, CUT, DATA, DCM, DCR, DCX, DDS, DFONT, DJVU, DNG, DOT, DPX, DXT1, DXT5, EPDF, EPI, EPS, EPS2, EPS3, EPSF, EPSI, EPT, EPT2, EPT3, ERF, EXR, FAX, FILE, FITS, FRACTAL, FTP, FTS, G3, G4, GIF, GIF87, GRADIENT, GRAY, GRAYA, GROUP4, GV, H, HALD, HDR, HEIC, HISTOGRAM, HRZ, HTM, HTML, HTTP, HTTPS, ICB, ICO, ICON, IIQ, INFO, INLINE, IPL, ISOBRL, ISOBRL6, J2C, J2K, JBG, JBIG, JNG, JNX, JP2, JPC, JPE, JPEG, JPG, JPM, JPS, JPT, JSON, K25, KDC, LABEL, M2V, M4V, MAC, MAGICK, MAP, MASK, MAT, MATTE, MEF, MIFF, MKV, MNG, MONO, MOV, MP4, MPC, MPG, MRW, MSL, MSVG, MTV, MVG, NEF, NRW, NULL, ORF, OTB, OTF, PAL, PALM, PAM, PANGO, PATTERN, PBM, PCD, PCDS, PCL, PCT, PCX, PDB, PDF, PDFA, PEF, PES, PFA, PFB, PFM, PGM, PGX, PICON, PICT, PIX, PJPEG, PLASMA, PNG, PNG00, PNG24, PNG32, PNG48, PNG64, PNG8, PNM, POCKETMOD, PPM, PREVIEW, 
Directive => Local Value => Master Value
imagick.allow_zero_dimension_images => 0 => 0
imagick.locale_fix => 0 => 0
imagick.progress_monitor => 0 => 0
imagick.set_single_thread => 1 => 1
imagick.shutdown_sleep_count => 10 => 10
imagick.skip_version_check => 1 => 1

@thelovekesh thelovekesh force-pushed the poc/plugins-test branch 2 times, most recently from 7ee3299 to f658608 Compare February 25, 2024 04:25
@thelovekesh thelovekesh mentioned this pull request Feb 26, 2024
3 tasks
@thelovekesh thelovekesh changed the base branch from move/modules-to-plugins to feature/modules-to-plugins February 26, 2024 18:54
@thelovekesh thelovekesh changed the base branch from feature/modules-to-plugins to move/modules-to-plugins February 26, 2024 18:55
Comment on lines 11 to 23
// Determine correct location for plugins directory to use.
if ( false !== getenv( 'WP_PLUGIN_DIR' ) ) {
define( 'WP_PLUGIN_DIR', getenv( 'WP_PLUGIN_DIR' ) );
} else {
define( 'WP_PLUGIN_DIR', dirname( TESTS_PLUGIN_DIR ) );
}

// Load Composer dependencies if applicable.
if ( file_exists( TESTS_PLUGIN_ROOT . '/vendor/autoload.php' ) ) {
require_once TESTS_PLUGIN_ROOT . '/vendor/autoload.php';
}

// Detect where to load the WordPress tests environment from.
$_tests_dir = WPP_Tests_Helpers::get_path_to_wp_test_dir();
require_once $_tests_dir . '/includes/functions.php';

// Force plugin to be active.
$GLOBALS['wp_tests_options'] = array(
'active_plugins' => array( basename( TESTS_PLUGIN_DIR ) . '/load.php' ),
);

// Start up the WP testing environment.
require $_tests_dir . '/includes/bootstrap.php';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code would be shared across all standalone plugins' bootstrap files, correct? I think this would make sense to put up in some common shared tests dir:

Suggested change
// Determine correct location for plugins directory to use.
if ( false !== getenv( 'WP_PLUGIN_DIR' ) ) {
define( 'WP_PLUGIN_DIR', getenv( 'WP_PLUGIN_DIR' ) );
} else {
define( 'WP_PLUGIN_DIR', dirname( TESTS_PLUGIN_DIR ) );
}
// Load Composer dependencies if applicable.
if ( file_exists( TESTS_PLUGIN_ROOT . '/vendor/autoload.php' ) ) {
require_once TESTS_PLUGIN_ROOT . '/vendor/autoload.php';
}
// Detect where to load the WordPress tests environment from.
$_tests_dir = WPP_Tests_Helpers::get_path_to_wp_test_dir();
require_once $_tests_dir . '/includes/functions.php';
// Force plugin to be active.
$GLOBALS['wp_tests_options'] = array(
'active_plugins' => array( basename( TESTS_PLUGIN_DIR ) . '/load.php' ),
);
// Start up the WP testing environment.
require $_tests_dir . '/includes/bootstrap.php';
require TESTS_PLUGIN_ROOT . '/tests/standalone-bootstrap.php';

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@westonruter I think we should keep the configurable part in the plugin-specific bootstrap file. In the case of some plugin, we need to add some test filter before WP tests bootstrap, then we again need to make changes to this file.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may need to. But if we don't now, maybe err on the side of reducing duplication until we need to?

Comment on lines 11 to 23
// Determine correct location for plugins directory to use.
if ( false !== getenv( 'WP_PLUGIN_DIR' ) ) {
define( 'WP_PLUGIN_DIR', getenv( 'WP_PLUGIN_DIR' ) );
} else {
define( 'WP_PLUGIN_DIR', dirname( TESTS_PLUGIN_DIR ) );
}

// Load Composer dependencies if applicable.
if ( file_exists( TESTS_PLUGIN_ROOT . '/vendor/autoload.php' ) ) {
require_once TESTS_PLUGIN_ROOT . '/vendor/autoload.php';
}

// Detect where to load the WordPress tests environment from.
$_tests_dir = WPP_Tests_Helpers::get_path_to_wp_test_dir();
require_once $_tests_dir . '/includes/functions.php';

// Force plugin to be active.
$GLOBALS['wp_tests_options'] = array(
'active_plugins' => array( basename( TESTS_PLUGIN_DIR ) . '/load.php' ),
);

// Start up the WP testing environment.
require $_tests_dir . '/includes/bootstrap.php';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment below for plugins/webp-uploads/tests/bootstrap.php

Comment on lines 11 to 23
// Determine correct location for plugins directory to use.
if ( false !== getenv( 'WP_PLUGIN_DIR' ) ) {
define( 'WP_PLUGIN_DIR', getenv( 'WP_PLUGIN_DIR' ) );
} else {
define( 'WP_PLUGIN_DIR', dirname( TESTS_PLUGIN_DIR ) );
}

// Load Composer dependencies if applicable.
if ( file_exists( TESTS_PLUGIN_ROOT . '/vendor/autoload.php' ) ) {
require_once TESTS_PLUGIN_ROOT . '/vendor/autoload.php';
}

// Detect where to load the WordPress tests environment from.
$_tests_dir = WPP_Tests_Helpers::get_path_to_wp_test_dir();
require_once $_tests_dir . '/includes/functions.php';

// Force plugin to be active.
$GLOBALS['wp_tests_options'] = array(
'active_plugins' => array( basename( TESTS_PLUGIN_DIR ) . '/auto-sizes.php' ),
);

// Start up the WP testing environment.
require $_tests_dir . '/includes/bootstrap.php';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment below for plugins/webp-uploads/tests/bootstrap.php

tests/helpers.php Outdated Show resolved Hide resolved
tests/helpers.php Outdated Show resolved Hide resolved
@thelovekesh thelovekesh force-pushed the move/modules-to-plugins branch 2 times, most recently from 8a89eda to 9e9c47c Compare March 2, 2024 11:25
Base automatically changed from move/modules-to-plugins to trunk March 5, 2024 18:53
@thelovekesh
Copy link
Member Author

thelovekesh commented Mar 20, 2024

This PR has huge code changes and has changed its base branch multiple times. Hence I am closing this and will open a new PR with its changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Infrastructure Issues for the overall performance plugin infrastructure no milestone PRs that do not have a defined milestone for release [Type] Enhancement A suggestion for improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants