-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Notifier] Introduce PHPUnit constraints and assertions for the Notifier #46895
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
Conversation
0b2c788
to
3864f8d
Compare
src/Symfony/Bundle/FrameworkBundle/Test/NotificationAssertionsTrait.php
Outdated
Show resolved
Hide resolved
.../Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Controller/NotifierController.php
Outdated
Show resolved
Hide resolved
3864f8d
to
e3a1aa0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @ismail1432!
This makes sense to me and looks to match the email assertions. I'm not that familiar with notifier but should there be assertions on:
- Who the notification was sent to?
- What was in the body?
@OskarStark, you've been wanting this type of feature for some time and use notifier in your app(s). Do these assertions cover your needs?
src/Symfony/Bundle/FrameworkBundle/Test/NotificationAssertionsTrait.php
Outdated
Show resolved
Hide resolved
047a607
to
356e312
Compare
I add more assertion, my point here is to introduce basic assertions and add more later by me or community 👍 |
Sure, that's fair! |
src/Symfony/Component/Notifier/Test/Constraint/NotificationTransportIsEqual.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Notifier/Test/Constraint/NotificationImportanceIsEqual.php
Outdated
Show resolved
Hide resolved
766d1ca
to
b6b6383
Compare
b6b6383
to
747e9cb
Compare
Thank you @ismail1432. |
…(javiereguiluz) This PR was merged into the 6.2 branch. Discussion ---------- [Notifier] Rename some arguments in notifier assertions | Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | if this is merged, I'll update symfony/symfony-docs#17116 While documenting #46895, I thought about renaming some arguments. So here's a PR with that proposal for your consideration. Thanks. Commits ------- b5178dd [Notifier] Rename some arguments in notifier assertions
Since 4.4 we have assertions for email , the goal of this PR is to introduce the same kind of assertions for the Notifier component.