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

[11.x] Update tests to ensure mail Message implements the fluent interface pattern #51969

Merged
merged 1 commit into from
Jul 1, 2024

Conversation

seriquynh
Copy link
Contributor

Because Illuminate\Mail\Message implements the fluent interface pattern. Each of these methods: from, sender, returnPath, to, cc, bcc, replyTo, subject and priority must change and return the instance itself. But current tests can be passed if a method return a clone/new static Message instance.

Let's take a look at from method.

<?php

public function from($address, $name = null)
{
    $msg = clone $this; // Clone the called object.

    is_array($address)
        ? $msg->message->from(...$address)
        : $msg->message->from(new Address($address, (string) $name));

    return $msg; // Return the cloned one.
}

public function testFromMethod()
{
    # These assertions are still passed.
    $this->assertInstanceOf(Message::class, $message = $this->message->from('[email protected]', 'Foo'));
    $this->assertEquals(new Address('[email protected]', 'Foo'), $message->getSymfonyMessage()->getFrom()[0]);

    # But these won't because we check the message instance itself.
    $this->assertSame($this->message, $this->message->from('[email protected]', 'Foo'));
        $this->assertEquals(new Address('[email protected]', 'Foo'), $this->message->getSymfonyMessage()->getFrom()[0]);
}

I think if a class implements the fluent interface pattern, assertSame is the required assertion when writing tests.

@taylorotwell taylorotwell merged commit f53a77a into laravel:11.x Jul 1, 2024
30 checks passed
@seriquynh seriquynh deleted the update-tests branch July 2, 2024 02:26
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

2 participants