Skip to content

Clear out and remove /tests directory #1804

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.

Sign up for GitHub

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

Merged
merged 20 commits into from
Jan 28, 2025
Merged

Clear out and remove /tests directory #1804

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
1eeeb61
Move bootstrap.php to tools/phpunit dir
ShyamGadde Jan 16, 2025
2d62b91
Ignore PHPCS errors in test files
ShyamGadde Jan 16, 2025
3a724c8
Move test images to plugin specific directories
ShyamGadde Jan 16, 2025
8679cba
Move Optimization Detective test helpers to its plugin specific direc…
ShyamGadde Jan 16, 2025
e3f3b3e
Remove non-existent tests/multisite.xml from workflow paths
ShyamGadde Jan 21, 2025
ca4f56a
Merge branch 'trunk' into update/remove-root-tests-dir
ShyamGadde Jan 21, 2025
f829057
Merge branch 'trunk' into update/remove-root-tests-dir
ShyamGadde Jan 24, 2025
2fa9d60
Restore EOF EOL
westonruter Jan 27, 2025
7098a50
Merge branch 'trunk' into update/remove-root-tests-dir
ShyamGadde Jan 27, 2025
fcaa10e
Rename TESTS_PLUGIN_DIR to REPO_ROOT_DIR to reflect its actual intent
ShyamGadde Jan 27, 2025
9f563d5
Re-introduce TESTS_PLUGIN_DIR
ShyamGadde Jan 27, 2025
70de425
Adjust `TESTS_PLUGIN_DIR` to point to plugin root directory
ShyamGadde Jan 28, 2025
2a647be
Merge branch 'trunk' into update/remove-root-tests-dir
ShyamGadde Jan 28, 2025
6df3ca2
Prefix `REPO_ROOT_DIR` with `TESTS_` for clarity
ShyamGadde Jan 28, 2025
b002bff
Fix PHP fatal error caused by duplicate trait declaration
ShyamGadde Jan 28, 2025
a4b05d2
Fix missing constants for PHPStan
ShyamGadde Jan 28, 2025
6421043
Remove obsolete bootstrap logic missed in #1476
westonruter Jan 28, 2025
c29d670
Replace relative paths with `TESTS_REPO_ROOT_DIR`
ShyamGadde Jan 28, 2025
6d26270
Fix incorrect plugin path
ShyamGadde Jan 28, 2025
a985976
Use TESTS_PLUGIN_DIR constant in uninstall tests
westonruter Jan 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .github/workflows/php-test-plugins.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ on:
- '**/package.json'
- 'package-lock.json'
- 'phpunit.xml.dist'
- 'tests/multisite.xml'
- 'composer.json'
- 'composer.lock'
pull_request:
Expand All @@ -25,7 +24,6 @@ on:
- '**/package.json'
- 'package-lock.json'
- 'phpunit.xml.dist'
- 'tests/multisite.xml'
- 'composer.json'
- 'composer.lock'
types:
Expand Down
1 change: 0 additions & 1 deletion phpstan.neon.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ parameters:
treatPhpDocTypesAsCertain: false
paths:
- plugins
- tests
- performance.php
- plugins/performance-lab/load.php
bootstrapFiles:
Expand Down
2 changes: 1 addition & 1 deletion phpunit.xml.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<phpunit
colors="true"
backupGlobals="false"
bootstrap="tests/bootstrap.php"
bootstrap="tools/phpunit/bootstrap.php"
convertErrorsToExceptions="true"
convertNoticesToExceptions="true"
convertWarningsToExceptions="true"
Expand Down
0 tests/data/images/leaves.jpg → ...s/auto-sizes/tests/data/images/leaves.jpg
View file
Open in desktop
File renamed without changes
42 changes: 21 additions & 21 deletions plugins/dominant-color-images/tests/data/class-testcase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,85 +13,85 @@ abstract class TestCase extends WP_UnitTestCase {
public function provider_get_dominant_color(): array {
return array(
'animated_gif' => array(
'image_path' => __DIR__ . '/images/animated.gif',
'image_path' => TESTS_PLUGIN_DIR . '/tests/data/images/animated.gif',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regarding this specific change, should this also be reverted to the original __DIR__ pattern?

Copy link
Member

Choose a reason for hiding this comment

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

I think it makes sense to consistently use TESTS_PLUGIN_DIR in those cases. This way all path strings after the constant are relative to the same root.

'expected_color' => array( '874e4e', '864e4e', 'df7f7f' ),
'expected_transparency' => true,
),
'red_jpg' => array(
'image_path' => __DIR__ . '/images/red.jpg',
'image_path' => TESTS_PLUGIN_DIR . '/tests/data/images/red.jpg',
'expected_color' => array( 'ff0000', 'fe0000' ),
'expected_transparency' => false,
),
'green_jpg' => array(
'image_path' => __DIR__ . '/images/green.jpg',
'image_path' => TESTS_PLUGIN_DIR . '/tests/data/images/green.jpg',
'expected_color' => array( '00ff00', '00ff01', '02ff01' ),
'expected_transparency' => false,
),
'white_jpg' => array(
'image_path' => __DIR__ . '/images/white.jpg',
'image_path' => TESTS_PLUGIN_DIR . '/tests/data/images/white.jpg',
'expected_color' => array( 'ffffff' ),
'expected_transparency' => false,
),

'red_gif' => array(
'image_path' => __DIR__ . '/images/red.gif',
'image_path' => TESTS_PLUGIN_DIR . '/tests/data/images/red.gif',
'expected_color' => array( 'ff0000' ),
'expected_transparency' => false,
),
'green_gif' => array(
'image_path' => __DIR__ . '/images/green.gif',
'image_path' => TESTS_PLUGIN_DIR . '/tests/data/images/green.gif',
'expected_color' => array( '00ff00' ),
'expected_transparency' => false,
),
'white_gif' => array(
'image_path' => __DIR__ . '/images/white.gif',
'image_path' => TESTS_PLUGIN_DIR . '/tests/data/images/white.gif',
'expected_color' => array( 'ffffff' ),
'expected_transparency' => false,
),
'trans_gif' => array(
'image_path' => __DIR__ . '/images/trans.gif',
'image_path' => TESTS_PLUGIN_DIR . '/tests/data/images/trans.gif',
'expected_color' => array( '5a5a5a', '020202' ),
'expected_transparency' => true,
),

'red_png' => array(
'image_path' => __DIR__ . '/images/red.png',
'image_path' => TESTS_PLUGIN_DIR . '/tests/data/images/red.png',
'expected_color' => array( 'ff0000' ),
'expected_transparency' => false,
),
'green_png' => array(
'image_path' => __DIR__ . '/images/green.png',
'image_path' => TESTS_PLUGIN_DIR . '/tests/data/images/green.png',
'expected_color' => array( '00ff00' ),
'expected_transparency' => false,
),
'white_png' => array(
'image_path' => __DIR__ . '/images/white.png',
'image_path' => TESTS_PLUGIN_DIR . '/tests/data/images/white.png',
'expected_color' => array( 'ffffff' ),
'expected_transparency' => false,
),
'trans_png' => array(
'image_path' => __DIR__ . '/images/trans.png',
'image_path' => TESTS_PLUGIN_DIR . '/tests/data/images/trans.png',
'expected_color' => array( '000000' ),
'expected_transparency' => true,
),

'red_webp' => array(
'image_path' => __DIR__ . '/images/red.webp',
'image_path' => TESTS_PLUGIN_DIR . '/tests/data/images/red.webp',
'expected_color' => array( 'ff0000' ),
'expected_transparency' => false,
),
'green_webp' => array(
'image_path' => __DIR__ . '/images/green.webp',
'image_path' => TESTS_PLUGIN_DIR . '/tests/data/images/green.webp',
'expected_color' => array( '00ff00' ),
'expected_transparency' => false,
),
'white_webp' => array(
'image_path' => __DIR__ . '/images/white.webp',
'image_path' => TESTS_PLUGIN_DIR . '/tests/data/images/white.webp',
'expected_color' => array( 'ffffff' ),
'expected_transparency' => false,
),
'trans_webp' => array(
'image_path' => __DIR__ . '/images/trans.webp',
'image_path' => TESTS_PLUGIN_DIR . '/tests/data/images/trans.webp',
'expected_color' => array( '000000' ),
'expected_transparency' => true,
),
Expand All @@ -101,7 +101,7 @@ public function provider_get_dominant_color(): array {
'expected_transparency' => false,
),
'half_opaque' => array(
'image_path' => __DIR__ . '/images/half-opaque.png',
'image_path' => TESTS_PLUGIN_DIR . '/tests/data/images/half-opaque.png',
'expected_color' => array( '7e7e7e' ),
'expected_transparency' => true,
),
Expand All @@ -116,10 +116,10 @@ public function provider_get_dominant_color(): array {
public function provider_get_dominant_color_invalid_images(): array {
return array(
'tiff' => array(
'image_path' => __DIR__ . '/images/test-image.tiff',
'image_path' => TESTS_PLUGIN_DIR . '/tests/data/images/test-image.tiff',
),
'bmp' => array(
'image_path' => __DIR__ . '/images/test-image.bmp',
'image_path' => TESTS_PLUGIN_DIR . '/tests/data/images/test-image.bmp',
),
);
}
Expand All @@ -132,10 +132,10 @@ public function provider_get_dominant_color_invalid_images(): array {
public function provider_get_dominant_color_none_images(): array {
return array(
'pdf' => array(
'files_path' => __DIR__ . '/images/wordpress-gsoc-flyer.pdf',
'files_path' => TESTS_PLUGIN_DIR . '/tests/data/images/wordpress-gsoc-flyer.pdf',
),
'mp4' => array(
'files_path' => __DIR__ . '/images/small-video.mp4',
'files_path' => TESTS_PLUGIN_DIR . '/tests/data/images/small-video.mp4',
),
);
}
Expand Down
0 tests/data/images/balloons.webp → ...or-images/tests/data/images/balloons.webp
View file
Open in desktop
File renamed without changes.
6 changes: 3 additions & 3 deletions plugins/dominant-color-images/tests/test-dominant-color.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ public function test_tag_add_adjust_to_image_attributes( string $image_path, arr
* @param bool $expected Whether the dominant color should be added.
*/
public function test_dominant_color_img_tag_add_dominant_color_requires_proper_quotes( string $image, bool $expected ): void {
$attachment_id = self::factory()->attachment->create_upload_object( __DIR__ . '/data/images/red.jpg' );
$attachment_id = self::factory()->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/data/images/red.jpg' );
wp_maybe_generate_attachment_metadata( get_post( $attachment_id ) );

$image_url = wp_get_attachment_image_url( $attachment_id );
Expand Down Expand Up @@ -206,7 +206,7 @@ public function data_dominant_color_img_tag_add_dominant_color_requires_proper_q
* @param string $expected The expected style attribute and value.
*/
public function test_dominant_color_img_tag_add_dominant_color_should_add_dominant_color_inline_style( string $filtered_image, string $expected ): void {
$attachment_id = self::factory()->attachment->create_upload_object( __DIR__ . '/data/images/red.jpg' );
$attachment_id = self::factory()->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/data/images/red.jpg' );
wp_maybe_generate_attachment_metadata( get_post( $attachment_id ) );

list( $src, $width, $height ) = wp_get_attachment_image_src( $attachment_id );
Expand Down Expand Up @@ -246,7 +246,7 @@ public function data_provider_dominant_color_check_inline_style(): array {
* @param string $expected The expected style attribute and value.
*/
public function test_dominant_color_update_attachment_image_attributes( string $style_attr, string $expected ): void {
$attachment_id = self::factory()->attachment->create_upload_object( __DIR__ . '/data/images/red.jpg' );
$attachment_id = self::factory()->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/data/images/red.jpg' );

$attachment_image = wp_get_attachment_image( $attachment_id, 'full', false, array( 'style' => $style_attr ) );
$this->assertStringContainsString( $expected, $attachment_image );
Expand Down
3 changes: 3 additions & 0 deletions plugins/embed-optimizer/tests/bootstrap.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,6 @@

// Require the suggested plugin.
require_once __DIR__ . '/../../optimization-detective/load.php';
Copy link
Member

Choose a reason for hiding this comment

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

Other places where __DIR__ . '../../' is used we can also replace with:

Suggested change
require_once __DIR__ . '/../../optimization-detective/load.php';
require_once TESTS_REPO_ROOT_DIR . '/plugins/optimization-detective/load.php';

Copy link
Member

Choose a reason for hiding this comment

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

Note that this manual loading of Optimization Detective is done for Embed Optimizer but not for Image Prioritizer because the latter has Optimization Detective as an explicit dependency, whereas the former it is a recommended dependency. The test bootstrap logic automatically loads any explicit dependencies.

Copy link
Member

@westonruter westonruter Jan 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Here are the relevant instances (updated):

plugins/optimization-detective/tests/test-uninstall.php:27:		require __DIR__ . '/../../../plugins/optimization-detective/uninstall.php';
plugins/embed-optimizer/tests/bootstrap.php:9:require_once __DIR__ . '/../../optimization-detective/load.php';
plugins/speculation-rules/tests/test-speculation-rules-uninstall.php:27:		require __DIR__ . '/../../../plugins/speculation-rules/uninstall.php';

Copy link
Member

Choose a reason for hiding this comment

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

Oh, the lines in auto-sizes are obsolete. They should have been removed with #1476

Copy link
Member

Choose a reason for hiding this comment

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

Oh, the lines in auto-sizes are obsolete. They should have been removed with #1476

Removed in 6421043


// Load the test helpers.
require_once TESTS_REPO_ROOT_DIR . '/plugins/optimization-detective/tests/class-optimization-detective-test-helpers.php';
9 changes: 9 additions & 0 deletions plugins/image-prioritizer/tests/bootstrap.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?php
/**
* Test Bootstrap for Image Prioritizer.
*
* @package image-prioritizer
*/

// Load the test helpers.
require_once TESTS_REPO_ROOT_DIR . '/plugins/optimization-detective/tests/class-optimization-detective-test-helpers.php';
0 tests/data/images/car.jpeg → ...ge-prioritizer/tests/data/images/car.jpeg
View file
Open in desktop
File renamed without changes
9 changes: 9 additions & 0 deletions plugins/optimization-detective/tests/bootstrap.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?php
/**
* Test Bootstrap for Optimiztaion Detective.
*
* @package optimization-detective
*/

// Load the test helpers.
require_once TESTS_REPO_ROOT_DIR . '/plugins/optimization-detective/tests/class-optimization-detective-test-helpers.php';
2 changes: 1 addition & 1 deletion ...s-optimization-detective-test-helpers.php → ...s-optimization-detective-test-helpers.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
/**
* Helper trait for Optimization Detective tests.
*
* @package performance-lab
* @package optimization-detective
*
* @noinspection PhpDocMissingThrowsInspection
* @noinspection PhpUnhandledExceptionInspection
Expand Down
Binary file added plugins/webp-uploads/tests/data/images/balloons.webp
View file
Open in desktop
Binary file not shown.
Binary file added plugins/webp-uploads/tests/data/images/car.jpeg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
0 tests/data/images/dice.png → ...s/webp-uploads/tests/data/images/dice.png
View file
Open in desktop
File renamed without changes
0 tests/data/images/earth.gif → .../webp-uploads/tests/data/images/earth.gif
View file
Open in desktop
File renamed without changes
0 tests/data/images/image.jpeg → ...webp-uploads/tests/data/images/image.jpeg
View file
Open in desktop
File renamed without changes
0 tests/data/images/image.jpg → .../webp-uploads/tests/data/images/image.jpg
View file
Open in desktop
File renamed without changes
Binary file added plugins/webp-uploads/tests/data/images/leaves.jpg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
0 tests/data/images/paint.jpeg → ...webp-uploads/tests/data/images/paint.jpeg
View file
Open in desktop
File renamed without changes
3 changes: 3 additions & 0 deletions tools/phpcs/phpcs.ruleset.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,15 @@
</rule>
<rule ref="WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents">
<exclude-pattern>tests/*</exclude-pattern>
<exclude-pattern>tools/phpunit/*</exclude-pattern>
</rule>
<rule ref="WordPress.Security.EscapeOutput.OutputNotEscaped">
<exclude-pattern>tests/*</exclude-pattern>
<exclude-pattern>tools/phpunit/*</exclude-pattern>
</rule>
<rule ref="WordPress.WP.AlternativeFunctions.unlink_unlink">
<exclude-pattern>tests/*</exclude-pattern>
<exclude-pattern>tools/phpunit/*</exclude-pattern>
</rule>
<rule ref="WordPress.WP.GlobalVariablesOverride.Prohibited">
<exclude-pattern>tests/*</exclude-pattern>
Expand Down
1 change: 1 addition & 0 deletions tools/phpstan/constants.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

define( 'TESTS_PLUGIN_DIR', './' );
define( 'TESTS_REPO_ROOT_DIR', './' );
define( 'WPINC', 'wp-includes' );

define( 'SPECULATION_RULES_VERSION', '0.0.0' );
Expand Down
27 changes: 13 additions & 14 deletions tests/bootstrap.php → tools/phpunit/bootstrap.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,18 @@
* @package performance-lab
*/

define( 'TESTS_PLUGIN_DIR', dirname( __DIR__ ) );
define( 'TESTS_REPO_ROOT_DIR', dirname( __DIR__, 2 ) );

// 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 ) );
define( 'WP_PLUGIN_DIR', dirname( TESTS_REPO_ROOT_DIR ) );
}

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

// Detect where to load the WordPress tests environment from.
Expand All @@ -26,8 +26,8 @@
$_test_root = getenv( 'WP_DEVELOP_DIR' ) . '/tests/phpunit';
} elseif ( false !== getenv( 'WP_PHPUNIT__DIR' ) ) {
$_test_root = getenv( 'WP_PHPUNIT__DIR' );
} elseif ( file_exists( TESTS_PLUGIN_DIR . '/../../../../tests/phpunit/includes/functions.php' ) ) {
$_test_root = TESTS_PLUGIN_DIR . '/../../../../tests/phpunit';
} elseif ( file_exists( TESTS_REPO_ROOT_DIR . '/../../../../tests/phpunit/includes/functions.php' ) ) {
$_test_root = TESTS_REPO_ROOT_DIR . '/../../../../tests/phpunit';
} else { // Fallback.
$_test_root = '/tmp/wordpress-tests-lib';
}
Expand All @@ -47,7 +47,7 @@
if (
'--testsuite' === $arg &&
isset( $_SERVER['argv'][ $index + 1 ] ) &&
file_exists( TESTS_PLUGIN_DIR . '/plugins/' . $_SERVER['argv'][ $index + 1 ] )
file_exists( TESTS_REPO_ROOT_DIR . '/plugins/' . $_SERVER['argv'][ $index + 1 ] )
) {
$plugin_name = $_SERVER['argv'][ $index + 1 ];
}
Expand All @@ -58,13 +58,15 @@
$plugin_name = 'performance-lab';
}

define( 'TESTS_PLUGIN_DIR', TESTS_REPO_ROOT_DIR . "/plugins/$plugin_name" );

/**
* Load plugin bootstrap and any dependencies.
*
* @param string $plugin_name Plugin slug to load.
*/
$load_plugin = static function ( string $plugin_name ) use ( &$load_plugin ): void {
$plugin_test_path = TESTS_PLUGIN_DIR . '/plugins/' . $plugin_name;
$plugin_test_path = TESTS_REPO_ROOT_DIR . '/plugins/' . $plugin_name;
Copy link
Member

Choose a reason for hiding this comment

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

See above:

Suggested change
$plugin_test_path = TESTS_REPO_ROOT_DIR . '/plugins/' . $plugin_name;
$plugin_test_path = TESTS_PLUGIN_DIR;

Alternatively, we could get rid of this variable entirely, since it's only referencing the constant as-is.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, we can't do this. This function also needs to load plugin dependencies. I thought this at first myself.

Copy link
Member

Choose a reason for hiding this comment

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

Which function do you mean?

Copy link
Member

Choose a reason for hiding this comment

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

See this $load_plugin function here is used recursively. It loads any Requires Plugins.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure when the $load_plugin approach was added, just seeing this now.

It doesn't make sense to me to use this approach, especially given the complexity you're describing. There is a special global to control which plugins should be active. For example:

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

Can we not use that approach? If the problem is that we're loading plugins from a custom subdirectory, I wonder whether we could maybe override WP_PLUGIN_DIR to be TESTS_REPO_ROOT_DIR . '/plugins'.

Overall, it certainly makes more sense to me to tie into WordPress's built-in plugin loading mechanism instead of trying to manually require files here (which BTW plugins_loaded is, strictly speaking, also too late for).

Copy link
Member

Choose a reason for hiding this comment

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

The current approach seems to have been working well, but what you propose seems good. Maybe do it another PR?

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense, that would be a separate enhancement.

if ( file_exists( $plugin_test_path . '/' . $plugin_name . '.php' ) ) {
$plugin_file = $plugin_test_path . '/' . $plugin_name . '.php';
} elseif ( file_exists( $plugin_test_path . '/load.php' ) ) {
Expand Down Expand Up @@ -102,9 +104,9 @@ static function () use ( $load_plugin, $plugin_name ): void {
tests_add_filter(
'plugins_loaded',
static function () use ( $plugin_name ): void {
require_once TESTS_PLUGIN_DIR . '/plugins/' . $plugin_name . '/includes/admin/load.php';
require_once TESTS_PLUGIN_DIR . '/plugins/' . $plugin_name . '/includes/admin/server-timing.php';
require_once TESTS_PLUGIN_DIR . '/plugins/' . $plugin_name . '/includes/admin/plugins.php';
require_once TESTS_REPO_ROOT_DIR . '/plugins/' . $plugin_name . '/includes/admin/load.php';
require_once TESTS_REPO_ROOT_DIR . '/plugins/' . $plugin_name . '/includes/admin/server-timing.php';
require_once TESTS_REPO_ROOT_DIR . '/plugins/' . $plugin_name . '/includes/admin/plugins.php';
Comment on lines +107 to +109
Copy link
Member

Choose a reason for hiding this comment

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

See above:

Suggested change
require_once TESTS_REPO_ROOT_DIR . '/plugins/' . $plugin_name . '/includes/admin/load.php';
require_once TESTS_REPO_ROOT_DIR . '/plugins/' . $plugin_name . '/includes/admin/server-timing.php';
require_once TESTS_REPO_ROOT_DIR . '/plugins/' . $plugin_name . '/includes/admin/plugins.php';
require_once TESTS_PLUGIN_DIR . '/includes/admin/load.php';
require_once TESTS_PLUGIN_DIR . '/includes/admin/server-timing.php';
require_once TESTS_PLUGIN_DIR . '/includes/admin/plugins.php';

Copy link
Member

Choose a reason for hiding this comment

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

Actually we can't do this. See #1804 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

What do you mean? These files don't require any dependencies.

Copy link
Member

Choose a reason for hiding this comment

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

Plugin dependencies (i.e. Image Prioritizer depending on Optimization Detective)

Copy link
Member

Choose a reason for hiding this comment

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

I don't understand. Specific files are being loaded here that only matter for performance-lab.

Copy link
Member

Choose a reason for hiding this comment

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

Oh right. At least these should be wrapped in a condition so they only are loaded if the $plugin_name is performance-lab

Copy link
Member

Choose a reason for hiding this comment

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

Oh, but that already it the case. This logic is wrapped in:

if ( 'performance-lab' === $plugin_name ) {

},
1
);
Expand All @@ -127,8 +129,5 @@ static function ( bool $passthrough ): bool {
}
);

// Require helper classes.
require_once __DIR__ . '/class-optimization-detective-test-helpers.php';

// Start up the WP testing environment.
require $_test_root . '/includes/bootstrap.php';
Loading