2
\$\begingroup\$

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()
\$\endgroup\$

1 Answer 1

Reset to default
4
\$\begingroup\$

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())
\$\endgroup\$
0

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.