[Update]Be sure to read the comments if you’re interested in the topic. There’s a lot of interesting info to find there[/Update]
Last month I’ve given a fast-start Smalltalk training session to a new team member at a Customer’s project and since that person was an experienced developer we not only covered Smalltalk basics but also some special areas like sUnit. As I’ve mentioned before, I found a few errors in my material and I am currently working on corrections and extensions of the slides. So now I am taking a fresh look at sUnit – especially the variants of #should: – and found a few interesting things that I’d like to share with whoever is interested.
Let’s kill should:!
Let me first explain what should: is good for. In essence, it is the same as assert:, but it accepts a zero-argument Block. So the following two assertion statements are completely equivalent:
self assert: 1 < 3. self should: [1 < 3].The only difference between the two is that assert: expects a Boolean parameter and should: expects a Block which it will evaluate before delegating to assert:. The same is true for deny: and shouldnt:.
[Update] To be a bit more precise: assert: means “I expect this expression to be true” while should: means “I expect this Block to evaluate to true”. In effect, both means the same thing: the result of the parameter is expected to be true.
[/Update]
So my first thought here is: why not eliminate should: completely and implement assert: in a way that it can work with both Booleans and Blocks. While this sounds complicated, it really isn’t: since in many Smalltalk dialects there is an implementation of value in Object (so that Objects return themselves and Blocks the result of their evaluation*), the only thing to do is to send value to the parameter of assert:.
[Update] The Implementation of TestAsserter>>should: is in fact nothing more than
should: aBlock self assert: aBlock valueWhile assert: looks like this:
assert: aBoolean aBoolean ifFalse: [self logFailure: 'Assertion failed'. TestResult failure sunitSignalWith: 'Assertion failed'].In the end, the implementation of our new assert: is easy:
assert: aBooleanOrBlock aBooleanOrBlock value ifFalse: [self logFailure: 'Assertion failed'. TestResult failure sunitSignalWith: 'Assertion failed'].[/Update]
This would mean there’s no need to memorize a different method for making assertions about Blocks. We could simply write
self assert: [a < b].
So let’s move on to should:raise: and kill it also
The idea of should:raise: is to formulate an assertion about whether a piece of code will raise a certain exception. Like this:
self should: [5/0] raise: ZeroDivide
description: 'Division by zero was not signalled'.
This will work like a Clockwork: The exception is raised, so the test case will pass, because we expected it to.
My first critique here is again the naming: why not call it assert:raises: instead of should:raise:? I find should:raise: is ambiguous. Does it mean the Block should run and not raise an exception (because #should: means the Block is expected to evaluate to true, and therefor run correctly), or does it mean evaluating the Block should raise the exception? In fact the latter is the case, but I find it hard to memorise.
A second argument for assert:raises: would be that it is in line with method names like assert:equals: as they were backported from jUnit.
Together with the above-mentioned value trick, this would mean that you can even test every piece of code (not only Blocks) whether it raises an exception or not. The following two assertions would be equivalent, while there is currently no way of achieving the second one:
self assert: [5/0] raises: ZeroDivide description: 'Division by zero was not signalled'.self assert: (5/0) raises: ZeroDivide description: 'Division by zero was not signalled'.
[Update]Giving this an even deeper look, there still is somethng wrong with all this should: stuff. While should: means “I expect this Block to evaluate to true”, should:raise: means “This Block should raise the following exception, no matter what it evaluates to if it doesn’t”. So the meaning of should: and should:raise: is far more different than their names suggests. Not sure what exactly that means for my naming idea yet, but I’m still meditating on it ;-).
Also, I am not yet decided if I like assert:doesntRaise: more than deny:raises: because the latter one suggests I want the parameter (namely Block or expression) to be false. Well, I guess I like the *doesntRaise: variant more…[/Update]
So, back to shouldnt:raise:: I found a bug!
Last, I found out that while should:raise: can test whether an expected exception gets thrown, the opposite case is only partially testable:
self shouldnt: [5/1] raise: ExError description: 'An Error was signalled unexpectedly'.Can test whether an exception is not thrown. Unfortunately, this only works if no exception is thrown at all in the Block. The case of an explicit exclusion of an exception that should not be thrown by a Block is not covered by sUnit. What I mean is that I want to formulate an assertion that means something like
No matter what kind of exception this piece of code may throw, I want to be sure it is not an exception of Kind X.
"This doesn't work!!! - results in an error instead of passing"
self shouldnt: [5/0] raise: ExHalt description: 'A Halt was signalled unexpectedly'.I’d be interested in hearing what other sUnit users think about my suggestion of replacing should: with assert: and whether there are any reasons for keeping them that I haven’t thought of. Also I’d like to learn what others think about the “Exception Exclusion bug”.
*) The implementation of value in Object is a brilliant example of how useful polymorphism can be. I like the value trick very much and find it extremely helpful in making object protocols lightweight and elegant (just like in this example of sUnit). Being able to pass either an object or a Block as a parameter into a message without a performance penalty or code overhead other than sending value to a method argument is extremely powerful. The whole topic is probably worth a blog post on itself…
Please note: the code examples were tried on VA Smalltalk 8.0.
Another note: I’ve been wanting to post these ideas to some forum that is read by Smalltalkers of all flavors, but since I think the relevance of comp.lang.smalltalk is somewhat extinct (and my carrier even won’t let me use Usenet any more) and I know of no suitable mailing list for the topic, It’s only here…
Joachims Small World 
12 responses to “sUnit and should:/shouldnt: vs. assert:/deny:”
Were I to keep one, it would be should:, and toss assert:. Why? The “assert” speaks the language of test, while “should” speaks the languages of executable specifications.
We are specifying what the thing (which does not even exist yet, in tdd) should do, not asserting that it does something. So a bdd-style test method might be named “shouldBlinkLightsWhenStarting” not “assertBlinkLightsWhenStarting”, and consistent with that we use in the method body “self should:’ not a “self assert”.
So I prefer “should” to “assert” because it’s closer to the language of behavior-driven-development, and that helps bridge communication gaps with people outside the technical team. See rspec or cucumber in Ruby; hamcrest matchers in Java. Didn’t Dave Astels build a Smalltalk bdd-test framework?
LikeLike
Alan,
I must admit I haven’t given the name assert any deeper thought than “it’s the standard message in sUnit”. While your thoughts on this name are reasonable, I think it would be too much of a change to sUnit and break lots of existing code. (well, obsoleting should: would of course also do that…). But the first thing you learn about sUnit is “self assert: 1<2", and I guess a lot of people never used should: anyways, since you can always use "self assert: myBlock value". Maybe most of the times people do compare the result of a Block evaluation to an expected result in a normal assertion anyways, so should/shouldnt are probably used very rarely and we are discussing a more theoretical edge case.
But even if that's true: should/shouldnt seem to me as if they were fat than can be trimmed off easily, if only to make sUnit slimmer and friendlier. To me it's very artificial to differentiate between a Block and a normal expression, because – ignoring the implementation details – there is not much difference between (a<b) and [a<b]. I assert it will be true in the end, no matter how many hoops the system has to jump to find out.
But coming back to your argument: should sounds much more like "If all is the way I think or expect it to be, then this code returns true".
When we're talking about how we'd love sUnit to be, I must say I'd rather build objects and use an API like the Canvas API in Seaside. Thus I could drop many variants of assertion methods by simply configuring an assertion object. I mean something like
self addAssertion running: [myTestCustomer isNoOlderThan: 24]; raises: OutOfRangeExcpetion; logFailure: 'The Customer is older than 24!'; logError: 'Age check signalled an error!'.LikeLike
VisualWorks *did* have Object>>value, in the way back machine. I want to say in either VW 1.0 or 2.0. But then it was taken back out. For reasons along the lines that Reinout describes. It basically just makes things messy. And value just has too much loaded on it already. There’s no reason to not use something like #evaluate in VisualWorks which is clearer. Though I question whether ValueModel>>evaluate should be implemented the way it is.
I think one should obsolete the should: patterns, because minimal frameworks are easier to deal with. Unfortunately, the trend with SUnit has been anything but in the past. I wrote an SUnit fork once upon a time to deal with that. It’s nice to see others wanting to trim the fat off.
LikeLike
Travis,
on a completely different track: why should evaluate be any better? I am not asking an object to evaluate, I am asking it for its value. I don’t care if it needs to make a handstand and meditate a bit to find its value or if it evaluates itself 😉 But as you write: There’s no reason to not use something like #evaluate, because you can always see it the other way round: I want the object to evaluate itself, and if it’s not the block, the job of evaluating is not too hard.
The problem with forking off sUnit is that it makes life harder for people. They need to decide whether to use sUnit or sUnitToo(ls) (which I guess you’re talking about), and they need to memorize the differences between the two. It’s hard enough to convince a team to use unit testing at all, why make it more complicated. The chance for a fork to be existent or used on only one dialect is high. Making sUnit slicker and more effective would surely be better in my opinion.
I’m glad to hear people tend to agree on retiring should: . Maybe we should really do it…
LikeLike
The dsitinction between assert: and should: is very hard to explain to my students. So I would appreciate the killing of should:
Johannes
LikeLike
Johannes,
I also began to stutter in my above-mentioned training when it became obvious that my slide contained an error. It had the very same explanation for should:raise: and shouldnt:raise:, just in different words, and I couldn’t really explain the real difference between the two.
Does should:raise: mean it should raise or it should or otherwise raise or what?
But as I already wrote: what is the best name for an assertion that an exception is not raised? I tend to like the assert:doesntRaise: idea best.
But is there any use for a deny:raise: or deny:doesntRaise: ??? I guess not.
Maybe I just opened a cask that’s way too philosophical for my simple mind…
LikeLike
“*) The implementation of value in Object is a brilliant example of how useful polymorphism can be. I like the value trick very much and find it extremely helpful in making object protocols lightweight and elegant (just like in this example of sUnit). Being able to pass either an object or a Block as a parameter into a message without a performance penalty or code overhead other than sending value to a method argument is extremely powerful.”
Where I work we have forbidden this extension.
-It breaks base VisualWorks (exception handling if I’m not mistaken).
-it breaks unsuspecting libraries (in the pasr loading this method for Squeak compatibility would break the VW encryption libraries)
-It papers over bugs when designing code where keeping upped and downed objects apart is important (because they are *not* polymorphic, even though one tries to pretend they are).
-It doesn’t have universal behavior so you get alternatives like #evaluate, #_value etc.
Just like the map is not the territory, the block is not the value – except in microcosms (like SUnit).
Object>>value has cost us *much* more than it has delivered, hence it is verboten (until we have usable selector namespaces).
LikeLike
Reinout,
so the problems you describe sound quite VW specific (nevertheless making my suggestion a bit more complicated to implement but still helpful) and is only related to how I suggested to implement assert:. Which means you’d have to check the parameter for whether it is a Block and only then evaluate it. Not as clean code as I suggested but still doable and still helpful with respect to a better and easier API of sUnit (IMO).
VAST has just added Object>>value in a very recent release, I guess for Squeak/Pharo compatibility, so it shouldn’t hurt there. In fact we use #value a lot in our widget framework on top of Seaside.
I must admit I didn’t understand your argument: “-It doesn’t have universal behavior so you get alternatives like #evaluate, #_value etc.”…
LikeLike
“nevertheless making my suggestion a bit more complicated to implement but still helpful”
–not at all, if you name it #asBooleanForSUnit you should have no problems.
Even better: if you implement it on Boolean and Block only you have a more robust system than if you implement it on Object. (this happens to also make your system more self-documenting).
My gripe is with the fact that so many people decide what #value should mean (universal behavior), and so many people assuming (without thinking it seems) that #value should be implemented on Object, while its absence delivers more value to the developer (fail fast when code confuses maps with territories).
One example re no universal behavior: at some point Pollock introduced #evaluate. It seemed to do the same as #value but not quite always…
LikeLike
okay, you’re right. For sUnit purposes, a new method like #sunitValue (I must admit I don’t like your *asBoolean* suggestion because it doesn’t convert a Block into a Boolean, it evaluates it, and it should be implemented in Object rather than Boolean) implemented on Block and Object would probably be much better.
So it’s not necessarily a hard change for the assert:* methods.
LikeLike
“it evaluates it”
–no it doesn’t: only blocks do, Booleans Don’t.
So #xValue as a selector name is wrong if you read it as ‘evaluate’.
On top of that I am of the skool that says methods should name what they do – not how they do it. Steven Kelly and clan express this by using the phrases ‘domain vocabulary’ vs ‘solution vocabulary’. (More derogatory: you are talking to the machine, not to the next programmer who reads your code).
“and it should be implemented in Object rather than Boolean”
Of course not 🙂
In the context you describe it provides a service to SUnit and there it has three cases to consider:
1) Blocks should evaluate themselves,
2) Booleans should return themselves,
3) other types should signal a broken test.
Methinks a DNU exception nicely implements the semiotics expected from 3), moreover having only implementations on Boolean and Block clearly documents your test framework, having implementations on Block an Object on the other hand only obfuscates your intent.
Hope I’m not too abrasive…
LikeLike
Reinout,
Your comments about whether or not to implement the evaluation message on Object make sense. I tend to avoid getting errors in Tests because for me that means something is broken, not that my assertions aren’t correctly formulated. But that is probably a wrong usage token. If my assertion is written in a way that it doesn’t work in sUnit, this is an error (my fault) and not a failing test. So I’ve learnt something from your comment and tend to agree on this one. an Object shouldn’t have an #sunitValue. Still, the thought “well, an object’s value is the object, what’s the problem?” is reluctant to leave my brain 😉
I don’t read #sunitValue as an action, it’s the request to an object to return it’s value. If the object is an object, its value is itself. If it’s a Block, its value is the result of its evaluation. I don’t see that as a problem. And since #value is implemented on Object in VAST and other Smalltalks anyways, why bother 😉
LikeLike