Skip to content

Commit

Permalink
minor #49431 [Mailer][Translation] Remove some static occurrences t…
Browse files Browse the repository at this point in the history
…hat 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
  • Loading branch information
nicolas-grekas committed Feb 21, 2023
1 parent 6d4bac5 commit c486fb0
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 11 deletions.
23 changes: 12 additions & 11 deletions Tests/Transport/PostmarkTransportFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

namespace Symfony\Component\Mailer\Bridge\Postmark\Tests\Transport;

use Psr\Log\NullLogger;
use Symfony\Component\HttpClient\MockHttpClient;
use Symfony\Component\Mailer\Bridge\Postmark\Transport\PostmarkApiTransport;
use Symfony\Component\Mailer\Bridge\Postmark\Transport\PostmarkSmtpTransport;
use Symfony\Component\Mailer\Bridge\Postmark\Transport\PostmarkTransportFactory;
Expand All @@ -20,9 +22,9 @@

class PostmarkTransportFactoryTest extends TransportFactoryTestCase
{
public static function getFactory(): TransportFactoryInterface
public function getFactory(): TransportFactoryInterface
{
return new PostmarkTransportFactory(self::getDispatcher(), self::getClient(), self::getLogger());
return new PostmarkTransportFactory(null, new MockHttpClient(), new NullLogger());
}

public static function supportsProvider(): iterable
Expand Down Expand Up @@ -55,42 +57,41 @@ public static function supportsProvider(): iterable

public static function createProvider(): iterable
{
$dispatcher = self::getDispatcher();
$logger = self::getLogger();
$logger = new NullLogger();

yield [
new Dsn('postmark+api', 'default', self::USER),
new PostmarkApiTransport(self::USER, self::getClient(), $dispatcher, $logger),
new PostmarkApiTransport(self::USER, new MockHttpClient(), null, $logger),
];

yield [
new Dsn('postmark+api', 'example.com', self::USER, '', 8080),
(new PostmarkApiTransport(self::USER, self::getClient(), $dispatcher, $logger))->setHost('example.com')->setPort(8080),
(new PostmarkApiTransport(self::USER, new MockHttpClient(), null, $logger))->setHost('example.com')->setPort(8080),
];

yield [
new Dsn('postmark+api', 'example.com', self::USER, '', 8080, ['message_stream' => 'broadcasts']),
(new PostmarkApiTransport(self::USER, self::getClient(), $dispatcher, $logger))->setHost('example.com')->setPort(8080)->setMessageStream('broadcasts'),
(new PostmarkApiTransport(self::USER, new MockHttpClient(), null, $logger))->setHost('example.com')->setPort(8080)->setMessageStream('broadcasts'),
];

yield [
new Dsn('postmark', 'default', self::USER),
new PostmarkSmtpTransport(self::USER, $dispatcher, $logger),
new PostmarkSmtpTransport(self::USER, null, $logger),
];

yield [
new Dsn('postmark+smtp', 'default', self::USER),
new PostmarkSmtpTransport(self::USER, $dispatcher, $logger),
new PostmarkSmtpTransport(self::USER, null, $logger),
];

yield [
new Dsn('postmark+smtps', 'default', self::USER),
new PostmarkSmtpTransport(self::USER, $dispatcher, $logger),
new PostmarkSmtpTransport(self::USER, null, $logger),
];

yield [
new Dsn('postmark+smtps', 'default', self::USER, null, null, ['message_stream' => 'broadcasts']),
(new PostmarkSmtpTransport(self::USER, $dispatcher, $logger))->setMessageStream('broadcasts'),
(new PostmarkSmtpTransport(self::USER, null, $logger))->setMessageStream('broadcasts'),
];
}

Expand Down
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
],
"require": {
"php": ">=8.1",
"psr/event-dispatcher": "^1",
"symfony/mailer": "^5.4.21|^6.2.7"
},
"require-dev": {
Expand Down

0 comments on commit c486fb0

Please sign in to comment.