Class per State (cd)
I plan to start with some direct tests for the state classes. I think that will be interesting. And I’m expecting to use a Test Double, which is quite rare hereabouts. Plus: Amazing Grace.
One difficulty we’ve encountered with the state machine, and with transforming it from open code to method object to class per state, is the testing. I’ve taken a kind of “story test” approach, working with the higher level objects, sometimes even including World, but certainly Bot, and running the whole do_something logic. I can believe that we might want one or two such tests, but we have a lot of tests that are done more or less that way, and they are f-r-a-g-i-l-e fragile. The fragility is largely because, as things change underneath the high-level objects, we have to find different creative ways to set up the situations we want.
Today, I plan to start with some direct tests on the two existing state classes, and then to write suitable tests for the remaining one, and only then, break it out.
I usually predict what is going to happen, and today I have a list from GeePaw saying what he thinks he would do if it were up to him:
Steps I’d take next: 1) TDD Walking, 2) Install Walking. 3) Backfill tests for Looking & Launching. 4) Kill off the 3-ple. 5) Kill off the methods that support the 3-ple. 6) Consider whether Machine should get pulled back into bots. 7) Tighten the Machine tests so they’re not so hard to set up.
I think that’s close to what will happen, though I plan to start with the backfill tests first, because I think I’ll learn something about testing them that will be useful with the final one.
Let’s begin by perusing, looking over, and examining the existing state classes:
class Looking:
def update(self, machine, knowledge):
if knowledge.has_block:
knowledge.tired = 5
return machine.walking_update, machine.walking_action, None
else:
return None, None, Looking()
def action(self, knowledge):
if knowledge.can_take:
return ['take']
return []
class Laden:
def update(self, machine, knowledge):
if not knowledge.has_block:
knowledge.tired = 5
return machine.walking_update, machine.walking_action, None
else:
return None, None, Looking()
def action(self, knowledge):
if knowledge.tired <= 0:
if knowledge.can_drop:
return ['drop']
return []
The best thing that I see when I look at these is that the Knowledge object is wise: it can answer questions like has_block and can_take, with those methods embedding the “intelligent” pattern matching or whatever it takes to do the thing. What I like about that is that it tells me that we can pass in a fake object as the knowledge, and directly control what the object should do.
Let’s try it with Looking. We’ll test the update first.
- Side Quest
- I expect to build a test double to provide
knowledgeto the object under test. There are at least three ways one might do that: a standard class, a dataclass, or a namedtuple. I don’t think I have ever used the latter two, so I’ll probably experiment a bit to see what fits best. Clearly all we really need is an object with attributed likecan_dropreturning boolean, and so on.
Let’s test Looking.action:
class Looking:
def action(self, knowledge):
if knowledge.can_take:
return ['take']
return []
class FakeKnowledge():
def __init__(self, *, can_take=False):
self.can_take = can_take
class TestClassPerStateMachine:
def test_looking_action(self):
state = Looking()
knowledge = FakeKnowledge(can_take=True)
assert state.action(knowledge) == ['toke']
Test fails as intended:
Expected :['toke']
Actual :['take']
Oops, typo in the test. Fix it:
class TestClassPerStateMachine:
def test_looking_action(self):
state = Looking()
knowledge = FakeKnowledge(can_take=True)
assert state.action(knowledge) == ['take']
Green. Commit: initial direct tests for class per state.
Rename and add a test:
def test_looking_action_can_take(self):
state = Looking()
knowledge = FakeKnowledge(can_take=True)
assert state.action(knowledge) == ['take']
def test_looking_action_cannot_take(self):
state = Looking()
knowledge = FakeKnowledge(can_take=False)
assert state.action(knowledge) == []
Green. Commit: additional tests
My cunning plan now is just to continue adding tests for update and then for Laden. Then, finally, I’ll begin to TDD the Walking class into place.
But those tests are almost trivial!
Yes, they are … and yet, they are testing exactly what needs testing in the Looking class, whether, when it can_take, it issues a take command, and otherwise does not. These are nearly good examples of what Hill calls “microtests”, and I think that when objects are able to be tested this way, it is a decent sign that they’re about the right size and have responsibilities separated reasonably well. In this project, we have very few tests like that and it’s not a good thing.
It is, however, an explainable thing. No excuse, sir, but an explanation: Our program has grown by exploration, and, owing to the fact that I am only human, it does not always have the responsibilities split out and distributed ideally. When we simply cannot write microtests, it’s a sign that we need refactoring.
Let’s test update:
class Looking:
def update(self, machine, knowledge):
if knowledge.has_block:
knowledge.tired = 5
return machine.walking_update, machine.walking_action, None
else:
return None, None, Looking()
Start with the easy case:
def test_looking_update_with_no_block(self):
state = Looking()
knowledge = FakeKnowledge(has_block=False)
machine = None
n1, n2, looking = state.update(machine, knowledge)
assert n1 is None
assert n2 is None
assert isinstance(looking, Looking)
class FakeKnowledge():
def __init__(self, *, can_take=False, has_block=False):
self.can_take = can_take
self.has_block = has_block
That one’s green. Commit: more testing.
Next test requires a machine we could instantiate a real one, but I think we’ll do a fake one. I think we’ll make a new fake class for the purpose.
Here’s the test:
def test_looking_update_with_block(self):
state = Looking()
knowledge = FakeKnowledge(has_block=True)
machine = FakeMachine()
m_update, m_action, m_class = state.update(machine, knowledge)
assert m_update == machine.walking_update
assert m_action == machine.walking_action
assert m_class is None
And the FakeMachine:
class FakeMachine:
def walking_update(self):
pass
def walking_action(self):
pass
That could have been done with a fake like the FakeKnowledge, with just numbers or something for the methods, but this is more real and not any more difficult.
We are green. One more check, though, for tired.
def test_looking_update_with_block(self):
state = Looking()
knowledge = FakeKnowledge(has_block=True)
machine = FakeMachine()
m_update, m_action, m_class = state.update(machine, knowledge)
assert m_update == machine.walking_update
assert m_action == machine.walking_action
assert m_class is None
assert knowledge.tired == 5
That passes as it stands, but PyCharm knows there was no tired defined in FakeKnowledge, so I add it, initialized to 99.
Commit: looking tests complete.
It remains to do the Laden tests. I’ll spare you the details unless something interesting happens.
Ah! With this test:
def test_laden_update_has_block(self):
state = Laden()
knowledge = FakeKnowledge(has_block=True)
u, a, c = state.update(None, knowledge)
assert u is None
assert a is None
assert isinstance(c, Laden)
I think I’ve found a bug. If we are in laden state, that means that we may have a block, unless we have just dropped it. If we have dropped it, we should go to walking state. If we still have it, we should stay in Laden state. But the code says this:
class Laden:
def update(self, machine, knowledge):
if not knowledge.has_block:
knowledge.tired = 5
return machine.walking_update, machine.walking_action, None
else:
return None, None, Looking()
That sends us to Looking, not Laden. I think that’s wrong. I’m going to change it and see what breaks. Nothing breaks, game works fine. What we did not notice yesterday was that, while the bots in the game did make piles, they were doing it more slowly. Why? Because much of the time, while laden, they were in looking state, so they would come to a place where they could have dropped but were in the wrong state to check. The other part of the time, they’d be in Laden and they would drop.
As a double check, the old code in Machine was this:
def laden_update(self):
if not self._knowledge.has_block:
self._knowledge.tired = 5
return self.walking_states()
else:
return self.laden_states()
That makes it clear that the code in the state class above should be pointing to Laden, not Looking. Fixed. Committed.
Final test is green. Commit: microtests in place for Looking and Laden. Found a defect, yay!
The Laden tests are no surprise:
def test_laden_action_not_tired_cannot_drop(self):
state = Laden()
knowledge = FakeKnowledge(tired=0, can_drop=False)
assert state.action(knowledge) == []
def test_laden_action_not_tired_can_drop(self):
state = Laden()
knowledge = FakeKnowledge(tired=0, can_drop=True)
assert state.action(knowledge) == ['drop']
def test_laden_update_has_block(self):
state = Laden()
knowledge = FakeKnowledge(has_block=True)
u, a, c = state.update(None, knowledge)
assert u is None
assert a is None
assert isinstance(c, Laden)
def test_laden_update_has_no_block(self):
state = Laden()
knowledge = FakeKnowledge(has_block=False)
machine = FakeMachine()
m_update, m_action, m_class = state.update(machine, knowledge)
assert m_update == machine.walking_update
assert m_action == machine.walking_action
assert m_class is None
assert knowledge.tired == 5
And here are the fake classes again:
class FakeKnowledge():
def __init__(self, *,
can_take=False,
can_drop=False,
has_block=False,
tired=99):
self.can_take = can_take
self.can_drop = can_drop
self.has_block = has_block
self.tired = tired
class FakeMachine:
def walking_update(self):
pass
def walking_action(self):
pass
Nice and simple.
I am absolutely elated, however, to have found that defect. First of all, it was not detected by any of the more invasive tests we had written. Second, it was an intermittent bug: sometimes the bot would be in a position to drop and it would drop. Other times it would be in a position to drop but its state would be wrong and it would not drop. This was not easy to see when watching the game, since they bounce around so rapidly, and if you followed a given bot long enough, it would get rid of its burden appropriately. It’s just that it might well have skipped some opportunities.
And these tests were so easy to write, especially given the test doubles FakeKnowledge and FakeMachine.
I think I’ll stop here. It has been about 90 minutes, which is long enough and this is a great pausing point. Maybe Hill or Bryan will want to pair later today.
Summary
We didn’t even get to the side quest, but that’s fine. I do want to experiment with other forms for the fake objects, but the way they’re done now is just dandy.
For the first time in a few sessions, I feel strongly positive about what has happened. And I only changed Looking to Laden in the production code. A net reduction in code of two characters. But what lovely progress, and I feel like a paragon of morality, having worked out how to easily write tests for the states, then having done so, and along the way finding a very subtle defect. Honestly, I am surprised that I am not up for canonization.
See you next time! And be not afraid of this biblically accurate picture of me.
