[Seaside-dev] self shouldnt: [ whatever ] raise: Error.

[Sven Van Caekenberghe]

On 20 Oct 2013, at 15:23, Jan van de Sandt <jvdsandt at gmail.com> wrote:

> Hi,
> 
> Probably related to https://pharo.fogbugz.com/f/cases/11876

COPIED:

As pointed out on several occasions, writing test expression such as self shouldnt: [ ... ] raise: Error are counter productive. Testing for a specific error is sort of ok (still of mostly of no use).

=========================================================================================

I accidentally run the dev version of moose on a 2.0 to check if the VMs work properly
and I ran into a bunch of failing tests.

https://ci.inria.fr/pharo/view/3.0-VM/job/PharoVM-AcceptanceTest/27/SLAVE=pharo-linux.ci.inria.fr,TEST_IMAGE=vm-test.zip/testReport/

First of all, my problem, since I wanted to test the #stable release anyway.
In any case I had a look at the bugs since I was interested to see if it was 
in any way VM-related:

https://ci.inria.fr/pharo/view/3.0-VM/job/PharoVM-AcceptanceTest/27/SLAVE=pharo-linux.ci.inria.fr,TEST_IMAGE=vm-test.zip/testReport/Moose.Tests.Finder/MooseFinderForSimpleClassesTest/testMenuEntriesForClasses/

Ok, 0 information :(, why? Because of #shouldnt:raise:.
On a linguistic basis the #shouldnt:raise: message makes sense, sort of the counter 
part of #should:raise:... BUT how does it handle errors exactly?

Example 1:
----------
1:

    self shouldnt: [ 1 doeSomething ] raise: MessageNotUnderstood.
ok, seems fine, the test should fail, most probably with an assertion failure, 
and this is what it does indeed.

Example 2:
----------
1:

    self shouldnt: [ 1/0 ] raise: MessageNotUnderstood.
will that now fail or not? Because it doesn't raise the MessageNotUnderstood, 
so the logic conclusion is that it doesn't fail. Well, you're wrong, it WILL FAIL,
with a ZeroDivide, which is the major reason #shouldnt:raise: is not helpful.

That is utterly confusing and rather useless. Because you can just write
1:
2:
3:

    
    
1 doeSomething.

    
1/0.
- In case 1 you get an MessageNotUnderstood instead of an AssertionFailure => test fails in both cases
- In the second case you get the same message, so nothing to discuss about.

The interesting part is, that case 1 almost never happens like I've shown it. 
Most of the time it looks more like this (including the failing moose tests):
1:

    self shouldnt: [ 1 doeSomething ] raise: Error.
    
This is in my eye useless, even worse, it is obstructing, since you basically 
mute an otherwise useful error. In the original example, I was left with no 
stack trace nor a decent reason why the test would fail. Whereas with a bit of
stack trace I would get reasonable context to start real debugging efforts.


Conclusion:
-----------
a) do not use #shouldnt:raise:
b) still don't use it
c) if you want to use #shouldnt:raise: for dubious reasons, use very specific errors you test for

> Cheers,
> Jan
> 
> 
> On Sun, Oct 20, 2013 at 3:17 PM, Philippe Marschall <philippe.marschall at gmail.com> wrote:
> Hi
> 
> Somebody decided that the following
> 
> self shouldnt: [ whatever ] raise: Error.
> self shouldnt: [ whatever ] raise: Exception.
> 
> Should _always_ signal an Error in Pharo 3. Any ideas?
> 
> Cheers
> Philippe
> _______________________________________________
> seaside-dev mailing list
> seaside-dev at lists.squeakfoundation.org
> http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev
> 
> _______________________________________________
> seaside-dev mailing list
> seaside-dev at lists.squeakfoundation.org
> http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev