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

[10.x] Call renderForAssertions in all Mailable assertions #48254

Merged
merged 3 commits into from
Aug 31, 2023

Conversation

jamsch
Copy link
Contributor

@jamsch jamsch commented Aug 30, 2023

In the Laravel docs for Mail -- https://laravel.com/docs/10.x/mail the following code snippet is shown:

use App\Mail\InvoicePaid;
use App\Models\User;
 
public function test_mailable_content(): void
{
    $user = User::factory()->create();
 
    $mailable = new InvoicePaid($user);
 
    $mailable->assertFrom('jeffrey@example.com');

The $mailable->assertFrom('jeffrey@example.com'); example here will fail the test because the mailable's render() hasn't been called yet. I noticed in the PR #47728 - the renderForAssertions function was called for assertHasSubject but I could not see why this isn't being called for the other Mailable assertion functions, such as assertFrom taken from the docs.

So, in summary I've added the renderForAssertions to the following functions:

$mailable->assertFrom("");
$mailable->assertTo("");
$mailable->assertHasCc("");
$mailable->assertHasBcc("");
$mailable->assertHasReplyTo("");
$mailable->assertHasTag("");
$mailable->assertHasMetadata("");

Which will no longer mean that the user needs to call a specific $mailable->assertHasSubject() or $mailable->assertSeeInHtml() before being able to call the above methods.

@jamsch jamsch marked this pull request as draft August 30, 2023 23:59
@jamsch jamsch marked this pull request as ready for review August 31, 2023 01:33
@jamsch
Copy link
Contributor Author

jamsch commented Aug 31, 2023

PR should be ready. Just a note though: many of the tests have the same mailer container class stub which may warrant some DRY refactoring.

@taylorotwell taylorotwell merged commit 6595252 into laravel:10.x Aug 31, 2023
19 checks passed
@tmont
Copy link

tmont commented Sep 12, 2023

FYI this is a bit of a (minor) breaking change, as it kind of drastically changes behavior of merely asserting that a Mailable has a from address or something. Now it forces a render of the entire Mailable for every assertion which may be unwanted during a testing environment (for performance reasons, for example).

Part of my email rendering process involves inlining CSS which is not something I care about nor want during a unit test, and this change forces that to happen at times when it's not needed.

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.

None yet

4 participants