-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Add email assertions trait #9322
Conversation
protected $_email; | ||
|
||
/** | ||
* @param array|string|null $content |
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.
Missing parameter comment
|
||
/** | ||
* @param array|string|null $content The email's content to send. | ||
*/ |
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.
Missing @return tag in function comment
* @param string $email Sender's email address. | ||
* @param string $name Sender's name. | ||
* @param string $message The failure message to define. | ||
*/ |
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.
Missing @return tag in function comment
ffa9c5f
to
f813a52
Compare
$expected = [$email => $name]; | ||
$result = $this->email()->to(); | ||
$this->assertSame($expected, $result, $message); | ||
} |
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.
Should there also be assertions for cc, bcc and attachments?
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.
and also maybe a assertEmailToContains()
to look for an email address in a list
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.
Totally, was just a PoC to start with.
I think this is a neat idea. I like the trait approach more than another base class that requires inheritance. |
} | ||
|
||
/** | ||
* @param string $expected Expected attachment. |
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.
Doc comment for parameter $expected does not match actual variable name $filename
I have added all suggestions and written some functional tests which would serve as sample. Was not sure how to best unit test it without it just being mocks. Pointers welcomed. |
Functional tests are more valuable than a pile of mocks would be. |
Current coverage is 95.12% (diff: 73.07%)@@ master #9322 diff @@
==========================================
Files 410 411 +1
Lines 28055 29098 +1043
Methods 3362 3677 +315
Messages 0 0
Branches 0 0
==========================================
+ Hits 26653 27680 +1027
- Misses 1402 1418 +16
Partials 0 0
|
|
||
/** | ||
* @param string $filename Expected attachment's filename. | ||
* @param array $file Expected attachment's file info. |
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.
|null are missing in a few places.
You can use my sniffer to auto fix this by the way :)
@jadb Just curious. None of your test methods have a short description in the doc block. |
@ravage84 no. Just opted not to waste time before I make sure all is done and no methods needs changed or otherwise. But feel free to add descriptions and push to this PR if you want to help :) |
* | ||
* @copyright Copyright (c) Cake Software Foundation, Inc. (http://cakefoundation.org) | ||
* @link http://cakephp.org CakePHP(tm) Project | ||
* @since 3.3.1 |
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.
3.3.3 :)
This will allow assertions in tests, like:
I haven't written tests yet but will do once everyone thinks the API is good enough.