From d28bd44898d2dcc5e3b1c0d4eca891e772f012c4 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Tue, 24 Jan 2023 15:02:24 +0100 Subject: [PATCH 1/8] Update license years (last time) --- LICENSE | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/LICENSE b/LICENSE index 5c7ba05..f37c76b 100644 --- a/LICENSE +++ b/LICENSE @@ -1,4 +1,4 @@ -Copyright (c) 2019-2023 Fabien Potencier +Copyright (c) 2019-present Fabien Potencier Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal From ac7d182031a905a3963fa64e1fc620f1fe7b1cec Mon Sep 17 00:00:00 2001 From: Oskar Stark Date: Wed, 14 Dec 2022 15:42:16 +0100 Subject: [PATCH 2/8] Migrate to `static` data providers using `rector/rector` --- Tests/EventListener/MessageListenerTest.php | 2 +- Tests/Exception/UnsupportedSchemeExceptionTest.php | 4 ++-- Tests/Transport/DsnTest.php | 4 ++-- Tests/Transport/NativeTransportFactoryTest.php | 4 ++-- Tests/Transport/Smtp/Stream/AbstractStreamTest.php | 2 +- Tests/TransportTest.php | 6 +++--- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Tests/EventListener/MessageListenerTest.php b/Tests/EventListener/MessageListenerTest.php index 2b2e5df..5f5def7 100644 --- a/Tests/EventListener/MessageListenerTest.php +++ b/Tests/EventListener/MessageListenerTest.php @@ -36,7 +36,7 @@ public function testHeaders(Headers $initialHeaders, Headers $defaultHeaders, He $this->assertEquals($expectedHeaders, $event->getMessage()->getHeaders()); } - public function provideHeaders(): iterable + public static function provideHeaders(): iterable { $initialHeaders = new Headers(); $defaultHeaders = (new Headers()) diff --git a/Tests/Exception/UnsupportedSchemeExceptionTest.php b/Tests/Exception/UnsupportedSchemeExceptionTest.php index 54ce6db..9a7abad 100644 --- a/Tests/Exception/UnsupportedSchemeExceptionTest.php +++ b/Tests/Exception/UnsupportedSchemeExceptionTest.php @@ -59,7 +59,7 @@ public function testMessageWhereSchemeIsPartOfSchemeToPackageMap(string $scheme, ); } - public function messageWhereSchemeIsPartOfSchemeToPackageMapProvider(): \Generator + public static function messageWhereSchemeIsPartOfSchemeToPackageMapProvider(): \Generator { yield ['gmail', 'symfony/google-mailer']; yield ['mailgun', 'symfony/mailgun-mailer']; @@ -83,7 +83,7 @@ public function testMessageWhereSchemeIsNotPartOfSchemeToPackageMap(string $expe ); } - public function messageWhereSchemeIsNotPartOfSchemeToPackageMapProvider(): \Generator + public static function messageWhereSchemeIsNotPartOfSchemeToPackageMapProvider(): \Generator { yield [ 'The "somethingElse" scheme is not supported.', diff --git a/Tests/Transport/DsnTest.php b/Tests/Transport/DsnTest.php index 2816333..f3c0a0b 100644 --- a/Tests/Transport/DsnTest.php +++ b/Tests/Transport/DsnTest.php @@ -45,7 +45,7 @@ public function testInvalidDsn(string $dsn, string $exceptionMessage) Dsn::fromString($dsn); } - public function fromStringProvider(): iterable + public static function fromStringProvider(): iterable { yield 'simple smtp without user and pass' => [ 'smtp://example.com', @@ -88,7 +88,7 @@ public function fromStringProvider(): iterable ]; } - public function invalidDsnProvider(): iterable + public static function invalidDsnProvider(): iterable { yield [ 'some://', diff --git a/Tests/Transport/NativeTransportFactoryTest.php b/Tests/Transport/NativeTransportFactoryTest.php index 556a01a..495fd8c 100644 --- a/Tests/Transport/NativeTransportFactoryTest.php +++ b/Tests/Transport/NativeTransportFactoryTest.php @@ -66,7 +66,7 @@ public function testCreateSendmailWithNoSendmailPath() $sut->create(Dsn::fromString('native://default')); } - public function provideCreateSendmailWithNoHostOrNoPort(): \Generator + public static function provideCreateSendmailWithNoHostOrNoPort(): \Generator { yield ['native://default', '', '', '']; yield ['native://default', '', 'localhost', '']; @@ -95,7 +95,7 @@ public function testCreateSendmailWithNoHostOrNoPort(string $dsn, string $sendma $sut->create(Dsn::fromString($dsn)); } - public function provideCreate(): \Generator + public static function provideCreate(): \Generator { yield ['native://default', '/usr/sbin/sendmail -t -i', '', '', new SendmailTransport('/usr/sbin/sendmail -t -i')]; diff --git a/Tests/Transport/Smtp/Stream/AbstractStreamTest.php b/Tests/Transport/Smtp/Stream/AbstractStreamTest.php index cc901cc..aeb2834 100644 --- a/Tests/Transport/Smtp/Stream/AbstractStreamTest.php +++ b/Tests/Transport/Smtp/Stream/AbstractStreamTest.php @@ -29,7 +29,7 @@ public function testReplace(string $expected, string $from, string $to, array $c $this->assertSame($expected, $result); } - public function provideReplace() + public static function provideReplace() { yield ['ca', 'ab', 'c', ['a', 'b', 'a']]; yield ['ac', 'ab', 'c', ['a', 'ab']]; diff --git a/Tests/TransportTest.php b/Tests/TransportTest.php index 690b0ee..3ffd706 100644 --- a/Tests/TransportTest.php +++ b/Tests/TransportTest.php @@ -34,7 +34,7 @@ public function testFromString(string $dsn, TransportInterface $transport) $this->assertEquals($transport, $transportFactory->fromString($dsn)); } - public function fromStringProvider(): iterable + public static function fromStringProvider(): iterable { $transportA = new DummyTransport('a'); $transportB = new DummyTransport('b'); @@ -68,7 +68,7 @@ public function testFromDsn(string $dsn, TransportInterface $transport) $this->assertEquals($transport, Transport::fromDsn($dsn)); } - public function fromDsnProvider(): iterable + public static function fromDsnProvider(): iterable { yield 'multiple transports' => [ 'failover(smtp://a smtp://b)', @@ -88,7 +88,7 @@ public function testFromWrongString(string $dsn, string $error) $transportFactory->fromString($dsn); } - public function fromWrongStringProvider(): iterable + public static function fromWrongStringProvider(): iterable { yield 'garbage at the end' => ['dummy://a some garbage here', 'The DSN has some garbage at the end: " some garbage here".']; From d4861a43afe5fcbe5fdf61844e9ef8f276f0f1b3 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Tue, 14 Feb 2023 15:04:49 +0100 Subject: [PATCH 3/8] use proper methods to assert exception messages contain certain strings --- Tests/Transport/NativeTransportFactoryTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Transport/NativeTransportFactoryTest.php b/Tests/Transport/NativeTransportFactoryTest.php index 495fd8c..c253b4c 100644 --- a/Tests/Transport/NativeTransportFactoryTest.php +++ b/Tests/Transport/NativeTransportFactoryTest.php @@ -47,7 +47,7 @@ function ini_get(\$key) public function testCreateWithNotSupportedScheme() { $this->expectException(UnsupportedSchemeException::class); - $this->expectErrorMessageMatches('#The ".*" scheme is not supported#'); + $this->expectExceptionMessage('The "sendmail" scheme is not supported'); $sut = new NativeTransportFactory(); $sut->create(Dsn::fromString('sendmail://default')); From 44ec77a8fa5c085bb893e3d423101b49adbb6d10 Mon Sep 17 00:00:00 2001 From: Oskar Stark Date: Tue, 14 Feb 2023 10:58:00 +0100 Subject: [PATCH 4/8] [BC Break] Make data providers for abstract test cases static --- CHANGELOG.md | 8 ++++++++ Test/TransportFactoryTestCase.php | 8 ++++---- Tests/Transport/NullTransportFactoryTest.php | 4 ++-- Tests/Transport/SendmailTransportFactoryTest.php | 6 +++--- Tests/Transport/Smtp/EsmtpTransportFactoryTest.php | 4 ++-- 5 files changed, 19 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cc6cd6f..678acf4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,14 @@ CHANGELOG ========= +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 c2426a0..460f0d1 100644 --- a/Test/TransportFactoryTestCase.php +++ b/Test/TransportFactoryTestCase.php @@ -37,16 +37,16 @@ abstract class TransportFactoryTestCase extends TestCase abstract public function getFactory(): TransportFactoryInterface; - abstract public function supportsProvider(): iterable; + abstract public static function supportsProvider(): iterable; - abstract public function createProvider(): iterable; + abstract public static function createProvider(): iterable; - public function unsupportedSchemeProvider(): iterable + public static function unsupportedSchemeProvider(): iterable { return []; } - public function incompleteDsnProvider(): iterable + public static function incompleteDsnProvider(): iterable { return []; } diff --git a/Tests/Transport/NullTransportFactoryTest.php b/Tests/Transport/NullTransportFactoryTest.php index 9b39a61..13a8a07 100644 --- a/Tests/Transport/NullTransportFactoryTest.php +++ b/Tests/Transport/NullTransportFactoryTest.php @@ -24,7 +24,7 @@ public function getFactory(): TransportFactoryInterface return new NullTransportFactory($this->getDispatcher(), $this->getClient(), $this->getLogger()); } - public function supportsProvider(): iterable + public static function supportsProvider(): iterable { yield [ new Dsn('null', ''), @@ -32,7 +32,7 @@ public function supportsProvider(): iterable ]; } - public function createProvider(): iterable + public static function createProvider(): iterable { yield [ new Dsn('null', 'null'), diff --git a/Tests/Transport/SendmailTransportFactoryTest.php b/Tests/Transport/SendmailTransportFactoryTest.php index 55f893b..d8b1777 100644 --- a/Tests/Transport/SendmailTransportFactoryTest.php +++ b/Tests/Transport/SendmailTransportFactoryTest.php @@ -24,7 +24,7 @@ public function getFactory(): TransportFactoryInterface return new SendmailTransportFactory($this->getDispatcher(), $this->getClient(), $this->getLogger()); } - public function supportsProvider(): iterable + public static function supportsProvider(): iterable { yield [ new Dsn('sendmail+smtp', 'default'), @@ -32,7 +32,7 @@ public function supportsProvider(): iterable ]; } - public function createProvider(): iterable + public static function createProvider(): iterable { yield [ new Dsn('sendmail+smtp', 'default'), @@ -45,7 +45,7 @@ public function createProvider(): iterable ]; } - public function unsupportedSchemeProvider(): iterable + public static function unsupportedSchemeProvider(): iterable { yield [ new Dsn('sendmail+http', 'default'), diff --git a/Tests/Transport/Smtp/EsmtpTransportFactoryTest.php b/Tests/Transport/Smtp/EsmtpTransportFactoryTest.php index 5d75f15..c786766 100644 --- a/Tests/Transport/Smtp/EsmtpTransportFactoryTest.php +++ b/Tests/Transport/Smtp/EsmtpTransportFactoryTest.php @@ -25,7 +25,7 @@ public function getFactory(): TransportFactoryInterface return new EsmtpTransportFactory($this->getDispatcher(), $this->getClient(), $this->getLogger()); } - public function supportsProvider(): iterable + public static function supportsProvider(): iterable { yield [ new Dsn('smtp', 'example.com'), @@ -43,7 +43,7 @@ public function supportsProvider(): iterable ]; } - public function createProvider(): iterable + public static function createProvider(): iterable { $eventDispatcher = $this->getDispatcher(); $logger = $this->getLogger(); From 82f4321ba128783d24dd2ef163aaf54298f40a29 Mon Sep 17 00:00:00 2001 From: Alexandre Daubois Date: Thu, 16 Feb 2023 19:53:58 +0100 Subject: [PATCH 5/8] [Translation][Mailer] Convert `$this` calls to static ones in data providers --- CHANGELOG.md | 4 +-- Test/TransportFactoryTestCase.php | 26 ++++++++++++------- Tests/Transport/NullTransportFactoryTest.php | 6 ++--- .../SendmailTransportFactoryTest.php | 8 +++--- .../Smtp/EsmtpTransportFactoryTest.php | 8 +++--- composer.json | 2 +- 6 files changed, 30 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 678acf4..cd9c3d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,9 @@ CHANGELOG 5.4.21 ------ -* [BC BREAK] The following data providers for `TransportFactoryTestCase` are now static: + * [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: + * [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 460f0d1..d125531 100644 --- a/Test/TransportFactoryTestCase.php +++ b/Test/TransportFactoryTestCase.php @@ -13,6 +13,8 @@ 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; @@ -31,11 +33,11 @@ abstract class TransportFactoryTestCase extends TestCase protected const USER = 'u$er'; protected const PASSWORD = 'pa$s'; - protected $dispatcher; - protected $client; - protected $logger; + protected static $dispatcher; + protected static $client; + protected static $logger; - abstract public function getFactory(): TransportFactoryInterface; + abstract public static function getFactory(): TransportFactoryInterface; abstract public static function supportsProvider(): iterable; @@ -100,18 +102,22 @@ public function testIncompleteDsnException(Dsn $dsn) $factory->create($dsn); } - protected function getDispatcher(): EventDispatcherInterface + protected static function getDispatcher(): EventDispatcherInterface { - return $this->dispatcher ?? $this->dispatcher = $this->createMock(EventDispatcherInterface::class); + return self::$dispatcher ?? self::$dispatcher = new class() implements EventDispatcherInterface { + public function dispatch($event, string $eventName = null): object + { + } + }; } - protected function getClient(): HttpClientInterface + protected static function getClient(): HttpClientInterface { - return $this->client ?? $this->client = $this->createMock(HttpClientInterface::class); + return self::$client ?? self::$client = new MockHttpClient(); } - protected function getLogger(): LoggerInterface + protected static function getLogger(): LoggerInterface { - return $this->logger ?? $this->logger = $this->createMock(LoggerInterface::class); + return self::$logger ?? self::$logger = new NullLogger(); } } diff --git a/Tests/Transport/NullTransportFactoryTest.php b/Tests/Transport/NullTransportFactoryTest.php index 13a8a07..50c35cb 100644 --- a/Tests/Transport/NullTransportFactoryTest.php +++ b/Tests/Transport/NullTransportFactoryTest.php @@ -19,9 +19,9 @@ class NullTransportFactoryTest extends TransportFactoryTestCase { - public function getFactory(): TransportFactoryInterface + public static function getFactory(): TransportFactoryInterface { - return new NullTransportFactory($this->getDispatcher(), $this->getClient(), $this->getLogger()); + return new NullTransportFactory(self::getDispatcher(), self::getClient(), self::getLogger()); } public static function supportsProvider(): iterable @@ -36,7 +36,7 @@ public static function createProvider(): iterable { yield [ new Dsn('null', 'null'), - new NullTransport($this->getDispatcher(), $this->getLogger()), + new NullTransport(self::getDispatcher(), self::getLogger()), ]; } } diff --git a/Tests/Transport/SendmailTransportFactoryTest.php b/Tests/Transport/SendmailTransportFactoryTest.php index d8b1777..d13f23e 100644 --- a/Tests/Transport/SendmailTransportFactoryTest.php +++ b/Tests/Transport/SendmailTransportFactoryTest.php @@ -19,9 +19,9 @@ class SendmailTransportFactoryTest extends TransportFactoryTestCase { - public function getFactory(): TransportFactoryInterface + public static function getFactory(): TransportFactoryInterface { - return new SendmailTransportFactory($this->getDispatcher(), $this->getClient(), $this->getLogger()); + return new SendmailTransportFactory(self::getDispatcher(), self::getClient(), self::getLogger()); } public static function supportsProvider(): iterable @@ -36,12 +36,12 @@ public static function createProvider(): iterable { yield [ new Dsn('sendmail+smtp', 'default'), - new SendmailTransport(null, $this->getDispatcher(), $this->getLogger()), + new SendmailTransport(null, self::getDispatcher(), self::getLogger()), ]; yield [ new Dsn('sendmail+smtp', 'default', null, null, null, ['command' => '/usr/sbin/sendmail -oi -t']), - new SendmailTransport('/usr/sbin/sendmail -oi -t', $this->getDispatcher(), $this->getLogger()), + new SendmailTransport('/usr/sbin/sendmail -oi -t', self::getDispatcher(), self::getLogger()), ]; } diff --git a/Tests/Transport/Smtp/EsmtpTransportFactoryTest.php b/Tests/Transport/Smtp/EsmtpTransportFactoryTest.php index c786766..5c284d1 100644 --- a/Tests/Transport/Smtp/EsmtpTransportFactoryTest.php +++ b/Tests/Transport/Smtp/EsmtpTransportFactoryTest.php @@ -20,9 +20,9 @@ class EsmtpTransportFactoryTest extends TransportFactoryTestCase { - public function getFactory(): TransportFactoryInterface + public static function getFactory(): TransportFactoryInterface { - return new EsmtpTransportFactory($this->getDispatcher(), $this->getClient(), $this->getLogger()); + return new EsmtpTransportFactory(self::getDispatcher(), self::getClient(), self::getLogger()); } public static function supportsProvider(): iterable @@ -45,8 +45,8 @@ public static function supportsProvider(): iterable public static function createProvider(): iterable { - $eventDispatcher = $this->getDispatcher(); - $logger = $this->getLogger(); + $eventDispatcher = self::getDispatcher(); + $logger = self::getLogger(); $transport = new EsmtpTransport('localhost', 25, false, $eventDispatcher, $logger); diff --git a/composer.json b/composer.json index 86093d8..42416e1 100644 --- a/composer.json +++ b/composer.json @@ -27,7 +27,7 @@ "symfony/service-contracts": "^1.1|^2|^3" }, "require-dev": { - "symfony/http-client-contracts": "^1.1|^2|^3", + "symfony/http-client": "^4.4|^5.0|^6.0", "symfony/messenger": "^4.4|^5.0|^6.0" }, "conflict": { From e4f84c633b72ec70efc50b8016871c3bc43e691e Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 21 Feb 2023 11:33:08 +0100 Subject: [PATCH 6/8] 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 [ From 60c5f5a29399ff591fadd99da345a4a8bf048c41 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 21 Feb 2023 11:33:08 +0100 Subject: [PATCH 7/8] 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 | 2 -- Test/TransportFactoryTestCase.php | 26 +++++++----------- Tests/Transport/NullTransportFactoryTest.php | 8 +++--- .../SendmailTransportFactoryTest.php | 10 ++++--- .../Smtp/EsmtpTransportFactoryTest.php | 27 ++++++++++--------- 5 files changed, 35 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cd9c3d1..bdeffe4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,6 @@ CHANGELOG * [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 d125531..121643f 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 ?? self::$dispatcher = new class() implements EventDispatcherInterface { - public function dispatch($event, string $eventName = null): object - { - } - }; + return $this->dispatcher ?? $this->dispatcher = $this->createMock(EventDispatcherInterface::class); } - protected static function getClient(): HttpClientInterface + protected function getClient(): HttpClientInterface { - return self::$client ?? self::$client = new MockHttpClient(); + return $this->client ?? $this->client = $this->createMock(HttpClientInterface::class); } - protected static function getLogger(): LoggerInterface + protected function getLogger(): LoggerInterface { - return self::$logger ?? self::$logger = new NullLogger(); + return $this->logger ?? $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 5c284d1..4978935 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->setRestartThreshold(10, 1); 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->setPingThreshold(10); yield [ From 6330cd465dfd8b7a07515757a1c37069075f7b0b Mon Sep 17 00:00:00 2001 From: Tema Yud Date: Sun, 5 Mar 2023 22:32:25 +0500 Subject: [PATCH 8/8] [Mailer] STDOUT blocks infinitely under Windows when STDERR is filled --- Transport/Smtp/Stream/ProcessStream.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Transport/Smtp/Stream/ProcessStream.php b/Transport/Smtp/Stream/ProcessStream.php index a8a8603..bc721ad 100644 --- a/Transport/Smtp/Stream/ProcessStream.php +++ b/Transport/Smtp/Stream/ProcessStream.php @@ -35,7 +35,7 @@ public function initialize(): void $descriptorSpec = [ 0 => ['pipe', 'r'], 1 => ['pipe', 'w'], - 2 => ['pipe', 'w'], + 2 => ['pipe', '\\' === \DIRECTORY_SEPARATOR ? 'a' : 'w'], ]; $pipes = []; $this->stream = proc_open($this->command, $descriptorSpec, $pipes);