Codeception / module-yii2

Codeception module for Yii2 framework
MIT License
16 stars 36 forks source link

No longer possible to test with multiple mailers (Yii2) #31

Closed VirtualRJ closed 2 years ago

VirtualRJ commented 5 years ago

What are you trying to achieve?

For my application I have two mailer components defined. The default mailer component with the key 'mailer' is used for application specific mails and uses the company mailserver. The customer mailer (defined as a component 'customerMailer') is a specific configuration for the customer where they can use their own mailserver for sending out communication to their customers. Up until 2.3 I used the TestMailer class to mock this customerMailer as well since it is just a implementation of BaseMailer and exposes everything necessary to test the behavior.

My test.php in the common config directory looks like this and this would give me access to everything necessary for testing by accessing this component:

    'components' => [
         ...
        'customerMailer' => [
            'class' => 'Codeception\Lib\Connector\Yii2\TestMailer',
        ],
    ],

What do you get instead?

Since Codeception/Codeception#5185 a lot of the features were moved from the TestMailer class to the Yii2 Connector class. The Connector class works perfectly fine for the default mailer component. But because it makes a lot of assumptions on using only the default mailer component it is no longer possible to mock different mailer instances with it.

I see no way how to configure this new implementation of TestMailer class to store and handle mails without essentialy extending the TestMailer class and restoring functionality.

Details

class_name: UnitTester
bootstrap: false
modules:
    enabled:
        - Yii2:
            part: [orm, email, fixtures]
SamMousa commented 5 years ago

The features that you need you can still access.

  1. Configure the mailer like in your example.
  2. Fire the request.
  3. Use Yii::$app->customerMailer->getSentMessages() in your assertions.
VirtualRJ commented 5 years ago

This is how I used it like before, but now this throws an UnknownMethodException:

[yii\base\UnknownMethodException] Calling unknown method: Codeception\Lib\Connector\Yii2\TestMailer::getSentMessages()
SamMousa commented 5 years ago

Ah, sry was looking at the wrong branch locally. The new TestMailer classes uses a callback. You can simply do assertions in the callback directly or aggregate mails in your test:

public function testSomeThing(FunctionalTester $I)
{
    $emails = [];
    \Yii::$app->customerMailer->callback = function(MessageInterface $message) use (&$emails) {
        $emails[] = $message;
    };
    $I->assertNotEmpty($emails);
}
VirtualRJ commented 5 years ago

While that is possible it is less than ideal compared to the previous solution with a centralized store. It just adds more complexity and code to tests.

SamMousa commented 5 years ago

We no longer reset the whole application in between requests inside a single test. This allows us to inspect the full application state after a request was handled. Due to this fact it means that we cannot store state inside a component since it will not be cleared for the next request.

If you feel this code is too complicated / you don't want to repeat it in your tests then you could implement this somewhere where it's always available, but note that you'll be responsible for clearing this state in between requests within a single test.

One of the goals of the refactoring was to give stronger guarantees as to what you could inspect before and after a request during a test.

VirtualRJ commented 5 years ago

I've put the initialization code in the UnitTester (might as well be the FunctionalTester) class. Just must not forget to init it before calling the send to be able to actually fetch the mails. This works for my use case now and clearing should be easy enough to implement as well.

class UnitTester extends \Codeception\Actor
{
    use _generated\UnitTesterActions;
    use Asserts;

    public $customerMails = [];

    public function initCustomerMailer()
    {
        if (!Yii::$app->has('customerMailer')) {
            throw new ModuleException($this, "Component customerMailer is not available in current application");
        }

        /** @var TestMailer $mailer */
        $mailer = Yii::$app->get('customerMailer');

        if (empty($mailer->callback)) {
            $mailer->callback = function (MessageInterface $message) {
                $this->customerMails[] = $message;
            };
        }

        return $mailer;
    }

    public function grabSentCustomerEmails()
    {
        $customerMailer = $this->grabCustomerMailer();
        if (!$customerMailer instanceof TestMailer) {
            throw new ModuleException($this, "Customer mailer module is not mocked, can't test emails");
        }
        return $this->customerMails;
    }
}
SamMousa commented 5 years ago

Just out of curiosity, what kind of assertions do you do? Could you show me the code of a test?

VirtualRJ commented 5 years ago

Don't think I'm doing any special assertions. Just checking whether emails have been sent and through the correct channel.

$someAction->getMail()->sendMail();
$this->tester->seeCustomerEmailIsSent(1);
SamMousa commented 5 years ago

One solution that would make it more flexible while at the same time improving the code a bit (currently mocking the mailer is hard-coded):

  1. Add a setting mailerComponents = ['mailer'] to allow configuring custom name(s) for mailer components.
  2. Add an optional parameter to seeEmailIsSent with the name of the mailer: function seeEmailIsSent($num = null, string$mailer = null); this would default to the first mailer in given in mailerComponents.
  3. Do the same for other mail related functions.
  4. Store emails separately in each component id in Codeception\Lib\Connector\Yii2::$emails.
  5. Store last sent email (for getting the last email from any mailer) separately.

This would support syntax like:

$someAction->getMail()->sendMail();
$this->tester->seeEmailIsSent(1, 'customer');

I'd definitely review / merge a PR implementing that. --> Feel free to leave this issue open if you're interested in implementing such a PR, otherwise, if your issue is resolved please close this issue!

VirtualRJ commented 5 years ago

Will have a look at the complexity of implementing it. For now I'll leave it open and have a look at it in the coming weeks.

SamMousa commented 2 years ago

I'm closing this due to its age. If it is still relevant in current versions feel free to create a new issue.