merged branch ricardclau/master (PR #328)
Commits ------- f2cc243b Merge pull request #1 from igorw/swiftmailer-test f43dd5a5 Fix SwiftmailerServiceProviderTest (stub the spool) 1798d08f Merge branch 'master' into swiftmailer-test 5ed7e45f remove variable f1edad0c following @igorw approach a238971e Merge remote-tracking branch 'upstream/master' 34ddab52 add a test for swiftmailerServiceProvider app->finish event 5d599be3 add test for SwiftmailerServiceProvider and update composer.json require-dev section Discussion ---------- Add test for Swiftmailer service provider --------------------------------------------------------------------------- by igorw at 2012-05-25T12:39:59Z A test that verifies that Application->finish() sends mails would be nice. --------------------------------------------------------------------------- by ricardclau at 2012-05-25T13:03:39Z Ok, makes sense, will work on that and send a new pull request :) --------------------------------------------------------------------------- by GromNaN at 2012-05-25T13:05:58Z @ricardclau You can simply commit to the same branch to update the PR. --------------------------------------------------------------------------- by ricardclau at 2012-05-25T15:45:13Z What do you think about this way of testing listener? Couldn't find a better way to test it so any comments are more than welcome! --------------------------------------------------------------------------- by igorw at 2012-05-25T16:04:29Z Not a good idea, the asserts may not be executed. Also, please don't call run. Call terminate explicitly. My approach would be: * define a controller that sends a message * check the spool to make sure it's empty * call handle() * check the spool to make sure it has a message * call terminate * check the spool to make sure the message is flushed --------------------------------------------------------------------------- by ricardclau at 2012-05-25T16:10:27Z Sounds much better, I'll try my best as you suggest. Although your method seems much better than mine, I'm curious... in which cases asserts would not be executed? --------------------------------------------------------------------------- by ricardclau at 2012-05-25T18:11:33Z Seems I screwed up a little bit. Once all is ok, I can provide a clean pull request if you prefer so. Sorry and waiting for your comments --------------------------------------------------------------------------- by stof at 2012-05-25T22:07:17Z @ricardclau no need to create a new PR. Once the review is done, you can simply squash your commits and force the push of the branch, which will update the PR. This is described in [the Symfony doc](http://symfony.com/doc/current/contributing/code/patches.html#rework-your-patch) --------------------------------------------------------------------------- by ricardclau at 2012-05-26T02:49:31Z @stof I didn't know that! Amazing! The best thing of trying to contribute in Symfony community is always the amount of things you can learn from all you guys! Thanks! --------------------------------------------------------------------------- by ricardclau at 2012-05-30T09:19:52Z @igorw Ping? Is it now ok? Anything that should be changed? Waiting for your comments Regards! --------------------------------------------------------------------------- by igorw at 2012-05-30T15:17:02Z @ricardclau I've submitted a PR to your repo, please merge. --------------------------------------------------------------------------- by ricardclau at 2012-05-30T15:33:50Z Merged --------------------------------------------------------------------------- by igorw at 2012-05-30T15:45:16Z Ok, this looks good to me.
Showing
... | ... | @@ -35,7 +35,8 @@ |
"symfony/monolog-bridge": "2.1.*", | ||
"symfony/validator": "2.1.*", | ||
"twig/twig": ">=1.2.0", | ||
"doctrine/dbal": "2.1.*" | ||
"doctrine/dbal": "2.1.*", | ||
"swiftmailer/swiftmailer": "4.1.*" | ||
}, | ||
"autoload": { | ||
"psr-0": { "Silex": "src/" } | ||
... | ... |
tests/bootstrap.php
0 → 100644
Please register or sign in to comment