[Mailer] Add assertEmailAddressNotContains
#60740
Conversation
| @@ -1,6 +1,10 @@ | |||
| CHANGELOG | |||
| ========= | |||
|
|
|||
| 7.4 | |||
| --- | |||
| * added `assertEmailAddressNotContains()` to `MailerAssertionsTrait`. | |||
There was a problem hiding this comment.
| * added `assertEmailAddressNotContains()` to `MailerAssertionsTrait`. | |
| * Add `assertEmailAddressNotContains()` to the `MailerAssertionsTrait` |
There was a problem hiding this comment.
I agree with your suggestion, and I've made that change.
Thanks a lot for your help!
| $this->assertEmailAddressNotContains($email, 'To', 'fabien@fake.symfony.com'); | ||
| $this->assertEmailAddressNotContains($email, 'To', 'thomas@fake.symfony.com'); | ||
| $this->assertEmailAddressNotContains($email, 'Reply-To', 'me@fake.symfony.com'); |
There was a problem hiding this comment.
It seems that these assertions are redundant, given that the addresses specified have never been used before.
There was a problem hiding this comment.
I don’t think the assertions are an issue, but I’m happy to update the emails if you have any suggestions for alternatives.
Thanks again! 😄❤️
There was a problem hiding this comment.
I've updated the emails to helene@symfony.com. I believe this still tests the same behavior, but this format is more in line with the emails currently used in Symfony tests 😊
There was a problem hiding this comment.
A test that cannot fail is not useful. I think you should to add assertions that would actually fail if something breaks.
There was a problem hiding this comment.
I'd like to clarify a couple of points:
- The method is simply applying a LogicalNot to an existing Constraint that is already implemented and tested.
- If you look at several of the current assertions in the same test file, you'll notice that they cover similar cases to the ones I'm adding now. In fact, the same email is already being tested, just in a different way.
As always, if you have any suggestions, they're more than welcome! 😄
There was a problem hiding this comment.
Hi @Spomky @bkosun 👋, I also added this assertion
$this->assertEmailAddressNotContains($email, 'To', 'thomas@symfony.com');This checks that the first email does not include thomas@symfony.com in the "To" field.
Just to clarify: this address is included in the second email, but not in the first one, which is the behavior this assertion is verifying.
fd6bc22 to
7d47611
Compare
7d47611 to
b64a567
Compare
| @@ -1,6 +1,10 @@ | |||
| CHANGELOG | |||
| ========= | |||
|
|
|||
| 7.4 | |||
| --- | |||
There was a problem hiding this comment.
Thanks again! The change has been made.
Apologies for the many mistakes in the CHANGELOG.md, totally my fault 😄
b64a567 to
559d45b
Compare
|
Hi @santysisi, Indeed this could be a nice addition. |
|
Hi @Spomky 👋, thanks a lot for your comment! 😊 I see where you're coming from. The main concern with that approach is that it could lead to a large number of helper methods, like That said, if you and others feel that having separate, explicit methods is more readable or convenient, I'm happy to add them! 😄 Thanks again for your feedback ❤️ |
|
OK so let consider the suggested change from @bkosun and I'll approve this PR. |
10566de to
ce99b79
Compare
ce99b79 to
9cc6a63
Compare
This PR introduces a new assertion method
assertEmailAddressNotContains()to theMailerAssertionsTrait.The new method complements the existing
assertEmailAddressContains()by allowing developers to assert that a specific email address does not appear in a given header. This addition brings theassertEmailAddressContainsin line with other similar assertion pairs already available in the trait, such as:assertEmailTextBodyContains/assertEmailTextBodyNotContainsassertEmailHtmlBodyContains/assertEmailHtmlBodyNotContainsassertEmailSubjectContains/assertEmailSubjectNotContainsExample usage