[Tutor] Roulette Unit Test Questions

[Steven D'Aprano]

Ben Ganzfried wrote:
> Hey,
> 
> I'm having a lot of confusion getting the unit test working for one of my
> classes for the Roulette bot I'm working on and would greatly appreciate any
> advice or help.

[...]

> Here is my Bin class:
> 
> from Outcome import *
> class Bin:
>     def __init__(self, *outcomes):
>         self.outcomes = outcomes
>     def add(self, outcome):
>         self.outcomes += outcome
>         return self.outcomes
>     def __str__(self):
>         return (', '.join( map(str,self.outcomes)))
> 
> 
> 
> Here is the code for my BinTest class:
> 
> import Outcome
> import Bin
> 
> class BinTest:
>     def __init__(self):
>         pass

The __init__ method does nothing. That makes it pointless and a waste of 
time. Either get rid of it, or make it do something useful.


>     def create_Outcome(outcome):
>         o1 = Outcome(outcome)

Three problems with this... firstly, if this is a method, you don't seem 
to have declared a `self` parameter.

Secondly, this method creates an Outcome, stores it in a local variable, 
and then immediately throws it away when the method returns. To be 
useful, you need to either *return* the Outcome, or *save* it for later use:

         # Pick one.
         self.ol = Outcome(outcome)
         return Outcome(outcome)

But the second problem is more fundamental. This method seems pretty 
pointless to me. Why have a method that just calls a single function and 
does nothing else? You wouldn't do this, would you?

def create_int(x):
     return int(x)

value = create_int("42")

It is simpler, faster and more efficient to just call int("42") directly 
instead of adding an extra layer of indirection. Why add that extra 
layer in your class? If you have a good reason, that's fine, but what is 
that reason?


> #creates two instances of Bin
>     def create_Bin():

Misleading name. You call it "create_Bin" but it creates Bins plural.

Missing `self` parameter.

>         b1 = Bin(("Red", 5), ("Black", 17),("Red", 5))
>         b2 = Bin(("00-0-1-2-3"), ("00"))

Again, the fundamental problem here is that you create the two bins, 
store them in local variables, and then return from the method, which 
immediately deletes them.

Is this supposed to be a test, or a helper function for a test?


> #establishes that Bin can be constructed from Outcome
>     def construct_Bin_from_Outcome():
>         b3 = Bin(o2)
>         b4 = Bin(o4)
>         b5 = Bin(o1)

With the comment "establishes that..." it seems to me that this is an 
actual test. I presume that the previous methods were helpers, but they 
don't seem to be used anywhere.

Missing `self` parameter again.

Where do o2, o4 and o1 come from? What are they? How do they differ? Why 
is o3 missing? Why does o2 give you b3, but o4 gives you b4? Is there 
some pattern or meaning to the mysterious jumping numbers?

o1 -> b5
o2 -> b3
o3 -> ??
o4 -> b4
o5 doesn't seem to exist?


The point of unit tests is that *they*, not you, should test the result. 
If your unit tests are printing results, as your support code here does:

> def main():
>     bin_test1 = BinTest()
>     bin_test1.create_Outcome()
[...]
>     print("o2 is ", o2)
>     print("o3 is ", o3)
[...]

forcing *you* to read the results looking for errors, then your tests 
are too hard to use and you won't use them.


Here is how I would write the unit tests:

* Start with a naming convention for tests. A good convention is to call 
test methods "testSpam", with "Spam" replaced with some useful and 
descriptive name.

* Each test should test *one* thing.

* Although since "thing" can be arbitrarily simple, or complex, that 
gives you a lot of flexibility.

* Each test should be clear about what it is testing. Ideally, a 
stranger should be able to guess what the test does just from the name 
and at most a single short comment.

* Each test should either *pass* or *fail*. You shouldn't have to 
inspect the results to decide which it is -- the test should decide.

* Because tests are code, they can be buggy too. Keep your tests as 
simple as possible.

* Tests can also suffer *errors*, i.e. they raise an exception. That 
indicates a buggy test. Full-powered test frameworks like docttest and 
unittest have ways to keep going when a test crashes, but for this 
simple version, if a test crashes, the whole thing will stop.

With those principles in mind, here is what I have:


class BinTest:
     # Unit tests for the Bin class.
     def test_BinCreation(self):
         # Test that Bins can be created without failing. We don't
         # check that they are valid, only that they can be created.
         #
         # Test creation from multiple tuples.
         Bin(("Red", 5), ("Black", 17),("Red", 5))
         # Test creation from multiple strings.
         Bin("00-0-1-2-3", "00")
         # Test creation from an Outcome.
         Bin(Outcome("something goes here, I don't know what"))
         # If we get here, we have passed all the tests.
         return True

     def test_BinValidity(self):
         # Test that Bins aren't just created, but are correct.
         b = Bin("00")  # whatever...
         if b.something != "something":
             # Test has failed.
             return False
         if b.something_else != "something else":
             return False
         return True

    def test_BinString(self):
         # Test that Bins can be converted to strings correctly.
         b = Bin("00")  # whatever...
         s = str(b)
         return s == "Bin('00')"


# Now run the tests.
test = BinTest()
if test.test_BinCreate(): print "Bin creation passed."
else: print "Bin creation failed."
if test.test_BinValidity(): print "Created Bins are valid -- pass."
else: print "Created Bins are invalid -- fail."
if test.test_BinString(): print "String conversion -- pass."
else: print "String conversion -- fail."

You can see one disadvantage of this roll-your-own unit test suite... 
you have to remember to return True or False, otherwise the tests won't 
operate properly. This is why people have built automated test 
frameworks like unittest, which is moderately complex to learn, but 
handles 98% of the boring boilerplate for you.

An even bigger disadvantage is that *using* the tests is a PITA -- 
there's nearly as much code needed to run them as there is in the tests 
themselves. I would call this a fatal flaw for this roll-your-own unit 
test system (but for something I knocked up in ten minutes, it's not 
bad). Again, frameworks like unittest have "test discovery", that is, 
you write the tests, and the framework can automatically discover them 
and run them.



-- 
Steven