From e4f84c633b72ec70efc50b8016871c3bc43e691e Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 21 Feb 2023 11:33:08 +0100 Subject: [PATCH] minor #49431 [Mailer][Translation] Remove some `static` occurrences that may cause unstable tests (alexandre-daubois) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR was merged into the 6.3 branch. Discussion ---------- [Mailer][Translation] Remove some `static` occurrences that may cause unstable tests | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | _NA_ | License | MIT | Doc PR | _NA_ I had a little tchat with `@nicolas`-grekas who warned me that a few of my late edits on static data providers are a bit dangerous. Indeed, some helper methods were using static properties, which could lead to some leaks between test cases, and/or unstable tests. Helper classes doing so, found in `Translation` and `Mailer`, have been reverted to non-static ones. Data-providers are of course still statics and have been adapted to not use those methods. The targeted branch is 6.3 and this is intended, as requested by Nicolas. If you need any help during the backport of these edits, I'll be happy to help again! ℹ️ A lot of notifier bridges has been introduced in 6.3 and their data providers weren't updated. I also bundled this change in the PR, which should fix 6.3 pipeline as well. Finally, I updated `SmsapiTransportFactoryTest::missingRequiredOptionProvider`. As the `from` option has been made optional, the only dataset provided failed. Commits ------- 2ca9cf8988 [Mailer][Translation][Notifier] Remove some `static` occurrences that may cause unstable tests --- CHANGELOG.md | 14 ++++----- Test/TransportFactoryTestCase.php | 26 +++++++---------- Tests/Transport/NullTransportFactoryTest.php | 8 +++-- .../SendmailTransportFactoryTest.php | 10 ++++--- .../Smtp/EsmtpTransportFactoryTest.php | 29 ++++++++++--------- 5 files changed, 42 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3fb4577..87262ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,12 @@ CHANGELOG ========= +6.2.7 +----- + + * [BC BREAK] The following data providers for `TransportFactoryTestCase` are now static: + `supportsProvider()`, `createProvider()`, `unsupportedSchemeProvider()`and `incompleteDsnProvider()` + 6.2 --- @@ -18,14 +24,6 @@ CHANGELOG * The `HttpTransportException` class takes a string at first argument -5.4.21 ------- - - * [BC BREAK] The following data providers for `TransportFactoryTestCase` are now static: - `supportsProvider()`, `createProvider()`, `unsupportedSchemeProvider()`and `incompleteDsnProvider()` - * [BC BREAK] The following data providers for `TransportTestCase` are now static: - `toStringProvider()`, `supportedMessagesProvider()` and `unsupportedMessagesProvider()` - 5.4 --- diff --git a/Test/TransportFactoryTestCase.php b/Test/TransportFactoryTestCase.php index eda10fe..7bcd898 100644 --- a/Test/TransportFactoryTestCase.php +++ b/Test/TransportFactoryTestCase.php @@ -13,8 +13,6 @@ use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; -use Psr\Log\NullLogger; -use Symfony\Component\HttpClient\MockHttpClient; use Symfony\Component\Mailer\Exception\IncompleteDsnException; use Symfony\Component\Mailer\Exception\UnsupportedSchemeException; use Symfony\Component\Mailer\Transport\Dsn; @@ -33,11 +31,11 @@ abstract class TransportFactoryTestCase extends TestCase protected const USER = 'u$er'; protected const PASSWORD = 'pa$s'; - protected static $dispatcher; - protected static $client; - protected static $logger; + protected $dispatcher; + protected $client; + protected $logger; - abstract public static function getFactory(): TransportFactoryInterface; + abstract public function getFactory(): TransportFactoryInterface; abstract public static function supportsProvider(): iterable; @@ -102,22 +100,18 @@ public function testIncompleteDsnException(Dsn $dsn) $factory->create($dsn); } - protected static function getDispatcher(): EventDispatcherInterface + protected function getDispatcher(): EventDispatcherInterface { - return self::$dispatcher ??= new class() implements EventDispatcherInterface { - public function dispatch($event, string $eventName = null): object - { - } - }; + return $this->dispatcher ??= $this->createMock(EventDispatcherInterface::class); } - protected static function getClient(): HttpClientInterface + protected function getClient(): HttpClientInterface { - return self::$client ??= new MockHttpClient(); + return $this->client ??= $this->createMock(HttpClientInterface::class); } - protected static function getLogger(): LoggerInterface + protected function getLogger(): LoggerInterface { - return self::$logger ??= new NullLogger(); + return $this->logger ??= $this->createMock(LoggerInterface::class); } } diff --git a/Tests/Transport/NullTransportFactoryTest.php b/Tests/Transport/NullTransportFactoryTest.php index 50c35cb..b28935a 100644 --- a/Tests/Transport/NullTransportFactoryTest.php +++ b/Tests/Transport/NullTransportFactoryTest.php @@ -11,6 +11,8 @@ namespace Symfony\Component\Mailer\Tests\Transport; +use Psr\Log\NullLogger; +use Symfony\Component\HttpClient\MockHttpClient; use Symfony\Component\Mailer\Test\TransportFactoryTestCase; use Symfony\Component\Mailer\Transport\Dsn; use Symfony\Component\Mailer\Transport\NullTransport; @@ -19,9 +21,9 @@ class NullTransportFactoryTest extends TransportFactoryTestCase { - public static function getFactory(): TransportFactoryInterface + public function getFactory(): TransportFactoryInterface { - return new NullTransportFactory(self::getDispatcher(), self::getClient(), self::getLogger()); + return new NullTransportFactory(null, new MockHttpClient(), new NullLogger()); } public static function supportsProvider(): iterable @@ -36,7 +38,7 @@ public static function createProvider(): iterable { yield [ new Dsn('null', 'null'), - new NullTransport(self::getDispatcher(), self::getLogger()), + new NullTransport(null, new NullLogger()), ]; } } diff --git a/Tests/Transport/SendmailTransportFactoryTest.php b/Tests/Transport/SendmailTransportFactoryTest.php index d13f23e..a3d08f5 100644 --- a/Tests/Transport/SendmailTransportFactoryTest.php +++ b/Tests/Transport/SendmailTransportFactoryTest.php @@ -11,6 +11,8 @@ namespace Symfony\Component\Mailer\Tests\Transport; +use Psr\Log\NullLogger; +use Symfony\Component\HttpClient\MockHttpClient; use Symfony\Component\Mailer\Test\TransportFactoryTestCase; use Symfony\Component\Mailer\Transport\Dsn; use Symfony\Component\Mailer\Transport\SendmailTransport; @@ -19,9 +21,9 @@ class SendmailTransportFactoryTest extends TransportFactoryTestCase { - public static function getFactory(): TransportFactoryInterface + public function getFactory(): TransportFactoryInterface { - return new SendmailTransportFactory(self::getDispatcher(), self::getClient(), self::getLogger()); + return new SendmailTransportFactory(null, new MockHttpClient(), new NullLogger()); } public static function supportsProvider(): iterable @@ -36,12 +38,12 @@ public static function createProvider(): iterable { yield [ new Dsn('sendmail+smtp', 'default'), - new SendmailTransport(null, self::getDispatcher(), self::getLogger()), + new SendmailTransport(null, null, new NullLogger()), ]; yield [ new Dsn('sendmail+smtp', 'default', null, null, null, ['command' => '/usr/sbin/sendmail -oi -t']), - new SendmailTransport('/usr/sbin/sendmail -oi -t', self::getDispatcher(), self::getLogger()), + new SendmailTransport('/usr/sbin/sendmail -oi -t', null, new NullLogger()), ]; } diff --git a/Tests/Transport/Smtp/EsmtpTransportFactoryTest.php b/Tests/Transport/Smtp/EsmtpTransportFactoryTest.php index 65346e2..bcdf669 100644 --- a/Tests/Transport/Smtp/EsmtpTransportFactoryTest.php +++ b/Tests/Transport/Smtp/EsmtpTransportFactoryTest.php @@ -11,6 +11,8 @@ namespace Symfony\Component\Mailer\Tests\Transport\Smtp; +use Psr\Log\NullLogger; +use Symfony\Component\HttpClient\MockHttpClient; use Symfony\Component\Mailer\Test\TransportFactoryTestCase; use Symfony\Component\Mailer\Transport\Dsn; use Symfony\Component\Mailer\Transport\Smtp\EsmtpTransport; @@ -20,9 +22,9 @@ class EsmtpTransportFactoryTest extends TransportFactoryTestCase { - public static function getFactory(): TransportFactoryInterface + public function getFactory(): TransportFactoryInterface { - return new EsmtpTransportFactory(self::getDispatcher(), self::getClient(), self::getLogger()); + return new EsmtpTransportFactory(null, new MockHttpClient(), new NullLogger()); } public static function supportsProvider(): iterable @@ -45,17 +47,16 @@ public static function supportsProvider(): iterable public static function createProvider(): iterable { - $eventDispatcher = self::getDispatcher(); - $logger = self::getLogger(); + $logger = new NullLogger(); - $transport = new EsmtpTransport('localhost', 25, false, $eventDispatcher, $logger); + $transport = new EsmtpTransport('localhost', 25, false, null, $logger); yield [ new Dsn('smtp', 'localhost'), $transport, ]; - $transport = new EsmtpTransport('example.com', 99, true, $eventDispatcher, $logger); + $transport = new EsmtpTransport('example.com', 99, true, null, $logger); $transport->setUsername(self::USER); $transport->setPassword(self::PASSWORD); @@ -64,21 +65,21 @@ public static function createProvider(): iterable $transport, ]; - $transport = new EsmtpTransport('example.com', 465, true, $eventDispatcher, $logger); + $transport = new EsmtpTransport('example.com', 465, true, null, $logger); yield [ new Dsn('smtps', 'example.com'), $transport, ]; - $transport = new EsmtpTransport('example.com', 465, true, $eventDispatcher, $logger); + $transport = new EsmtpTransport('example.com', 465, true, null, $logger); yield [ new Dsn('smtps', 'example.com', '', '', 465), $transport, ]; - $transport = new EsmtpTransport('example.com', 465, true, $eventDispatcher, $logger); + $transport = new EsmtpTransport('example.com', 465, true, null, $logger); /** @var SocketStream $stream */ $stream = $transport->getStream(); $streamOptions = $stream->getStreamOptions(); @@ -101,14 +102,14 @@ public static function createProvider(): iterable $transport, ]; - $transport = new EsmtpTransport('example.com', 465, true, $eventDispatcher, $logger); + $transport = new EsmtpTransport('example.com', 465, true, null, $logger); yield [ Dsn::fromString('smtps://:@example.com?verify_peer='), $transport, ]; - $transport = new EsmtpTransport('example.com', 465, true, $eventDispatcher, $logger); + $transport = new EsmtpTransport('example.com', 465, true, null, $logger); $transport->setLocalDomain('example.com'); yield [ @@ -116,7 +117,7 @@ public static function createProvider(): iterable $transport, ]; - $transport = new EsmtpTransport('example.com', 465, true, $eventDispatcher, $logger); + $transport = new EsmtpTransport('example.com', 465, true, null, $logger); $transport->setMaxPerSecond(2.0); yield [ @@ -124,7 +125,7 @@ public static function createProvider(): iterable $transport, ]; - $transport = new EsmtpTransport('example.com', 465, true, $eventDispatcher, $logger); + $transport = new EsmtpTransport('example.com', 465, true, null, $logger); $transport->setRestartThreshold(10, 1); yield [ @@ -132,7 +133,7 @@ public static function createProvider(): iterable $transport, ]; - $transport = new EsmtpTransport('example.com', 465, true, $eventDispatcher, $logger); + $transport = new EsmtpTransport('example.com', 465, true, null, $logger); $transport->setPingThreshold(10); yield [