Changeset 54366 for trunk/tests/phpunit/includes/abstract-testcase.php

Timestamp:

10/02/2022 03:11:24 PM (2 years ago)

Author:

SergeyBiryukov

Message:

Build/Test Tools: Call wpTearDownAfterClass() before deleting all data, instead of after.

As of [35186] and [51568], there are two sets of methods used for setup/teardown in the test suite before and after a test class is run:

• set_up_before_class() / tear_down_after_class()
• wpSetUpBeforeClass() / wpTearDownAfterClass(). (Note the wp prefix, these are WordPress' own methods and are not the same as the native PHPUnit setUpBeforeClass() / tearDownAfterClass() methods.)

The main difference is that wpSetUpBeforeClass() receives the $factory argument for ease of use, and both wpSetUpBeforeClass() and wpTearDownAfterClass() don't need to call self::commit_transaction().

Many tests use the wpTearDownAfterClass() method to clean up posts, users, roles, etc. created via wpSetUpBeforeClass(). However, due to how the method was previously called​, this cleanup happened after all data is already deleted from the database.

This could cause some confusion when refactoring tests. For example:

public static function wpTearDownAfterClass() {
	$GLOBALS['_wp_additional_image_sizes'] = self::$_sizes;
}

public static function tear_down_after_class() {
	wp_delete_attachment( self::$large_id, true );
	parent::tear_down_after_class();
}

At a glance, it seems like these two methods can be combined:

public static function wpTearDownAfterClass() {
	wp_delete_attachment( self::$large_id, true );

	$GLOBALS['_wp_additional_image_sizes'] = self::$_sizes;
}

However, that would not work as expected: by the time wp_delete_attachment() runs, the attachment ID is no longer in the database, so it returns early, leaving some files in the uploads directory.

By calling wpTearDownAfterClass() in WP_UnitTestCase_Base::tear_down_after_class() before deleting all data, instead of after, we ensure that both of these methods have access to the same data and can be used interchangeably to perform cleanup as necessary.

Additionally, this commit moves the calls to parent methods in WP_UnitTestCase_Base:

• parent::set_up_before_class() to be the first thing called in ::set_up_before_class()
• parent::tear_down_after_class() to be the last thing called in ::tear_down_after_class()

This does not have any effect in practice, but brings consistency with how these methods are called in the test suite.

Follow-up to [35186], [35225], [35242], [38398], [39626], [49001], [51568].

Props ironprogrammer, SergeyBiryukov.
Fixes #55918. See #55652.

File:

• trunk/tests/phpunit/includes/abstract-testcase.php (modified) (3 diffs)

trunk/tests/phpunit/includes/abstract-testcase.php

@@ -63 +63 @@
         global $wpdb;
 
+        parent::set_up_before_class();
+
         $wpdb->suppress_errors = false;
         $wpdb->show_errors     = true;
@@ -68 +70 @@
         ini_set( 'display_errors', 1 );
 
-        parent::set_up_before_class();
-
         $class = get_called_class();
 
@@ -83 +83 @@
      */
     public static function tear_down_after_class() {
-        parent::tear_down_after_class();
+        $class = get_called_class();
+
+        if ( method_exists( $class, 'wpTearDownAfterClass' ) ) {
+            call_user_func( array( $class, 'wpTearDownAfterClass' ) );
+        }
 
         _delete_all_data();
         self::flush_cache();
 
-        $class = get_called_class();
-
-        if ( method_exists( $class, 'wpTearDownAfterClass' ) ) {
-            call_user_func( array( $class, 'wpTearDownAfterClass' ) );
-        }
-
         self::commit_transaction();
+
+        parent::tear_down_after_class();
     }