Skip to content

Add tests #15

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions MANIFEST.in
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
include *.txt
include tests.py
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should not ship tests.py in the package

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nowadays it's recommended to ship tests too. For example, downstream packagers like Fedora want to run the tests to make sure it's working.

131 changes: 131 additions & 0 deletions frozendict/tests.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
from unittest import TestCase

from frozendict import frozendict, FrozenOrderedDict


class BaseTestCase(TestCase):
def assertMappingEqual(self, m1, m2):
# `assertDictEqual` will make a failing type check when comparing a `dict`
# with a `frozendict`. Instead, we'll compare the item sets:
self.assertSetEqual(set(m1.items()), set(m2.items()))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is no assertSetEqual in Python 2.6

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need assertSetEqual? Because set equality can be tested via == anyways and we can just use assertEqual.


def assertRaisesAssignmentError(self, *args, **kwargs):
self.assertRaisesRegexp(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is no assertRaisesRegexp in Python 2.6

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In 2020 I strongly recommend dropping 2.6 (and consider 2.7).

TypeError,
r'object does not support item assignment',
*args, **kwargs
)


def setitem(dct, key, value):
dct[key] = value


class TestFrozenDict(BaseTestCase):
def test_assignment_fails(self):
self.assertRaisesAssignmentError(setitem, frozendict(), 'key', 'value')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with self.assertRaises(TypeError):
    frozendict.key = value

but assertRaises needs to be backported. I suggest that you copy B2 CLI TestBase.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise not a problem in 2020 if dropping 2.6.


def test_init(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test is practically worthless, please remove

frozendict()

def test_init_with_keys(self):
fd = frozendict(a=0, b=1)
self.assertMappingEqual(fd, dict(a=0, b=1))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd also be tempted to add something like:

fd1 = frozendict(a=0, b=1)
fd2 = frozendict(b=1, a=0)
self.assertMappingEqual(fd1, fd2)

(and then perhaps the opposite for FrozenOrderedDict)


def test_init_with_dict(self):
fd = frozendict(dict(a=0, b=1))
self.assertMappingEqual(fd, dict(a=0, b=1))

def test_init_with_dict_and_keys(self):
fd = frozendict(dict(a=0, b=1), b=2, c=3)
self.assertMappingEqual(fd, dict(a=0, b=2, c=3))

def test_getitem(self):
fd = frozendict(key='value')
self.assertEqual(fd['key'], 'value')

def test_copy(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test doesn't check if the copy is actually the same object or not

fd = frozendict(a=0, b=1)
copied = fd.copy()
self.assertIs(type(copied), type(fd))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assertIs is new in 2.7

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK in 2020

self.assertMappingEqual(copied, fd)

def test_copy_with_keys(self):
fd = frozendict(a=0, b=1)
copied = fd.copy(b=2, c=3)
self.assertIs(type(copied), type(fd))
self.assertMappingEqual(copied, dict(a=0, b=2, c=3))

def test_iter(self):
fd = frozendict(a=0, b=1)
keys = {key for key in fd}
self.assertSetEqual(keys, {'a', 'b'})

def test_len(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

len of empty dict should also be checked

fd = frozendict(a=0, b=1)
self.assertEqual(len(fd), 2)

def test_repr(self):
fd = frozendict(a=0, b=1)
r = repr(fd)
self.assertTrue(r.startswith('<frozendict '))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assertion is too loose, we want to check what is actually in there, not just the header and the footer

self.assertTrue(r.endswith('>'))

def test_hash(self):
fd = frozendict(a=0, b=1)
hash(fd)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hashes of two frozendicts which have the same contents, should be identical



class TestFrozenOrderedDict(BaseTestCase):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

many tests in this class are copies of the tests from class above. Use inheritance and a class field to prevent repetition of code

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the FrozenOrderedDict tests should use an assertOrderedMappingEqual function?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, FrozenOrderedDict cannot be tested on Python 2.6 where it is not supported. Maybe raise nose.SkipTest() in setUp?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK in 2020

def test_assignment_fails(self):
self.assertRaisesAssignmentError(setitem, FrozenOrderedDict(), 'key', 'value')

def test_init(self):
FrozenOrderedDict()

def test_init_with_keys(self):
fd = FrozenOrderedDict(a=0, b=1)
self.assertMappingEqual(fd, dict(a=0, b=1))

def test_init_with_dict(self):
fd = FrozenOrderedDict(dict(a=0, b=1))
self.assertMappingEqual(fd, dict(a=0, b=1))

def test_init_with_dict_and_keys(self):
fd = FrozenOrderedDict(dict(a=0, b=1), b=2, c=3)
self.assertMappingEqual(fd, dict(a=0, b=2, c=3))

def test_getitem(self):
fd = FrozenOrderedDict(key='value')
self.assertEqual(fd['key'], 'value')

def test_copy(self):
fd = FrozenOrderedDict(a=0, b=1)
copied = fd.copy()
self.assertIs(type(copied), type(fd))
self.assertMappingEqual(copied, fd)

def test_copy_with_keys(self):
fd = FrozenOrderedDict(a=0, b=1)
copied = fd.copy(b=2, c=3)
self.assertIs(type(copied), type(fd))
self.assertMappingEqual(copied, dict(a=0, b=2, c=3))

def test_iter(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a 1/16th chance that this test will pass with frozendict. Please increase the test data size to reduce the chance to a more reasonable value

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, could you explain further?

fd = FrozenOrderedDict([('a', 0), ('c', 2), ('b', 1), ('d', 3)])
keys = [key for key in fd]
self.assertListEqual(keys, ['a', 'c', 'b', 'd']) # verifies order

def test_len(self):
fd = FrozenOrderedDict(a=0, b=1)
self.assertEqual(len(fd), 2)

def test_repr(self):
fd = FrozenOrderedDict(a=0, b=1)
r = repr(fd)
self.assertTrue(r.startswith('<FrozenOrderedDict '))
self.assertTrue(r.endswith('>'))

def test_hash(self):
fd = FrozenOrderedDict(a=0, b=1)
hash(fd)
4 changes: 3 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,7 @@
license = 'MIT License',

description = 'An immutable dictionary',
long_description = open('README.rst').read()
long_description = open('README.rst').read(),

test_suite = 'tests',
)