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
-------

2ca9cf8 [Mailer][Translation][Notifier] Remove some `static` occurrences that may cause unstable tests
  • Loading branch information
nicolas-grekas committed Feb 21, 2023
1 parent 6627291 commit dee9e76
Show file tree
Hide file tree
Showing 18 changed files with 263 additions and 258 deletions.
Expand Up @@ -13,6 +13,8 @@

use AsyncAws\Core\Configuration;
use AsyncAws\Ses\SesClient;
use Psr\Log\NullLogger;
use Symfony\Component\HttpClient\MockHttpClient;
use Symfony\Component\Mailer\Bridge\Amazon\Transport\SesApiAsyncAwsTransport;
use Symfony\Component\Mailer\Bridge\Amazon\Transport\SesHttpAsyncAwsTransport;
use Symfony\Component\Mailer\Bridge\Amazon\Transport\SesSmtpTransport;
Expand All @@ -23,9 +25,9 @@

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

public static function supportsProvider(): iterable
Expand Down Expand Up @@ -63,98 +65,97 @@ public static function supportsProvider(): iterable

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

yield [
new Dsn('ses+api', 'default', self::USER, self::PASSWORD),
new SesApiAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-1']), null, $client, $logger), $dispatcher, $logger),
new SesApiAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-1']), null, $client, $logger), null, $logger),
];

yield [
new Dsn('ses+api', 'default', self::USER, self::PASSWORD, null, ['region' => 'eu-west-2']),
new SesApiAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-2']), null, $client, $logger), $dispatcher, $logger),
new SesApiAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-2']), null, $client, $logger), null, $logger),
];

yield [
new Dsn('ses+api', 'example.com', self::USER, self::PASSWORD, 8080),
new SesApiAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-1', 'endpoint' => 'https://example.com:8080']), null, $client, $logger), $dispatcher, $logger),
new SesApiAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-1', 'endpoint' => 'https://example.com:8080']), null, $client, $logger), null, $logger),
];

yield [
new Dsn('ses+api', 'default', self::USER, self::PASSWORD, null, ['session_token' => 'se$sion']),
new SesApiAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-1', 'sessionToken' => 'se$sion']), null, $client, $logger), $dispatcher, $logger),
new SesApiAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-1', 'sessionToken' => 'se$sion']), null, $client, $logger), null, $logger),
];

yield [
new Dsn('ses+api', 'default', self::USER, self::PASSWORD, null, ['region' => 'eu-west-2', 'session_token' => 'se$sion']),
new SesApiAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-2', 'sessionToken' => 'se$sion']), null, $client, $logger), $dispatcher, $logger),
new SesApiAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-2', 'sessionToken' => 'se$sion']), null, $client, $logger), null, $logger),
];

yield [
new Dsn('ses+api', 'example.com', self::USER, self::PASSWORD, 8080, ['session_token' => 'se$sion']),
new SesApiAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-1', 'endpoint' => 'https://example.com:8080', 'sessionToken' => 'se$sion']), null, $client, $logger), $dispatcher, $logger),
new SesApiAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-1', 'endpoint' => 'https://example.com:8080', 'sessionToken' => 'se$sion']), null, $client, $logger), null, $logger),
];

yield [
new Dsn('ses+https', 'default', self::USER, self::PASSWORD),
new SesHttpAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-1']), null, $client, $logger), $dispatcher, $logger),
new SesHttpAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-1']), null, $client, $logger), null, $logger),
];

yield [
new Dsn('ses', 'default', self::USER, self::PASSWORD),
new SesHttpAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-1']), null, $client, $logger), $dispatcher, $logger),
new SesHttpAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-1']), null, $client, $logger), null, $logger),
];

yield [
new Dsn('ses+https', 'example.com', self::USER, self::PASSWORD, 8080),
new SesHttpAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-1', 'endpoint' => 'https://example.com:8080']), null, $client, $logger), $dispatcher, $logger),
new SesHttpAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-1', 'endpoint' => 'https://example.com:8080']), null, $client, $logger), null, $logger),
];

yield [
new Dsn('ses+https', 'default', self::USER, self::PASSWORD, null, ['region' => 'eu-west-2']),
new SesHttpAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-2']), null, $client, $logger), $dispatcher, $logger),
new SesHttpAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-2']), null, $client, $logger), null, $logger),
];

yield [
new Dsn('ses+https', 'default', self::USER, self::PASSWORD, null, ['session_token' => 'se$sion']),
new SesHttpAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-1', 'sessionToken' => 'se$sion']), null, $client, $logger), $dispatcher, $logger),
new SesHttpAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-1', 'sessionToken' => 'se$sion']), null, $client, $logger), null, $logger),
];

yield [
new Dsn('ses', 'default', self::USER, self::PASSWORD, null, ['session_token' => 'se$sion']),
new SesHttpAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-1', 'sessionToken' => 'se$sion']), null, $client, $logger), $dispatcher, $logger),
new SesHttpAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-1', 'sessionToken' => 'se$sion']), null, $client, $logger), null, $logger),
];

yield [
new Dsn('ses+https', 'example.com', self::USER, self::PASSWORD, 8080, ['session_token' => 'se$sion']),
new SesHttpAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-1', 'endpoint' => 'https://example.com:8080', 'sessionToken' => 'se$sion']), null, $client, $logger), $dispatcher, $logger),
new SesHttpAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-1', 'endpoint' => 'https://example.com:8080', 'sessionToken' => 'se$sion']), null, $client, $logger), null, $logger),
];

yield [
new Dsn('ses+https', 'default', self::USER, self::PASSWORD, null, ['region' => 'eu-west-2', 'session_token' => 'se$sion']),
new SesHttpAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-2', 'sessionToken' => 'se$sion']), null, $client, $logger), $dispatcher, $logger),
new SesHttpAsyncAwsTransport(new SesClient(Configuration::create(['accessKeyId' => self::USER, 'accessKeySecret' => self::PASSWORD, 'region' => 'eu-west-2', 'sessionToken' => 'se$sion']), null, $client, $logger), null, $logger),
];

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

yield [
new Dsn('ses+smtp', 'default', self::USER, self::PASSWORD, null, ['region' => 'eu-west-1']),
new SesSmtpTransport(self::USER, self::PASSWORD, 'eu-west-1', $dispatcher, $logger),
new SesSmtpTransport(self::USER, self::PASSWORD, 'eu-west-1', null, $logger),
];

yield [
new Dsn('ses+smtps', 'default', self::USER, self::PASSWORD, null, ['region' => 'eu-west-1']),
new SesSmtpTransport(self::USER, self::PASSWORD, 'eu-west-1', $dispatcher, $logger),
new SesSmtpTransport(self::USER, self::PASSWORD, 'eu-west-1', null, $logger),
];

yield [
new Dsn('ses+smtps', 'default', self::USER, self::PASSWORD, null, ['region' => 'eu-west-1', 'ping_threshold' => '10']),
(new SesSmtpTransport(self::USER, self::PASSWORD, 'eu-west-1', $dispatcher, $logger))->setPingThreshold(10),
(new SesSmtpTransport(self::USER, self::PASSWORD, 'eu-west-1', null, $logger))->setPingThreshold(10),
];
}

Expand Down
Expand Up @@ -11,6 +11,8 @@

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

use Psr\Log\NullLogger;
use Symfony\Component\HttpClient\MockHttpClient;
use Symfony\Component\Mailer\Bridge\Google\Transport\GmailSmtpTransport;
use Symfony\Component\Mailer\Bridge\Google\Transport\GmailTransportFactory;
use Symfony\Component\Mailer\Test\TransportFactoryTestCase;
Expand All @@ -19,9 +21,9 @@

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

public static function supportsProvider(): iterable
Expand Down Expand Up @@ -51,17 +53,17 @@ public static function createProvider(): iterable
{
yield [
new Dsn('gmail', 'default', self::USER, self::PASSWORD),
new GmailSmtpTransport(self::USER, self::PASSWORD, self::getDispatcher(), self::getLogger()),
new GmailSmtpTransport(self::USER, self::PASSWORD, null, new NullLogger()),
];

yield [
new Dsn('gmail+smtp', 'default', self::USER, self::PASSWORD),
new GmailSmtpTransport(self::USER, self::PASSWORD, self::getDispatcher(), self::getLogger()),
new GmailSmtpTransport(self::USER, self::PASSWORD, null, new NullLogger()),
];

yield [
new Dsn('gmail+smtps', 'default', self::USER, self::PASSWORD),
new GmailSmtpTransport(self::USER, self::PASSWORD, self::getDispatcher(), self::getLogger()),
new GmailSmtpTransport(self::USER, self::PASSWORD, null, new NullLogger()),
];
}

Expand Down
Expand Up @@ -11,6 +11,8 @@

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

use Psr\Log\NullLogger;
use Symfony\Component\HttpClient\MockHttpClient;
use Symfony\Component\Mailer\Bridge\Mailchimp\Transport\MandrillApiTransport;
use Symfony\Component\Mailer\Bridge\Mailchimp\Transport\MandrillHttpTransport;
use Symfony\Component\Mailer\Bridge\Mailchimp\Transport\MandrillSmtpTransport;
Expand All @@ -21,9 +23,9 @@

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

public static function supportsProvider(): iterable
Expand Down Expand Up @@ -61,43 +63,42 @@ public static function supportsProvider(): iterable

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

yield [
new Dsn('mandrill+api', 'default', self::USER),
new MandrillApiTransport(self::USER, $client, $dispatcher, $logger),
new MandrillApiTransport(self::USER, $client, null, $logger),
];

yield [
new Dsn('mandrill+api', 'example.com', self::USER, '', 8080),
(new MandrillApiTransport(self::USER, $client, $dispatcher, $logger))->setHost('example.com')->setPort(8080),
(new MandrillApiTransport(self::USER, $client, null, $logger))->setHost('example.com')->setPort(8080),
];

yield [
new Dsn('mandrill', 'default', self::USER),
new MandrillHttpTransport(self::USER, $client, $dispatcher, $logger),
new MandrillHttpTransport(self::USER, $client, null, $logger),
];

yield [
new Dsn('mandrill+https', 'default', self::USER),
new MandrillHttpTransport(self::USER, $client, $dispatcher, $logger),
new MandrillHttpTransport(self::USER, $client, null, $logger),
];

yield [
new Dsn('mandrill+https', 'example.com', self::USER, '', 8080),
(new MandrillHttpTransport(self::USER, $client, $dispatcher, $logger))->setHost('example.com')->setPort(8080),
(new MandrillHttpTransport(self::USER, $client, null, $logger))->setHost('example.com')->setPort(8080),
];

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

yield [
new Dsn('mandrill+smtps', 'default', self::USER, self::PASSWORD),
new MandrillSmtpTransport(self::USER, self::PASSWORD, $dispatcher, $logger),
new MandrillSmtpTransport(self::USER, self::PASSWORD, null, $logger),
];
}

Expand Down
Expand Up @@ -11,6 +11,8 @@

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

use Psr\Log\NullLogger;
use Symfony\Component\HttpClient\MockHttpClient;
use Symfony\Component\Mailer\Bridge\Mailgun\Transport\MailgunApiTransport;
use Symfony\Component\Mailer\Bridge\Mailgun\Transport\MailgunHttpTransport;
use Symfony\Component\Mailer\Bridge\Mailgun\Transport\MailgunSmtpTransport;
Expand All @@ -21,9 +23,9 @@

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

public static function supportsProvider(): iterable
Expand Down Expand Up @@ -61,48 +63,47 @@ public static function supportsProvider(): iterable

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

yield [
new Dsn('mailgun+api', 'default', self::USER, self::PASSWORD),
new MailgunApiTransport(self::USER, self::PASSWORD, null, $client, $dispatcher, $logger),
new MailgunApiTransport(self::USER, self::PASSWORD, null, $client, null, $logger),
];

yield [
new Dsn('mailgun+api', 'default', self::USER, self::PASSWORD, null, ['region' => 'eu']),
new MailgunApiTransport(self::USER, self::PASSWORD, 'eu', $client, $dispatcher, $logger),
new MailgunApiTransport(self::USER, self::PASSWORD, 'eu', $client, null, $logger),
];

yield [
new Dsn('mailgun+api', 'example.com', self::USER, self::PASSWORD, 8080),
(new MailgunApiTransport(self::USER, self::PASSWORD, null, $client, $dispatcher, $logger))->setHost('example.com')->setPort(8080),
(new MailgunApiTransport(self::USER, self::PASSWORD, null, $client, null, $logger))->setHost('example.com')->setPort(8080),
];

yield [
new Dsn('mailgun', 'default', self::USER, self::PASSWORD),
new MailgunHttpTransport(self::USER, self::PASSWORD, null, $client, $dispatcher, $logger),
new MailgunHttpTransport(self::USER, self::PASSWORD, null, $client, null, $logger),
];

yield [
new Dsn('mailgun+https', 'default', self::USER, self::PASSWORD),
new MailgunHttpTransport(self::USER, self::PASSWORD, null, $client, $dispatcher, $logger),
new MailgunHttpTransport(self::USER, self::PASSWORD, null, $client, null, $logger),
];

yield [
new Dsn('mailgun+https', 'example.com', self::USER, self::PASSWORD, 8080),
(new MailgunHttpTransport(self::USER, self::PASSWORD, null, $client, $dispatcher, $logger))->setHost('example.com')->setPort(8080),
(new MailgunHttpTransport(self::USER, self::PASSWORD, null, $client, null, $logger))->setHost('example.com')->setPort(8080),
];

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

yield [
new Dsn('mailgun+smtps', 'default', self::USER, self::PASSWORD),
new MailgunSmtpTransport(self::USER, self::PASSWORD, null, $dispatcher, $logger),
new MailgunSmtpTransport(self::USER, self::PASSWORD, null, null, $logger),
];
}

Expand Down

0 comments on commit dee9e76

Please sign in to comment.