Allow multiple exceptions to be thrown in one call with andThrowExceptions (array $exceptions) #312
Conversation
| throw new Exception('You must pass an array of exception objects to andThrowExceptions'); | ||
| } | ||
| } | ||
| $this->andReturnValues($exceptions); |
There was a problem hiding this comment.
Just add return here, since andReturnValues also returns current expectation object.
|
@aik099 Changes made, new commit pushed. |
|
Can you please squash commits to make history cleaner? |
* Function andThrowExceptions(array $exceptions) added to Expectation.php. * Tests that exceptions are thrown in correct order, that the correct number of exceptions are thrown, and that an exception is thrown if a non-object is passed as an array element to the method.
|
@aik099 Commits squashed |
|
Hi @aik099 , Just wanted to make sure that this PR is good to go. If you'd like me to fix anything else up please let me know. |
| @@ -183,6 +183,53 @@ public function testThrowsExceptionSequentially() | |||
| $this->mock->foo(); | |||
| } | |||
|
|
|||
| public function testAndThrowExceptions() | |||
There was a problem hiding this comment.
Can this test be refactored to contain less duplication code?
There was a problem hiding this comment.
Possibly, but it's not immediately apparent to me how that could be done. It seems important to enclose both $this->mock->foo(); statements in full, separate try / catch blocks to ensure that the exceptions are called in the correct order and that the correct number of exceptions are thrown. Separating them into separate try / catch blocks (with catch blocks for both types of exceptions in both try / catch blocks) achieves both of those goals.
Furthermore, if we were to put the try / catch blocks into a separate helper function, wouldn't we have a problem with scope for both thexxxExceptionThrown booleans, as well as the count on the number of exceptions thrown? I suppose we could have the helper method return an array of which exceptions were caught, and the number of exceptions that were thrown in each block, but that doesn't seem like a clean approach.
Not saying I'm correct, but that is my first impression after looking over the code again. Any suggestions are welcome.
There was a problem hiding this comment.
I see. Then maybe adding some blank lines to separate logically independent code pieces would increase readability of the method.
It's @davedevelopment who can do the merging. I'm just helping out. Thanks for your contribution. |
| new OutOfBoundsException, | ||
| new InvalidArgumentException, | ||
| ) | ||
| ); |
There was a problem hiding this comment.
Add ) before this line and remove previous line to create properly formatted multi-line function call.
|
See #326 |
Tests for function have been written in ExpectationTest.php. Let me know if you'd like me to extend the tests before merging.