Python testing (pt 2) - False positives?
Continuing on from my Python testing part 1 post I’ve been adding basic functions to my game in a simplistic and iterative style.
My Player class has some functions to view health and view inventory. I’ve also added very simple take and drop functions that just append or remove items to the inventory list that starts off empty.
class Player(object):
""" Class object for a player of the game."""
def __init__(self, health=100, inventory=[]):
""" Create a player with full health and empty inventory."""
self.health = health
self.inventory = inventory
def take(self, i):
"""Enables an item to be taken / added to inventory."""
self.inventory.append(i)
def drop(self, i):
"""Enables an item to be dropped / removed from inventory."""
self.inventory.remove(i)
All pretty simple. Continuing the use of Pytest’s mark decorator, I’ve now created 4 inventory tests that check:
- a new player instance has an empty inventory
- I can view the inventory
- I can drop inventory items
- I can take items and add to the inventory
Something’s going on… #
When running these tests I started seeing test failures. My tests were super simple, instantiating an object, manually appending an item or using the take function to add to that object’s inventory, then asserting:
@pytest.mark.inventory
def test_view_inventory():
"""Test calling view_inventory() returns current inventory."""
p1 = Player()
p1.inventory.append('flamethrower')
assert p1.view_inventory() == "\t\t - flamethrower"
@pytest.mark.inventory
def test_take_adds_item_to_inventory():
"""Test take() can add to list."""
p2 = Player()
p2.take('iron bar')
assert p2.inventory == ['iron bar']
But the failures suggested the second object (p2) was not starting off with an empty inventory list. It was including the ‘flamethrower’ appended in the first test?!
This confused me, as the p1 and p2 should have been different instances of the Player class…?
To test what was going on here, and assuming that somehow Pytest was not recognising the different instances, I changed the instance name and ensured that the drop function ran before the take function, to clean up the inventory in sequence.
@pytest.mark.inventory
def test_view_inventory():
"""Test calling view_inventory() returns current inventory."""
p1 = Player()
p1.inventory.append('flamethrower')
assert p1.view_inventory() == "\t\t - flamethrower"
@pytest.mark.inventory
def test_drop_removes_item_from_inventory():
"""Test drop() removes item from list."""
banana = Player()
banana.drop('flamethrower')
assert banana.inventory == []
@pytest.mark.inventory
def test_take_adds_item_to_inventory():
"""Test take() can add to list."""
p2 = Player()
p2.take('iron bar')
assert p2.inventory == ['iron bar']
Sure enough this fixed the problem, even though I never added a flamethrower to my banana instance of Player().
So what’s going on? #
I haven’t a clue! I tried setting pdb and set_trace() to see if that gave any insight into the behaviour, but with no luck. What this does highlight is that it is easy to create ‘passing’ tests that do not reflect your desired behaviour.
My banana instance of Player() should have thrown an error when I tried to drop the flamethrower, as the new instance should have had a empty inventory list by default, and therefore no flamethrower in the inventory.
They are false positives; tests passing when they should not!
I am relying on my tests to drive the development process, so if they are providing false results, I may not catch the error until much later and introduce considerable rework rather than the refactoring I plan to do.
The solution… my code was wrong… #
Thanks to Zed’s fantastic support, the problem was not one with Pytest at all. It was the use of the empty list in the constructor. My expectation was that an empty list would be created with each instance.
In fact, this syntax only creates one list. You could call it a Python bug, but you’d meet some objections.
By amending the code to remove the empty list from init and return None instead. I could then manage the empty list in the assignment of the instance variable.
class Player(object):
""" Class object for a player of the game."""
def __init__(self, health=100, inventory=None):
""" Create a player with full health and empty inventory."""
self.health = health
self.inventory = inventory or []
This fix failed my tests that relied on the sequencing. So I rewrote them back to independent and isolated test cases. Job done.
