Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] Allow test assertions to be made about email contents - option #2 #35430

Closed
wants to merge 6 commits into from
Closed

Conversation

dusterio
Copy link

@dusterio dusterio commented Dec 1, 2020

Another implementation of a previously submitted PR: #35354

Currently, Laravel allows to make assertions about response contents, eg:

    /**
     * @test
     */
    public function homepage_welcomes_users()
    {
        $this->get('/')->assertSee('Welcome');
    }

You can make assertions about API (JSON) output as well. However, there is no easy way to test email contents as a part of integration testing - MailFake is not equipped for it. Developers have several options:

  • Extending Mailable class to "hijack" render() method
  • Extending Mailer class to extend render() or send() methods
  • Making assertions about data variables, rather than rendered output
  • Testing views - writing test units that render specific views

With this pull request, I'm proposing that MailFake class decorates Mailables to allow developers to test their outgoing emails:

    /**
     * @test
     */
    public function email_confirmation_is_correct()
    {
        Mail::fake();

        event(new TestEvent());
        config(['app.name' => 'Test App']);

        Mail::assertSent(TestMail::class, function (TestMail $mail) {
            return $mail->hasTo('test@test.com') && $mail->seeInHtml('shipped')
                && $mail->dontSeeInHtml('failed') && $mail->seeInText('Test App');
        });
    }

The job wasn't trivial - I tried to find a solution that has full backward compatibility, therefore MailFake still had to return user's own Mailable classes in closures - to account for developers who have type hints in there (just like the example above). Therefore, the only three options left were:

  • Extend Mailable class to make it suitable for testing. Not ideal since we don't want any test-related logic in framework code
  • Add Macroable trait to Mailable class so that it can be extended dynamically during testing. Mailable has a magic handler for methods starting with "with" and Macroable trait doesn't support masks/regular expressions at the moment
  • Create a proxy class that extends given Mailable class, it should forward all function calls and pass all public properties in case the user is testing these, plus add a handful of testing-related methods on top: seeInHtml(), seeInText() and so on. This is the option that I chose as the least intrusive.

How it works in short:

    // Before we return a Mailable from Mailfake as a part of a closure, we decorate that Mailable first
    protected function decorateMailable($mailable)
    {
        // We use Mockery to create a so-called proxy mock
        // This object will proxy all method calls to the original Mailable object
        $mock = m::mock($mailable);

        // It doesn't proxy properties, so we have to copy them over
        // We know that some developers make assertions about these
        foreach (get_object_vars($mailable) as $property => $value) {
            $mock->{$property} = $value;
        }

        // TestMailable is the class that implements assertions, it's designed after existing TestResponse class
        $renderer = new TestMailable($mailable);

        // Now we use Mockery again to dynamically add new methods - we copy them over from TestMailable
        // These methods will still be bound to TestMailable, even though launched from custom Mailable
        foreach (get_class_methods($renderer) as $method) {
            if (Str::startsWith($method, '__')) continue;
            $mock->shouldReceive($method)->andReturnUsing($renderer->{$method}());
        }

        return $mock;
    }

Extra remarks:

  • There is a small amount of code repetition between TestMailable and Mailer classes, namely renderView method. I couldn't find a reasonable way to share that piece of code between the two

Would love to hear more opinions, I'm sure there must be more developers out there in the wild who wanted to test their email contents at some stage!

@taylorotwell
Copy link
Member

In your example, how would the hasTo method even work? It isn't on the decorated mailable and the mailable is mocked so wouldn't mockery throw an unexpected method exception?

@dusterio
Copy link
Author

dusterio commented Dec 2, 2020

@taylorotwell hasTo is on Mailable and since this is a proxy (we passed an object to mock method when we created this test object), Mockery is going to forward all existing methods, unless they are explicitly overridden.

{
return func_get_args();
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of this "render" function?

@taylorotwell
Copy link
Member

Took a different approach to this rather than all this mocking. I've always been of opinion that testing of the content of mailables and testing of content should be separate tests.

You can now test contents of mailable straight off of a mailable in a separate test that is isolated.

$mailable = new Mailable;

$mailable->assertSeeInHtml(...).

@ahinkle ahinkle mentioned this pull request Mar 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants