On 2013-07-09, at 18:48, Frank Shearar <frank.shearar@gmail.com> wrote:
On 9 July 2013 17:42, Esteban Lorenzano <estebanlm@gmail.com> wrote:
Even more important, IMHO: allowing those methods to exists is to validate an anti-pattern: if you have an specific error to throw, you should create a specific descendant of Error, not just throw Error with an explanation.
So, +A LOT to remove them.
No. Failing to specify your error messages is a colossal fail. And if you don't _test_ your error messages, how are you going to know you haven't just made your error messages utterly useless in that last refactoring?
That is a separate test for your Exception: MyExceptionTest >> testMessage | message | message := self targetClass new description. ... I don't see the point in testing the description string somewhere else...
And no, checking that your KeyNotFound exception's #key is correctly set is insufficient, because you then _presume_ that the process of reporting the exception actually bothers to record that and doesn't just say "a KeyNotFound".
I do not fully see the point of testing that a certain exception is not signaled. 1. if an exception is signaled the test will fail anyways 2. if exceptions are signaled you can easily test that using something like: self should: [ .. ] raise: (NotFound, ZeroDivide, ...) 3. If you have a test that returns different exceptions in the tests you have nondeterminism, so that basically invalidates point 2. 3. Since you have deterministic tests you know that only one kind of error is signaled hence it suffices to use the #should:raise:... methods. Hence, I really do not see the point of the complex #shouldnt:raise:whoseDescriptionIncludes: 1. #shouldnt:raise: is more less fine. Though I would expect it ignore all the other errors thrown, which it doesn't essentially limiting the use of this method. 2. #shouldnt:raise:whoseDescriptionIncludes: is too complex. 3. #shouldnt:raise:whoseDescriptionDoesNotIncludes: essentially reads like a double negation which confuses the hell out of me ----------------------------------------------------------------------------------------- Example: -------- self shouldnt: [ ... ] raise: MyException whoseDescriptionIncludes: 'PING'. What does that mean for the following blocks? 1. [ Error signal ] 2. [ Error signal: 'PING' ] 3. [ MyException signal ] 4. [ MyException signal: 'PING' ] 3 and 4 definitely fail, I assume 1 fails too, and the only one left is case 2. And the monstrous #shouldnt:raise:whoseDescriptionDoesNotInclude: will only pass in case 1? ----------------------------------------------------------------------------------------- In any case, nobody uses the complex #shouldnt:raise:whose* methods so I am removing them and simplify the implementation of TAssertable.