From f6715fd047ed0e013cc09e04068a54c4f7b2de46 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 2 Jul 2021 00:09:33 +0200 Subject: [PATCH] TestCase: iterate on resetting of static properties The `PHPMailer::preSend()` method may alter the value of the _`protect static`_ `PHPMailer::$LE` property. https://github.com/PHPMailer/PHPMailer/blob/a9978d207919fdf5b90af42ca117fcab8609ae16/src/PHPMailer.php#L1494-L1504 This means that tests which rely on the value of the property being the default, may start to fail unexpectedly depending on the _order_ in which tests are being run. I.e. if the test relying on the default value is run first, all will be fine, however, if that same test is run _after_ a test which calls `preSend()` on a non-Windows box with PHP < 8.0 and with `$this->Mailer` set to `mail`, the test may fail unexpectedly. This commit fixes that by expanding the previously introduced "resetting of static properties" logic. This commit: * Expands the list of static properties in the `TestCase` class to include the `PHPMailer::$LE` property. * Introduces two new methods to the `TestCase` class: - `resetStaticProperties()` to reset all static properties listed in the `TestCase::$PHPMailerStaticProps` property to their default values. - `updateStaticProperty()` to change the value of one individual static property in a class. * The `updateStaticProperty()` method was made `public static` to allow for tests which do not need an instantiated PHPMailer object and thus extend the `Yoast\PHPUnitPolyfills\TestCases\TestCase` directly to be able to use the method as well. If any such tests would use the method, that particular test will be responsible for resetting the value back to the default after the test. * The resetting of the static properties has also been moved from the `set_up()` to the `tear_down()` method. As mentioned above, these static properties may be used by both tests extending this `TestCase` as well as test extending the `Yoast\PHPUnitPolyfills\TestCases\TestCase`. With that in mind, resetting _after_ each test will be more stable, as long as tests which extend the `Yoast\PHPUnitPolyfills\TestCases\TestCase` and would change static properties clean up after themselves. **Note**: I'm not including the `PHPMailer::$IcalMethods` property in this change as AFAICS, this property is not being changed anywhere in the package code and as the property is `protected`, it cannot easily be changed from within a test either (and if it would be changed by a test, that test would be responsible for resetting it). Also note: due to limitations in the `Reflection` extension, a hard-coded list of the default values of the static properties will need to be maintained in the `TestCase`. This cannot be helped as the `Reflection` extension does not have a way to accurately retrieve the default value of static properties in a PHP cross-version compatible manner. --- test/TestCase.php | 58 +++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 51 insertions(+), 7 deletions(-) diff --git a/test/TestCase.php b/test/TestCase.php index 62786179..8dcd8008 100644 --- a/test/TestCase.php +++ b/test/TestCase.php @@ -15,6 +15,8 @@ namespace PHPMailer\Test; use PHPMailer\PHPMailer\PHPMailer; use PHPMailer\PHPMailer\SMTP; +use ReflectionClass; +use ReflectionProperty; use Yoast\PHPUnitPolyfills\TestCases\TestCase as PolyfillTestCase; /** @@ -55,13 +57,18 @@ abstract class TestCase extends PolyfillTestCase private $NoteLog = []; /** - * List of *public static* properties in the PHPMailer class with their default values. + * List of *static* properties in the PHPMailer class which _may_ be changed from within a test, + * with their default values. * - * This list is used by the `set_up()` method. + * This list is used by the {@see `TestCase::resetStaticProperties()`} method. + * + * {@internal The default values have to be (manually) maintained here as the Reflection + * extension does not provide accurate information on the default values of static properties.} * * @var array Key is the property name, value the default as per the PHPMailer class. */ private $PHPMailerStaticProps = [ + 'LE' => PHPMailer::CRLF, 'validator' => 'php', ]; @@ -84,11 +91,6 @@ abstract class TestCase extends PolyfillTestCase */ protected function set_up() { - // Make sure that public static variables contain their default values at the start of each test. - foreach ($this->PHPMailerStaticProps as $key => $value) { - PHPMailer::${$key} = $value; - } - if (file_exists(\PHPMAILER_INCLUDE_DIR . '/test/testbootstrap.php')) { include \PHPMAILER_INCLUDE_DIR . '/test/testbootstrap.php'; // Overrides go in here. } @@ -159,12 +161,54 @@ abstract class TestCase extends PolyfillTestCase */ protected function tear_down() { + // Make sure that any changes to static variables are undone after each test. + $this->resetStaticProperties(); + // Clean test class native properties between tests. $this->Mail = null; $this->ChangeLog = []; $this->NoteLog = []; } + /** + * Reset the static properties in the PHPMailer class to their default values. + */ + protected function resetStaticProperties() + { + $reflClass = new ReflectionClass(PHPMailer::class); + $staticPropValues = $reflClass->getStaticProperties(); + + foreach ($this->PHPMailerStaticProps as $name => $default) { + if (isset($staticPropValues[$name]) && $staticPropValues[$name] === $default) { + continue; + } + + self::updateStaticProperty(PHPMailer::class, $name, $default); + } + } + + /** + * Update the value of a - potentially inaccessible - static property in a class. + * + * @param string $className The target class. + * @param string $propertyName The name of the static property. + * @param mixed $value The new value for the property. + */ + public static function updateStaticProperty($className, $propertyName, $value) + { + $reflProp = new ReflectionProperty($className, $propertyName); + $isPublic = $reflProp->isPublic(); + if ($isPublic !== true) { + $reflProp->setAccessible(true); + } + + $reflProp->setValue($value); + + if ($isPublic !== true) { + $reflProp->setAccessible(false); + } + } + /** * Build the body of the message in the appropriate format. */