On 2013-07-09, at 18:48, Frank Shearar <[email protected]> wrote:
> On 9 July 2013 17:42, Esteban Lorenzano <[email protected]> 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.


Reply via email to