Python unit testing for a Stack data structure

Question

I am completely new to unit testing. I read this chapter in Dive Into Python 3, and decided to give it a try on a basic data structure: a stack.

Here is the stack code (taken from "Problem Solving with Algorithms and Data Structures using Python"):

class Stack:
     def __init__(self):
         self.items = []

     def isEmpty(self):
         return self.items == []

     def push(self, item):
         self.items.append(item)

     def pop(self):
         return self.items.pop()

     def peek(self):
         return self.items[len(self.items)-1]

     def size(self):
         return len(self.items)

Here is my testing code:

import unittest

class StackTester(unittest.TestCase):

    def test_stack_init(self):
        s = Stack()
        self.assertEqual(0, s.size())

    def test_push1_size(self):
        item = 5
        s = Stack()
        s.push(item)
        self.assertEqual(1, s.size())

    def test_push1_items(self):
        s = Stack()
        s.push(5)
        self.assertEqual([5], s.items)

    def test_push2_size(self):
        s = Stack()
        s.push(5)
        s.push(6)
        self.assertEqual(2, s.size())

    def test_push2_items(self):
        s = Stack()
        s.push(5)
        s.push(6)
        self.assertEqual([5,6], s.items)

    def test_push2_pop1_size(self):
        s = Stack()
        s.push(5)
        s.push(6)
        s.pop()
        self.assertEqual(1, s.size())

    def test_push2_pop1_value(self):
        s = Stack()
        s.push(8)
        s.push(9)
        self.assertEqual(9, s.pop())

    def test_push2_pop2_size(self):
        s = Stack()
        s.push("Glob")
        s.push("Blob")
        s.pop()
        s.pop()
        self.assertEqual(0, s.size())

    def test_push2_pop2_value(self):
        s = Stack()
        s.push("Glob")
        s.push("Blob")
        s.pop()
        self.assertEqual("Glob", s.pop())

    def test_isEmpty_init(self):
        s = Stack()
        self.assertEqual(True, s.isEmpty())

    def test_isEmpty_push1(self):
        s = Stack()
        s.push(5)
        self.assertEqual(False, s.isEmpty())

    def test_isEmpty_push1_pop1(self):
        s = Stack()
        s.push(1)
        s.pop()
        self.assertEqual(True, s.isEmpty())

    def test_peek_push1(self):
        s = Stack()
        s.push(88)
        self.assertEqual(88, s.peek())

    def test_peek_push3_pop1(self):
        s = Stack()
        s.push(7)
        s.push(889)
        s.push(3)
        s.pop()
        self.assertEqual(889, s.peek())

    def test_pop_empty(self):
        s = Stack()
        self.assertRaises(IndexError, s.pop)

    def test_peek_empty(self):
        s = Stack()
        self.assertRaises(IndexError, s.peek)

if __name__ == '__main__':
    unittest.main()

Am I leaving anything out? Is 16 tests for a stack overkill? What are the best practices for something like this?

Answer 1

There are certainly some good tests here. In a related question for Java, the answer by Phil Wright states:

Here are some tests I'd perform at the very least. There may be more you'd want:

1. Create an empty Stack. Test that its size is 0.
2. Push an element onto the stack. Test that its size is now 1.
3. Push another element onto the stack. Test that its size is now 2.
4. Pop an element from the stack. Test that it matches the 2nd pushed value. Check that the size of the stack is now 1.
5. Pop an element from the stack. Test that it matches the 1st pushed value. Check that the size of the stack is 0.
6. Attempt to pop an element from the stack. You should receive an ArrayIndexOutOfBounds exception.

At a quick glance, you do most (if not all of these tests). So in some sense the coverage is good.

The amount of testing may be overkill. You have to realize you can't test every possible thing. With that in mind your tests should:

1. Find current issues with your code.
2. Nip likely future issues.
3. Prevent errors you've had in the past from coming back.

The past, present, and future. So for example, your function test_peek_push3_pop1 may have a good reason for being there. Maybe you had designed a stack before and had issues with pushing 3, popping 1, and then peeking. I've never personally had such an issue. It seems like a weird test case I wouldn't consider it, but maybe you have. If you have a compelling reason to have that test you should explain your reasoning in a comment so I can understand the purpose of the test. Ultimately:

Think and comment WHY you have a test.

Answer 2

In addition to @Dair's answer I have a couple of points to consider:

Consider your public API

When you write unit tests, ideally you want to be able to only test via the public API. This helps you to understand and exercise your API. It also helps to maintain a degree of separation between your unit tests and your implementation which makes it easier to refactor the implementation without having to update the tests.

Consider this test:

def test_push2_pop2_value(self):
    s = Stack()
    s.push("Glob")
    s.push("Blob")
    s.pop()
    self.assertEqual("Glob", s.pop())

It validates the behaviour of the Stack via the public API of the push and pop methods. In contrast:

def test_push2_items(self):
    s = Stack()
    s.push(5)
    s.push(6)
    self.assertEqual([5,6], s.items)

This test Peeks inside of the Stack class to look at the items member. This feels like it's peeking at the implementation of the Stack.

Consider naming

You've got two tests:

def test_isEmpty_init(self):
def test_isEmpty_push1(self):

One tests that the stack is empty, the other tests that it isn't. From the test names, it isn't obvious that this will be the case. Consider adding an expectation element to your naming. This helps the reader to know what it is a test is validating, without having to read the whole test:

stack_oncreate_isEmpty_True
stack_push1_isEmpty_False