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

Clear out and remove /tests directory #1804

Merged
merged 20 commits into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from 7 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
4 changes: 1 addition & 3 deletions .github/workflows/php-test-plugins.yml
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 Expand Up @@ -134,4 +132,4 @@ jobs:
directory: ./multisite-reports
flags: multisite
name: ${{ matrix.php }}-multisite-coverage
fail_ci_if_error: true
fail_ci_if_error: true
1 change: 0 additions & 1 deletion phpstan.neon.dist
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
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
2 changes: 1 addition & 1 deletion plugins/auto-sizes/tests/test-auto-sizes.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class Test_AutoSizes extends WP_UnitTestCase {
* Setup shared fixtures.
*/
public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ): void {
self::$image_id = $factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/data/images/leaves.jpg' );
self::$image_id = $factory->attachment->create_upload_object( __DIR__ . '/data/images/leaves.jpg' );
}

/**
Expand Down
2 changes: 1 addition & 1 deletion plugins/auto-sizes/tests/test-improve-calculate-sizes.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public static function set_up_before_class(): void {

switch_theme( 'twentytwentyfour' );

self::$image_id = self::factory()->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/data/images/leaves.jpg' );
self::$image_id = self::factory()->attachment->create_upload_object( __DIR__ . '/data/images/leaves.jpg' );
}

public function set_up(): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public function provider_get_dominant_color(): array {
'expected_transparency' => true,
),
'balloons_webp' => array(
'image_path' => TESTS_PLUGIN_DIR . '/tests/data/images/balloons.webp',
'image_path' => __DIR__ . '/images/balloons.webp',
'expected_color' => array( 'c1bbb9', 'c0bbb9', 'c0bab8', 'c3bdbd', 'bfbab8' ),
'expected_transparency' => false,
),
Expand Down
3 changes: 3 additions & 0 deletions plugins/embed-optimizer/tests/bootstrap.php
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

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 __DIR__ . '/../../optimization-detective/tests/class-optimization-detective-test-helpers.php';
9 changes: 9 additions & 0 deletions plugins/image-prioritizer/tests/bootstrap.php
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 __DIR__ . '/../../optimization-detective/tests/class-optimization-detective-test-helpers.php';
2 changes: 1 addition & 1 deletion plugins/image-prioritizer/tests/test-helper.php
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ static function ( $pre, $parsed_args, $url ) use ( $image_url ) {

'good_url_allowed_cdn_origin' => array(
'set_up' => function (): string {
$attachment_id = self::factory()->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/data/images/car.jpeg' );
$attachment_id = self::factory()->attachment->create_upload_object( __DIR__ . '/data/images/car.jpeg' );
$this->assertIsInt( $attachment_id );

add_filter(
Expand Down
9 changes: 9 additions & 0 deletions plugins/optimization-detective/tests/bootstrap.php
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 __DIR__ . '/class-optimization-detective-test-helpers.php';
Original file line number Diff line number Diff line change
@@ -0,0 +1,266 @@
<?php
/**
* Helper trait for Optimization Detective tests.
*
* @package optimization-detective
*
* @noinspection PhpDocMissingThrowsInspection
* @noinspection PhpUnhandledExceptionInspection
*/

if ( ! trait_exists( 'Optimization_Detective_Test_Helpers' ) ) {
Copy link
Member

Choose a reason for hiding this comment

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

Is this if necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this as I kept encountering the following error:

PHP Fatal error:  Cannot declare trait Optimization_Detective_Test_Helpers, because the name is already in use in /var/www/html/wp-content/plugins/performance/plugins/optimization-detective/tests/class-optimization-detective-test-helpers.php on line 18

Adding this if condition resolved the issue. If there's a better way to address this, I'd be happy to update it.

Copy link
Member

@westonruter westonruter Jan 28, 2025

Choose a reason for hiding this comment

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

Humm. Even with the require_once. I guess it has to do with Docker and how it mounts the directories in a way that PHP doesn't realize that the paths are the same?

What if lines like:

require_once __DIR__ . '/../../optimization-detective/tests/class-optimization-detective-test-helpers.php';

Are replaced with:

require_once TESTS_REPO_ROOT_DIR . '/plugins/optimization-detective/tests/class-optimization-detective-test-helpers.php';

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replacing it with TESTS_REPO_ROOT_DIR works perfectly and resolves the issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in b002bff.


/**
* @phpstan-type ElementDataSubset array{
* xpath: string,
* isLCP?: bool,
* intersectionRatio?: float
* }
*/
trait Optimization_Detective_Test_Helpers {

/**
* Populates complete URL metrics for the provided element data.
*
* @phpstan-param ElementDataSubset[] $elements
* @param array[] $elements Element data.
* @param bool $complete Whether to fully populate the groups.
* @throws Exception But it won't.
*/
public function populate_url_metrics( array $elements, bool $complete = true ): void {
$slug = od_get_url_metrics_slug( od_get_normalized_query_vars() );
$etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry(), null, null ); // Note: Tests rely on the od_current_url_metrics_etag_data filter to set the desired value.
$sample_size = $complete ? od_get_url_metrics_breakpoint_sample_size() : 1;
foreach ( array_merge( od_get_breakpoint_max_widths(), array( 1000 ) ) as $viewport_width ) {
for ( $i = 0; $i < $sample_size; $i++ ) {
OD_URL_Metrics_Post_Type::store_url_metric(
$slug,
$this->get_sample_url_metric(
array(
'etag' => $etag,
'viewport_width' => $viewport_width,
'elements' => $elements,
)
)
);
}
}
}

/**
* Gets a sample DOM rect for testing.
*
* @return array<string, float>
*/
public function get_sample_dom_rect(): array {
return array(
'width' => 500.1,
'height' => 500.2,
'x' => 100.3,
'y' => 100.4,
'top' => 0.1,
'right' => 0.2,
'bottom' => 0.3,
'left' => 0.4,
);
}

/**
* Gets a sample URL metric.
*
* @phpstan-param array{
* timestamp?: float,
* etag?: non-empty-string,
* url?: string,
* viewport_width?: int,
* viewport_height?: int,
* element?: ElementDataSubset,
* elements?: array<ElementDataSubset>
* } $params Params.
*
* @return OD_URL_Metric URL metric.
*/
public function get_sample_url_metric( array $params ): OD_URL_Metric {
$params = array_merge(
array(
'etag' => od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry(), null, null ), // Note: Tests rely on the od_current_url_metrics_etag_data filter to set the desired value.
'url' => home_url( '/' ),
'viewport_width' => 480,
'elements' => array(),
'timestamp' => microtime( true ),
'extended_root' => array(),
),
$params
);

if ( array_key_exists( 'element', $params ) ) {
$params['elements'][] = $params['element'];
}

$data = array_merge(
array(
'etag' => $params['etag'],
'url' => $params['url'],
'viewport' => array(
'width' => $params['viewport_width'],
'height' => $params['viewport_height'] ?? ceil( $params['viewport_width'] / 2 ),
),
'timestamp' => $params['timestamp'],
'elements' => array_map(
function ( array $element ): array {
return array_merge(
array(
'isLCP' => false,
'isLCPCandidate' => $element['isLCP'] ?? false,
'intersectionRatio' => 1,
'intersectionRect' => $this->get_sample_dom_rect(),
'boundingClientRect' => $this->get_sample_dom_rect(),
),
$element
);
},
$params['elements']
),
),
$params['extended_root']
);
return new OD_URL_Metric( $data );
}

/**
* Removes initial tabs from the lines in the input.
*
* @param string $input Input.
* @return string Output.
*/
public function remove_initial_tabs( string $input ): string {
return (string) preg_replace( '/^\t+/m', '', $input );
}

/**
* Gets JSON-serializable data from an array of JsonSerializable objects.
*
* @param JsonSerializable[] $items Items.
* @return array<string|int, mixed> Data from items.
*/
public function get_array_json_data( array $items ): array {
return array_map(
static function ( JsonSerializable $item ) {
return $item->jsonSerialize();
},
$items
);
}

/**
* Loads snapshot test cases.
*
* @param non-empty-string $directory Directory for test cases.
* @return array<string, array{ directory: non-empty-string }> Test cases.
*/
public function load_snapshot_test_cases( string $directory ): array {
$test_cases = array();
foreach ( (array) glob( $directory . '/*' ) as $test_case ) {
/**
* Test case directory.
*
* @var non-empty-string $test_case
*/
$test_cases[ basename( $test_case ) ] = array(
'directory' => $test_case,
);
}
return $test_cases;
}

/**
* Asserts equality against snapshot.
*
* @param non-empty-string $directory Directory for test cases.
*/
public function assert_snapshot_equals( string $directory ): void {
$set_up_file = "$directory/set-up.php";
if ( ! file_exists( $set_up_file ) ) {
throw new Exception( "Missing required file: $directory/set-up.php" );
}

$set_up = require $set_up_file;
$buffer_file = "$directory/buffer.html";

$buffer = file_get_contents( $buffer_file );
if ( ! is_string( $buffer ) ) {
throw new Exception( "Missing test case file: $buffer_file" );
}

$expected_file = "$directory/expected.html";
$actual_file = "$directory/actual.html";
if ( file_exists( $expected_file ) ) {
$expected = file_get_contents( $expected_file );
if ( ! is_string( $expected ) ) {
throw new Exception( "Missing test case file: $expected_file" );
}
} else {
$expected = null;
}

$replacements = $set_up( $this, $this::factory() );

// Replace placeholders with values computed in set_up.
if ( is_array( $replacements ) && count( $replacements ) > 0 ) {
$buffer = str_replace( array_keys( $replacements ), array_values( $replacements ), $buffer );
if ( is_string( $expected ) ) {
$expected = str_replace( array_keys( $replacements ), array_values( $replacements ), $expected );
}
}

$buffer = od_optimize_template_output_buffer( $buffer );

// Normalize script module content so changes do not impact snapshots.
$buffer = preg_replace_callback(
'#(<script type="module">)(.+?)(</script>)#s',
static function ( $matches ) {
array_shift( $matches );
list( $start_tag, $text, $end_tag ) = $matches;

$text = str_replace( "/* <![CDATA[ */\n", '', $text );
$text = str_replace( "/* ]]> */\n", '', $text );
$text = trim( $text );
if ( 1 === preg_match( '/^(import|const) \w+/', $text, $matches ) ) {
$text = '/* ' . $matches[0] . ' ... */';
}
return $start_tag . $text . $end_tag;
},
$buffer
);

// Undo replacements so that the placeholders are restored to the buffer for persisting in the snapshot.
$snapshot = $buffer;
if ( is_array( $replacements ) && count( $replacements ) > 0 ) {
$snapshot = str_replace( array_values( $replacements ), array_keys( $replacements ), $snapshot );
}

$buffer = $this->remove_initial_tabs( $buffer );
if ( is_string( $expected ) ) {
$expected = $this->remove_initial_tabs( $expected );
}

if ( $buffer !== $expected ) {
file_put_contents( $actual_file, $snapshot ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_operations_file_put_contents
}

$rel_actual_file = str_replace( trailingslashit( getcwd() ), '', $actual_file );
$rel_expected_file = str_replace( trailingslashit( getcwd() ), '', $expected_file );
$rel_buffer_file = str_replace( trailingslashit( getcwd() ), '', $buffer_file );

if ( null === $expected ) {
$this->markTestIncomplete( "Examine $actual_file to see if it is expected and if so rename to $expected_file." );
} else {
$this->assertEquals(
$expected,
$buffer,
"Examine $rel_actual_file for differences with $rel_buffer_file and if acceptable move to $rel_expected_file"
);
}
}
}
}
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Loading