Take-Away
Hello, loves!
Buzz is supposed to take the flower. Make it so. Some thoughts on testing while merely human.
Blurb says it all. Let’s look at Buzz’s code when he encounters Dot while she has the yellow flower.
class QuestGiverDenizen:
def __init__(self, *, knowledge):
self.knowledge = knowledge
self.machine=Machine(
initial_state='seeking',
seeking=State(
has_item=Event(to='satisfied', hook=self._giving_hook),
no_item=Event(to='seeking', hook=self._seeking_hook),
),
satisfied=State(
has_item=Event(to='satisfied', hook=self._satisfied_hook),
no_item=Event(to='satisfied', hook=self._satisfied_hook),
),
)
def event(self, event, interactor):
self.machine.event(event, interactor)
def _seeking_hook(self, interactor):
interactor.announce(next(self.knowledge.seeking_sayings))
def _giving_hook(self, interactor):
for saying in self.knowledge.giving_sayings:
interactor.announce(saying)
interactor.receive_content(self.knowledge.gift)
def _satisfied_hook(self, interactor):
interactor.announce(next(self.knowledge.satisfied_sayings))
The current action is in the _giving_hook, where he says his entire piece and then gives his gift, using the interactor’s receive_content method.
I’d like to understand how we get the has_item vs no_item decision made, as that will tell us what object we want to take.
That’s here:
class ContentFactory:
def quest_giver(self, *, name, quest_item,
seeking_sentences, giving_sentences, satisfied_sentences,
gift, resources, scale):
def interaction(self, interactor):
event = 'has_item' if interactor.has(quest_item) else 'no_item'
self.info.denizen.event(event, interactor)
return False
knowledge = self.create_knowledge(name, seeking_sentences, giving_sentences, satisfied_sentences, gift,
quest_item)
denizen = QuestGiverDenizen(knowledge=knowledge)
info = SimpleNamespace(denizen=denizen)
return Content(name=name, resources=resources,
scale=scale, interaction=interaction,
contained_items=(gift,),
info=info)
OK, the quest item is in knowledge so we can presumably ask the interactor to remove it, which it will not know how to do. Do we have tests for this guy? That would be useful, though I do feel that we could get this going via Wishful Thinking. Tests are better and last longer.
We do have tests for the denizen, including this one:
def test_table(self):
interactor = FakeInteractor()
seek_text = NameSayer.cycle('Buzz', ['want flower'])
gift_text = NameSayer.once('Buzz', ['here comb'])
sat_text = NameSayer.cycle('Buzz', ['bzzz'])
knowledge = SimpleNamespace(
gift='delicious honeycomb',
seeking_sayings=seek_text,
giving_sayings=gift_text,
satisfied_sayings=sat_text,
quest_item='a flower'
)
bee = QuestGiverDenizen(knowledge=knowledge)
bee.machine.event('has_item', interactor)
assert bee.machine.state == 'satisfied'
assert 'delicious honeycomb' in interactor.inventory
assert interactor.kwargs['message'] == 'Buzz: "here comb"'
We can just add a couple of assertions to this little story test:
bee = QuestGiverDenizen(knowledge=knowledge)
assert interactor.has('a flower')
bee.machine.event('has_item', interactor)
assert not interactor.has('a flower')
assert bee.machine.state == 'satisfied'
assert 'delicious honeycomb' in interactor.inventory
assert interactor.kwargs['message'] == 'Buzz: "here comb"'
We’ll assert that Dot has the flower, and then after the event, she hasn’t. This test should be failing on the not assertion.
But it isn’t. My bad, I need to give the FakeInteractor the item so that it will know it is there:
interactor.receive_content('a flower')
bee = QuestGiverDenizen(knowledge=knowledge)
assert interactor.has('a flower')
bee.machine.event('has_item', interactor)
assert not interactor.has('a flower')
assert bee.machine.state == 'satisfied'
assert 'delicious honeycomb' in interactor.inventory
assert interactor.kwargs['message'] == 'Buzz: "here comb"'
Now we need to decide what the message should be to the interactor to take an item. Let’s call it remove_inventory, which seems better than receive_content on the other side. So in the Fake Interactor:
def remove_inventory(self, item):
try:
self.inventory.remove(item)
except ValueError:
pass
This is the fairly standard “ask forgiveness rather than permission” style that Python folx seem to prefer. Test is still failing, of course, because we have to change the Denizen to remove the item:
def _giving_hook(self, interactor):
for saying in self.knowledge.giving_sayings:
interactor.announce(saying)
interactor.remove_inventory(self.knowledge.quest_item)
interactor.receive_content(self.knowledge.gift)
Test passes. We need the same facility in the real interactor. Do we have tests for that class? No, presumably because it is so simple. Let’s just add the method.
class Interactor:
def remove_inventory(self, item):
self.dungeon.remove_inventory(item)
class Dungeon:
def remove_inventory(self, item):
try:
self.player_inventory.remove(item)
except ValueError:
pass
This should do the job but we aren’t covered by tests, so I have to try it in the game. We’ll discuss that shortly.
I am disappointed, irritated, and confused by the fact that this didn’t work. Ah. We only have the name, not the thing. My bad. Also we need better tests. We’ll discuss that shortly, I promise.
class Dungeon:
def remove_inventory(self, item_name):
for item in self.player_inventory.copy():
if item.name == item_name:
self.player_inventory.remove(item)
return
That works as intended. Commit: QuestGiverDenizen now removes one item from inventory with its quest_item name.
I promised to talk about tests. We know that the Denizen attempts to remove the item, by name, because our FakeInteractor test does not work unless that happens. But we have no test for Dungeon’s method working, and no test that the Interactor has that method or forwards it.
If we had those tests, I think I’d not have felt the need to verify in the game, although I did want to see the inventory panel change. But I didn’t need to see it. As things stand, we did need to verify in game.
There are no tests for the Interactor, none at all. There is only one line of code that creates an Interactor, in Dungeon:
class Dungeon:
def _interactions_allow_move(self, cell):
return Interactor(self, self.pub_sub, cell).interact()
That method is called a few times. It’s called from move_player, and we do have tests for that, tons of them, in ‘test_dungeon’.
We can rig up some tests for this, using interactor or directly. But there is a hidden possibility for a mistake here, which is that we add a method to our FakeInteractor, make a note to add it to Interactor, and the dog eats our note, like she used to do with our homework.
We could make an abstract class and inherit both Fake and real Interactor from it, but what assurance have we that we’ll remember to add the new remove method to the abstract class?
Since this is Python, we could write a test that checks whether every method in FakeInteractor has a corresponding method in Interactor. I’m kind of tempted to do that, just to see what it would be like. Let’s do.
def test_real_interactor_has_all_methods(self):
required = FakeInteractor.__dict__
present = Interactor.__dict__
for name in required.keys():
assert name in present.keys()
That’s enough to fail if we don’t have everything. Not very sophisticated but good enough. If I remove our latest, the test fails:
FAILED [ 37%]
test_denizen.py:118 (TestDenizen.test_real_interactor_has_all_methods)
'remove_inventory' != dict_keys(['__module__', '__init__', 'announce', 'has', 'interact', 'publish', 'receive_content', 'xremove_inventory', '__dict__', '__weakref__', '__doc__'])
Expected :dict_keys(['__module__', '__init__', 'announce', 'has', 'interact', 'publish', 'receive_content', 'xremove_inventory', '__dict__', '__weakref__', '__doc__'])
Actual :'remove_inventory'
Messy but good enough, since we plan never to get this message. Commit the new test.
Now I think we should test the actual removal. If we do it using an Interactor on a real Dungeon, we should be in good shape for this method at least.
def test_inventory_interaction(self):
layout = DungeonLayout(10, 10)
pub_sub = PubSub()
cell = Cell(1, 1)
layout.add_room(Room([cell]))
dungeon = Dungeon(layout)
interactor = Interactor(dungeon, pub_sub, cell)
item = ContentFactory().receivable(name="treasure", resource='none', scale=0.5)
interactor.receive_content(item)
assert interactor.has('treasure')
interactor.remove_inventory('treasure')
assert not interactor.has('treasure')
You can see here why I’m writing fewer tests than would be ideal. I don’t mean “ideal against some fascist dictator’s demand that we must test everything”, I mean “ideal for my actual situation, where it would be easy to forget this method and I can only test it by running the entire game.” This test, had I written it, would have given me a lot of confidence that Buzz was actually taking the flower, and while I’d still have run the game, it would be more just to watch it work than to ensure that it did.
However, the test requires a truly irritating amount of setting up, requiring the use of six classes just to test one line of actual code.
It would be much easier to just test the Dungeon itself. Let’s do that for the comparison:
def test_dungeon_remove_inventory(self):
dungeon = Dungeon(None)
dungeon.player_inventory.append(SimpleNamespace(name='treasure'))
dungeon.remove_inventory('treasure')
assert not dungeon.dot_has('treasure')
Still a bit convoluted. That said, inventory itself is unfinished, waiting for us to recognize that there should be a smart object for player inventory. Another reason not to write very many tests, knowing that all the underpinnings are going to change, and thinking that if we test we’ll just have to change the tests anyway.
Sliding into Summary Mode …
That may be true … but even if it is, we may never improve the code and meanwhile we will be subject to mistakes in manipulating the player inventory.
My experience is that whenever I find myself resisting testing something, it is a clear indication that there is something in the design that is making the test undesirable. What “really needs to be done” is to improve the situation by improving the code.
But “what really needs to be done”, and “what we do anyway”, are often very different things, because real and imagined priorities get in the way. Here chez Ron, what usually gets in the way are impatience and laziness, two of my least excellent traits.
I am at best merely human …
I observe this openly and freely, because I am at best merely human, and my purpose here is to show you what happens when I, at best merely human, program. I don’t always do the right thing, even when I know it, and I don’t always know the right thing to do.
What I do seem to know how to do is to make small changes that make thing better, and to make enough small changes to keep the code alive, or to cast a series of small healing spells to bring it back.
In my long and varied experience, if my teams and I had just known that much, we’d have done even better than we did, and we generally did rather well.
That’s my story, and I’m sticking to it. Buzz accepts the flower, and the code is better tested than it was. Commit these tests and get outa here. See you next time!
