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 assertHasSubject #47728

Merged
merged 1 commit into from Jul 12, 2023

Conversation

ttrig
Copy link
Contributor

@ttrig ttrig commented Jul 12, 2023

Description

When testing the "subject" of a mailable with assertHasSubject that has multiple locale and has not been "prepared" returns the default/wrong subject.

Steps To Reproduce

class ExampleMail extends Mailable
{
    public function envelope(): Envelope
    {
        return new Envelope(subject: __('mail.example.subject'));
    }
}
// won't work
(new ExampleMail())
    ->locale('sv')
    ->assertHasSubject('swedish subject');

// will work
(new ExampleMail())
    ->locale('sv')
    ->assertSeeInHtml('...') // calls `renderForAssertions`
    ->assertHasSubject('swedish subject');

@taylorotwell taylorotwell merged commit 669a3a6 into laravel:10.x Jul 12, 2023
17 checks passed
@GrahamCampbell GrahamCampbell changed the title Call renderForAssertions in assertHasSubject [10.x] Call renderForAssertions in assertHasSubject Jul 12, 2023
@ttrig ttrig deleted the patch-1 branch July 13, 2023 08:17
@mabdullahsari
Copy link
Contributor

This change broke some of the tests in our test suite. Heads up to anyone who may encounter the same problem.

@driesvints
Copy link
Member

@mabdullahsari what broke? Can you please provide some code?

@mabdullahsari
Copy link
Contributor

@mabdullahsari what broke? Can you please provide some code?

You could argue it's "our fault" as some of our mail tests relied on the fact that the mail body was not rendered if you only asserted the header, which allowed us to skip some preparatory work in tests that only assert the header instead of the entire mail itself. So this change caused some of our user-land code to start breaking even though technically they're still valid tests with the previous behavior.

I don't think you have to revert anything per se, just wanted to drop my 0.02 if someone else also saw breakages due to this change.

Basically, this used to work:

#[Test]
public function envelope(): void
{
    $user = UserFactory::new()->make();

    $mail = new VerifyNewEmailMail($user);

    $mail->assertHasSubject('Confirm new email address');
    $mail->assertTo($user->pendingEmail->email);
}

Now, we have to do this (Mail body renders a temporarily signed URL which requires a token on the pendingEmail relation, and since it was not prepared for this test it caused URL generation exceptions):

#[Test]
public function envelope(): void
{
    $user = UserFactory::new()->make();
    $user->pendingEmail->setAttribute('token', 'abc123');

    $mail = new VerifyNewEmailMail($user);

    $mail->assertHasSubject('Confirm new email address');
    $mail->assertTo($user->pendingEmail->email);
}

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