Yesterday, Tozier and I had many difficulties with our Button’s touch logic. This didn’t surprise me, as I’ve worked ties Codea touch() before, and it was tricky then, too. In our end-of-session retrospective, though, we realized that we were working without tests, and we had found ourselves debugging, and we know what that means: we need some tests for this logic.

I mentioned early on that I don’t know how to test a game where you can see that the ship is either flying or it isn’t, and that I’d stay alert for signs that testing is needed. We have those signs, big enough to be seen from space[war]. So today, we’ll figure out how to test that logic. I think it’ll prove to be interesting.

It will also be profitable, I believe. We still aren’t even sure whether we’re going to trigger messages out of our Buttons, or whether the ship will inquire of the controls when it’s interested, so there are surely changes coming. Where there are changes, there are mistakes. Where there are mistakes, tests keep those errors from turning into bugs. Let’s see what happens.

Here’s the relevant code from Button:

function Button:touched(touch) 
    self.capturedID = self:getCapture(touch)
    self.pressed = self:getPressed(touch)
end

function Button:getCapture(touch) 
    if touch.state == BEGAN and self:insideCircle(touch.x, touch.y) and not self.capturedID then
        return touch.id
    elseif touch.state == ENDED and self.capturedID == touch.id then
        return nil
    else
        return self.capturedID
    end
end

function Button:getPressed(touch)
    if not self.capturedID then
        return false
    else 
        return self:insideCircle(touch.x,touch.y) and self.capturedID == touch.id
    end
end

Conveniently, the Programming By Intention approach gave us functions that return the Button’s captured Touch ID, or nil, and true or false depending on whether the button considers itself pressed. Those ought to be “easy” to test. And y’know what? We’re going to use a simple test fake or “mock object” as well. This should be fun …

Fascinating. Tozier wanted to see the assertTrue fail … Our tests look like this:

function assertTrue( boolean, message )
    if not boolean then 
        print(message)
    else 
        print("ok")
    end
end

function assertFalse(boolean, message)
    return assertTrue( not boolean, message)
end

function testButton()
    assertTrue(false, "this is really ok")
    local button = Button(nil, 100, 100, "nothing")
    local touch = { id=1, state=BEGAN, x=100, y=100 }
    assertTrue(button:insideCircle(touch.x, touch.y), "should be inside")
    touch.x = 201
    assertFalse(button:insideCircle(touch.x, touch.y), "should be outside")
end

In testing insideCircle, we discover by inspection that our button circle has diameter 100, but it’s called radius. This is something to do with ellipseMode, if we recall.

function testButton()
    assertTrue(false, "this is really ok")
    local button = Button(nil, 100, 100, "nothing")
    local touch = { id=1, state=BEGAN, x=100, y=100 }
    assertTrue(button:insideCircle(touch.x, touch.y), "should be inside")
    touch.x = 151
    assertFalse(button:insideCircle(touch.x, touch.y), "should be outside")
end

The 151 test fails: it thinks the point inside but we thought it should be outside, because we were thinking radius but the circle is drawn with diameter. We’re going to change the radius to 50, and the mode to draw in radius, so the picture stays the same. Then the test should run, we believe. We change those two things and yes the tests all run and the picture looks as intended.

Wow. We’re finding lots of surprises with these tests already. I didn’t even want to test insideCircle() but my pair made me do it. Wow.

Our test tells us that we should be passing the touch to insideCircle() and since our tests are green we’ll do that change.

function testButton()
    assertTrue(false, "this is really ok")
    local button = Button(nil, 100, 100, "nothing")
    local touch = { id=1, state=BEGAN, x=100, y=100 }
    assertTrue(button:insideCircle(touch), "should be inside")
    touch.x = 151
    assertFalse(button:insideCircle(touch), "should be outside")
end

And in Button,

function Button:insideCircle(touch)
    return self.pos:dist(vec2(touch.x, touch.y)) <= self.radius
end

We changed the calls as well but will spare you that. Tests still green, button still looks the right size and when we touch it it changes color as intended. So far so good. Now can we work on the logic?

function testButton()
    assertTrue(false, "this is really ok")
    local button = Button(nil, 100, 100, "nothing")
    local touch = { id=1, state=ENDED, x=100, y=100 }
    assertTrue(button:insideCircle(touch), "should be inside")
    touch.x = 151
    assertFalse(button:insideCircle(touch), "should be outside")
    touch.x = 100 -- touch is inside
    assertEqual(button.capturedID, nil, "captured id should be nil")
    assertEqual(button:getCapture(touch), nil, "should still be nil")
    touch.state = BEGAN
    assertEqual(button:getCapture(touch), 1, "should be id 1")
end

Here are our tests. They are all green. But they are not easy to read. We need to refactor them. I see two things that need doing. First, we should make some touches, with sensible names, and not change them, just use them. Second, we should consider moving to multiple test functions called by testButton(). After some rearranging, we decide not to pull the tests out into separate test functions yet.

function testButton()
    local button = Button(nil, 100, 100, "nothing")
    local insideEndTouch   = { id=1, state=ENDED, x=100, y=100 }
    local outsideEndTouch  = { id=2, state=ENDED, x=151, y=100 }
    local insideBeganTouch = { id=3, state=BEGAN, x=100, y=100 }
    
    assertTrue(button:insideCircle(insideEndTouch), "should be inside")
    
    assertFalse(button:insideCircle(outsideEndTouch), "should be outside")
    
    assertEqual(button.capturedID, nil, "initial captured id should be nil")
    assertEqual(button:getCapture(insideEndTouch), nil, "captured id should still be nil")
    
    assertEqual(button:getCapture(insideBeganTouch), 3, "captured id should be 3")
end

Yay, we’ve found a bug!

    assertEqual(button:getCapture(insideBeganTouch), 3, "captured id should be 3")
    
    assertEqual(button:getCapture(outsideBeganTouch), 3, "id should still be 3")

We expect when we have a captured ID, an outside touch with state-BEGAN will be ignored, leaving the same ID. This does not happen: the Button captures the new ID, which happens to be four. We believe that the bug will be in our if sequence, due to Lua’s short-circuiting of and. The code is:

function Button:getCapture(touch) 
    if touch.state == BEGAN and self:insideCircle(touch) and not self.capturedID then
        return touch.id
    elseif touch.state == ENDED and self.capturedID == touch.id then
        return nil
    else
        return self.capturedID
    end
end

Sure enough, this code will accept any BEGAN, no matter whether the touch is inside, and no matter whether we have a captured ID. (It occurs to me that we probably could write another test, starting from nil, expecting to stay there, that would also fail.) What are we going to do about these short-circuit things?

Oh this is fascinating! Our code is right, so far, and our tests are wrong. Here are revised tests that run green:

function testButton()
    local button = Button(nil, 100, 100, "nothing")
    local insideEndTouch    = { id=1, state=ENDED, x=100, y=100 }
    local outsideEndTouch   = { id=2, state=ENDED, x=151, y=100 }
    local insideBeganTouch  = { id=3, state=BEGAN, x=100, y=100 }
    local outsideBeganTouch = { id=4, state=BEGAN, x=151, y=100 }
    
    assertTrue(button:insideCircle(insideEndTouch), "should be inside")
    
    assertFalse(button:insideCircle(outsideEndTouch), "should be outside")
    
    assertEqual(button.capturedID, nil, "initial captured id should be nil")
    
    button:touched(insideEndTouch)
    assertEqual(button.capturedID, nil, "captured id should still be nil")
    
    button:touched(insideBeganTouch)
    assertEqual(button.capturedID, 3, "captured id should be 3")
    
    button:touched(outsideBeganTouch)
    assertEqual(button.capturedID, 3, "id should still be 3")
end

Note that we’re now calling touched on Button, not getCapture. That’s because Button’s code looks like this:

function Button:touched(touch) 
    self.capturedID = self:getCapture(touch)
    self.pressed = self:getPressed(touch)
end

function Button:getCapture(touch) 
    local answer = touch.state == BEGAN and self:insideCircle(touch) and not self.capturedID
    if touch.state == BEGAN and self:insideCircle(touch) and not self.capturedID then
        return touch.id 
    elseif touch.state == ENDED and self.capturedID == touch.id then
        return nil
    else
        return self.capturedID
    end
end

We were calling getCapture, expecting the button to remember its previous captured ID. But the ID is never saved in getCKapture, only in touched. So calling touched shows that, so far, our logic works.

But our brains were down a rathole almost this whole time, because yesterday, looking at the short-cut logic, we concluded, mistakenly, that the order of things in an and makes a difference. It does not. So most of yesterday and all of today, we were looking in the wrong place for our problem. We fiddled with nesting our and logic and other horrible things. We almost wound up on slashdot! All because we had our heads in the wrong orifice.

What’s this saying to me? It’s saying that at least here, with this logic, we really need this testing, because we are, if not stupid, at least easily confused. Tozier: “The strength of a powerful toolkit is to make the well-meaning appear stupid.” Um, OK.

Despite that I’m wishing I hadn’t promised “warts and all”, this is exactly why we do TDD-style testing, and it is a really good argument that even though I think I know how to write a program like this without unit testing, I’m not as right as I might be.

Lesson learned, at least for now …

Tozier is asking me some unintelligible question about legacy code and testing at a lower level, noting that testing at the slightly higher level gave us better results. I don’t know what he meant, but it does seem that testing at the lowest level wasn’t the best possible idea and this code was only one-day-old “legacy”. And we forgot, or didn’t notice, that our Button object had state and in our (OK, my) zeal to test the functions, I lost the information I needed to do the testing. In a situation where we don’t know the code, might we test at too low a level and encounter similar trouble? It’s certainly worth thinking about.

But I do know this: when Chet and I teach the Certified Scrum Developer course, and we talk about legacy code, we suggest writing a “characterization test” first, which surrounds the whole blob of code we’re working on, and ensures that we don’t break it. This isn’t quite the same thing but it does suggest that we need to be careful about where we stand as we test.

In TDD, I imagine that we’d have been testing the Button, and we would probably have considered those two get functions to be private, and we might not have tested them directly, instead always testing through touched(). Had we done that we might not have gotten in trouble … or at least it would have been different trouble.

Conclusions? I have none. Some leanings? Yes. We should probably test sooner and more comprehensively, especially where logic is the least bit tricky. And we should be sure we’re testing objects, not methods.

As we do more testing, we’ll need better test organization, whether that comes down to some kind of testing classes or to something else. And I’m already feeling the need for a bit more information from a failing test, and less from one that succeeds. We’ll keep an eye on that as we go forward.