Skip to content

Commit 60c5f5a

Browse files
nicolas-grekasnicolas-grekas
committed
minor #49431 [Mailer][Translation] Remove some static occurrences that may cause unstable tests (alexandre-daubois)
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
1 parent 82f4321 commit 60c5f5a

File tree

5 files changed

+35
-38
lines changed

5 files changed

+35
-38
lines changed

‎CHANGELOG.md

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ CHANGELOG
66

77
* [BC BREAK] The following data providers for `TransportFactoryTestCase` are now static:
88
`supportsProvider()`, `createProvider()`, `unsupportedSchemeProvider()`and `incompleteDsnProvider()`
9-
* [BC BREAK] The following data providers for `TransportTestCase` are now static:
10-
`toStringProvider()`, `supportedMessagesProvider()` and `unsupportedMessagesProvider()`
119

1210
5.4
1311
---

‎Test/TransportFactoryTestCase.php

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@
1313

1414
use PHPUnit\Framework\TestCase;
1515
use Psr\Log\LoggerInterface;
16-
use Psr\Log\NullLogger;
17-
use Symfony\Component\HttpClient\MockHttpClient;
1816
use Symfony\Component\Mailer\Exception\IncompleteDsnException;
1917
use Symfony\Component\Mailer\Exception\UnsupportedSchemeException;
2018
use Symfony\Component\Mailer\Transport\Dsn;
@@ -33,11 +31,11 @@ abstract class TransportFactoryTestCase extends TestCase
3331
protected const USER = 'u$er';
3432
protected const PASSWORD = 'pa$s';
3533

36-
protected static $dispatcher;
37-
protected static $client;
38-
protected static $logger;
34+
protected $dispatcher;
35+
protected $client;
36+
protected $logger;
3937

40-
abstract public static function getFactory(): TransportFactoryInterface;
38+
abstract public function getFactory(): TransportFactoryInterface;
4139

4240
abstract public static function supportsProvider(): iterable;
4341

@@ -102,22 +100,18 @@ public function testIncompleteDsnException(Dsn $dsn)
102100
$factory->create($dsn);
103101
}
104102

105-
protected static function getDispatcher(): EventDispatcherInterface
103+
protected function getDispatcher(): EventDispatcherInterface
106104
{
107-
return self::$dispatcher ?? self::$dispatcher = new class() implements EventDispatcherInterface {
108-
public function dispatch($event, string $eventName = null): object
109-
{
110-
}
111-
};
105+
return $this->dispatcher ?? $this->dispatcher = $this->createMock(EventDispatcherInterface::class);
112106
}
113107

114-
protected static function getClient(): HttpClientInterface
108+
protected function getClient(): HttpClientInterface
115109
{
116-
return self::$client ?? self::$client = new MockHttpClient();
110+
return $this->client ?? $this->client = $this->createMock(HttpClientInterface::class);
117111
}
118112

119-
protected static function getLogger(): LoggerInterface
113+
protected function getLogger(): LoggerInterface
120114
{
121-
return self::$logger ?? self::$logger = new NullLogger();
115+
return $this->logger ?? $this->logger = $this->createMock(LoggerInterface::class);
122116
}
123117
}

‎Tests/Transport/NullTransportFactoryTest.php

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
namespace Symfony\Component\Mailer\Tests\Transport;
1313

14+
use Psr\Log\NullLogger;
15+
use Symfony\Component\HttpClient\MockHttpClient;
1416
use Symfony\Component\Mailer\Test\TransportFactoryTestCase;
1517
use Symfony\Component\Mailer\Transport\Dsn;
1618
use Symfony\Component\Mailer\Transport\NullTransport;
@@ -19,9 +21,9 @@
1921

2022
class NullTransportFactoryTest extends TransportFactoryTestCase
2123
{
22-
public static function getFactory(): TransportFactoryInterface
24+
public function getFactory(): TransportFactoryInterface
2325
{
24-
return new NullTransportFactory(self::getDispatcher(), self::getClient(), self::getLogger());
26+
return new NullTransportFactory(null, new MockHttpClient(), new NullLogger());
2527
}
2628

2729
public static function supportsProvider(): iterable
@@ -36,7 +38,7 @@ public static function createProvider(): iterable
3638
{
3739
yield [
3840
new Dsn('null', 'null'),
39-
new NullTransport(self::getDispatcher(), self::getLogger()),
41+
new NullTransport(null, new NullLogger()),
4042
];
4143
}
4244
}

‎Tests/Transport/SendmailTransportFactoryTest.php

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
namespace Symfony\Component\Mailer\Tests\Transport;
1313

14+
use Psr\Log\NullLogger;
15+
use Symfony\Component\HttpClient\MockHttpClient;
1416
use Symfony\Component\Mailer\Test\TransportFactoryTestCase;
1517
use Symfony\Component\Mailer\Transport\Dsn;
1618
use Symfony\Component\Mailer\Transport\SendmailTransport;
@@ -19,9 +21,9 @@
1921

2022
class SendmailTransportFactoryTest extends TransportFactoryTestCase
2123
{
22-
public static function getFactory(): TransportFactoryInterface
24+
public function getFactory(): TransportFactoryInterface
2325
{
24-
return new SendmailTransportFactory(self::getDispatcher(), self::getClient(), self::getLogger());
26+
return new SendmailTransportFactory(null, new MockHttpClient(), new NullLogger());
2527
}
2628

2729
public static function supportsProvider(): iterable
@@ -36,12 +38,12 @@ public static function createProvider(): iterable
3638
{
3739
yield [
3840
new Dsn('sendmail+smtp', 'default'),
39-
new SendmailTransport(null, self::getDispatcher(), self::getLogger()),
41+
new SendmailTransport(null, null, new NullLogger()),
4042
];
4143

4244
yield [
4345
new Dsn('sendmail+smtp', 'default', null, null, null, ['command' => '/usr/sbin/sendmail -oi -t']),
44-
new SendmailTransport('/usr/sbin/sendmail -oi -t', self::getDispatcher(), self::getLogger()),
46+
new SendmailTransport('/usr/sbin/sendmail -oi -t', null, new NullLogger()),
4547
];
4648
}
4749

‎Tests/Transport/Smtp/EsmtpTransportFactoryTest.php

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
namespace Symfony\Component\Mailer\Tests\Transport\Smtp;
1313

14+
use Psr\Log\NullLogger;
15+
use Symfony\Component\HttpClient\MockHttpClient;
1416
use Symfony\Component\Mailer\Test\TransportFactoryTestCase;
1517
use Symfony\Component\Mailer\Transport\Dsn;
1618
use Symfony\Component\Mailer\Transport\Smtp\EsmtpTransport;
@@ -20,9 +22,9 @@
2022

2123
class EsmtpTransportFactoryTest extends TransportFactoryTestCase
2224
{
23-
public static function getFactory(): TransportFactoryInterface
25+
public function getFactory(): TransportFactoryInterface
2426
{
25-
return new EsmtpTransportFactory(self::getDispatcher(), self::getClient(), self::getLogger());
27+
return new EsmtpTransportFactory(null, new MockHttpClient(), new NullLogger());
2628
}
2729

2830
public static function supportsProvider(): iterable
@@ -45,17 +47,16 @@ public static function supportsProvider(): iterable
4547

4648
public static function createProvider(): iterable
4749
{
48-
$eventDispatcher = self::getDispatcher();
49-
$logger = self::getLogger();
50+
$logger = new NullLogger();
5051

51-
$transport = new EsmtpTransport('localhost', 25, false, $eventDispatcher, $logger);
52+
$transport = new EsmtpTransport('localhost', 25, false, null, $logger);
5253

5354
yield [
5455
new Dsn('smtp', 'localhost'),
5556
$transport,
5657
];
5758

58-
$transport = new EsmtpTransport('example.com', 99, true, $eventDispatcher, $logger);
59+
$transport = new EsmtpTransport('example.com', 99, true, null, $logger);
5960
$transport->setUsername(self::USER);
6061
$transport->setPassword(self::PASSWORD);
6162

@@ -64,21 +65,21 @@ public static function createProvider(): iterable
6465
$transport,
6566
];
6667

67-
$transport = new EsmtpTransport('example.com', 465, true, $eventDispatcher, $logger);
68+
$transport = new EsmtpTransport('example.com', 465, true, null, $logger);
6869

6970
yield [
7071
new Dsn('smtps', 'example.com'),
7172
$transport,
7273
];
7374

74-
$transport = new EsmtpTransport('example.com', 465, true, $eventDispatcher, $logger);
75+
$transport = new EsmtpTransport('example.com', 465, true, null, $logger);
7576

7677
yield [
7778
new Dsn('smtps', 'example.com', '', '', 465),
7879
$transport,
7980
];
8081

81-
$transport = new EsmtpTransport('example.com', 465, true, $eventDispatcher, $logger);
82+
$transport = new EsmtpTransport('example.com', 465, true, null, $logger);
8283
/** @var SocketStream $stream */
8384
$stream = $transport->getStream();
8485
$streamOptions = $stream->getStreamOptions();
@@ -101,30 +102,30 @@ public static function createProvider(): iterable
101102
$transport,
102103
];
103104

104-
$transport = new EsmtpTransport('example.com', 465, true, $eventDispatcher, $logger);
105+
$transport = new EsmtpTransport('example.com', 465, true, null, $logger);
105106

106107
yield [
107108
Dsn::fromString('smtps://:@example.com?verify_peer='),
108109
$transport,
109110
];
110111

111-
$transport = new EsmtpTransport('example.com', 465, true, $eventDispatcher, $logger);
112+
$transport = new EsmtpTransport('example.com', 465, true, null, $logger);
112113
$transport->setLocalDomain('example.com');
113114

114115
yield [
115116
new Dsn('smtps', 'example.com', '', '', 465, ['local_domain' => 'example.com']),
116117
$transport,
117118
];
118119

119-
$transport = new EsmtpTransport('example.com', 465, true, $eventDispatcher, $logger);
120+
$transport = new EsmtpTransport('example.com', 465, true, null, $logger);
120121
$transport->setRestartThreshold(10, 1);
121122

122123
yield [
123124
new Dsn('smtps', 'example.com', '', '', 465, ['restart_threshold' => '10', 'restart_threshold_sleep' => '1']),
124125
$transport,
125126
];
126127

127-
$transport = new EsmtpTransport('example.com', 465, true, $eventDispatcher, $logger);
128+
$transport = new EsmtpTransport('example.com', 465, true, null, $logger);
128129
$transport->setPingThreshold(10);
129130

130131
yield [

0 commit comments

Comments
 (0)