This is my first real attempt at unit testing. This is an area I'm currently focused on improving.
I say this because I would greatly appreciate any and every improvement!
Stack code:
class Stack:
def __init__(self):
self.items = []
self.size = 0
def is_empty(self):
return not self.items
def get_size(self):
return self.size
def pop(self):
if self.is_empty():
raise IndexError, 'Stack Empty'
else:
self.size =- 1
return self.items.pop()
def push(self, item, *args):
if isinstance(item, list):
self._push_list(item)
else:
self.items.append(item)
self.size += 1
if args:
for a in args:
self.items.append(a)
self.size += 1
def _push_list(self, l):
for item in l:
self.items.append(item)
self.size += len(l)
def get_stack(self):
while not self.is_empty():
return self.pop()
def peek(self):
if self.size == 0:
return None
else:
return self.items[self.size - 1]
Unit Test Code:
import unittest
from extras import Stack
class TestStackMethods(unittest.TestCase):
def setUp(self):
self.s = Stack()
self.l = [] # a generic list to append to
def tearDown(self):
while not self.s.is_empty():
self.s.pop()
def test_test(self):
self.assertEqual(2, (4/2))
def test_empty_function_with_items_in_stack(self):
self.s.push(1)
self.failIf(self.s.is_empty())
def test_empty_function_on_empty_stack(self):
self.failUnless(self.s.is_empty())
def test_pop_function_with_items_in_stack(self):
temp = 1
self.s.push(temp)
x = self.s.pop()
self.assertEqual(temp, x)
def test_pop_function_on_empty_stack(self):
with self.assertRaises(IndexError) as c :
self.s.pop()
self.assertTrue('Stack Empty' in c.exception)
def test_peek_function_with_items_in_stack(self):
self.s.push(1)
self.s.push(2)
self.assertTrue(self.s.peek() == 2)
def test_peek_function_on_empty_stack(self):
self.assertIs(self.s.peek(), None)
def test_push_function_for_one_item(self):
self.s.push(10)
self.assertTrue(self.s.size == 1)
def test_push_function_on_a_list_arg(self):
args = [1,2,3,4]
self.s.push(args)
self.assertTrue(self.s.size, 4)
def test_push_function_on_multiple_args(self):
self.s.push(1,2,3,4)
self.assertTrue(self.s.size, 4)
def test_lifo_functionality(self):
self.s.push(1,2,3,4)
while not self.s.is_empty():
self.l.append(self.s.pop())
self.assertEqual(self.l, [4,3,2,1])
if __name__ == '__main__':
unittest.main()
1 Answer 1
I'm just going to review the unit tests here.
Overall, it looks reasonably good. I don't have many changes to suggest.
Drop test_test
I think you probably created this to experiment with the testing infrastructure. It doesn't contribute to testing Stack
, so it no longer needs to remain.
The tear-down is unnecessary
The Stack
should not need its items removing after use, so just drop the tearDown
method. In fact, the setUp
is debatable - it might be as easy to have a local s = Stack()
in each test, rather than sharing a member.
Avoid deprecated methods
Replace failUnless
with assertTrue
, and failIf
with assertFalse
.
Simplest tests first
This might just be opinion, but I'd start with test_empty_function_on_empty_stack
before test_empty_function_with_items_in_stack
, and I'd write test_pop_function_on_empty_stack
before test_pop_function_with_items_in_stack
. One reason is that when I do test-driven development, it's natural to test the empty stack functions before implementing push
.
Don't use assertTrue
for comparison
Here, we don't get much information if the test fails:
def test_push_function_for_one_item(self):
self.s.push(10)
self.assertTrue(self.s.size == 1)
Instead, we want
self.assertEqual(1, self.s.size)
Avoid loops in tests
I'd be more explicit in the final test:
def test_lifo_functionality(self):
self.s.push(1,2,3,4)
self.assertEqual(4, self.s.pop())
self.assertEqual(3, self.s.pop())
self.assertEqual(2, self.s.pop())
self.assertEqual(1, self.s.pop())
self.assertTrue(self.s.is_empty())
Modified code
class TestStackMethods(unittest.TestCase):
def setUp(self):
self.s = Stack()
def test_empty_function_on_empty_stack(self):
self.assertTrue(self.s.is_empty())
self.assertEqual(0, self.s.size)
def test_empty_function_with_items_in_stack(self):
self.s.push(1)
self.assertFalse(self.s.is_empty())
def test_pop_function_on_empty_stack(self):
with self.assertRaises(IndexError) as c :
self.s.pop()
self.assertTrue('Stack Empty' in c.exception)
def test_pop_function_with_items_in_stack(self):
self.s.push(1)
self.assertEqual(1, self.s.pop())
def test_peek_function_on_empty_stack(self):
self.assertIs(self.s.peek(), None)
def test_peek_function_with_items_in_stack(self):
self.s.push(1)
self.s.push(2)
self.assertEqual(2, self.s.peek())
def test_push_function_for_one_item(self):
self.s.push(10)
self.assertEqual(1, self.s.size)
def test_push_function_on_a_list_arg(self):
args = [1,2,3,4]
self.s.push(args)
self.assertEqual(4, self.s.size)
def test_push_function_on_multiple_args(self):
self.s.push(1,2,3,4)
self.assertEqual(4, self.s.size)
def test_lifo_functionality(self):
self.s.push(1,2,3,4)
self.assertEqual(4, self.s.pop())
self.assertEqual(3, self.s.pop())
self.assertEqual(2, self.s.pop())
self.assertEqual(1, self.s.pop())
self.assertTrue(self.s.is_empty())