-
Notifications
You must be signed in to change notification settings - Fork 23
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
base: master
Are you sure you want to change the base?
Add tests #15
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
include *.txt | ||
include 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())) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is no There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we really need |
||
|
||
def assertRaisesAssignmentError(self, *args, **kwargs): | ||
self.assertRaisesRegexp( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is no There was a problem hiding this comment. Choose a reason for hiding this commentThe 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') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. with self.assertRaises(TypeError):
frozendict.key = value but There was a problem hiding this comment. Choose a reason for hiding this commentThe 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): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd also be tempted to add something like:
(and then perhaps the opposite for |
||
|
||
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): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ')) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, There was a problem hiding this comment. Choose a reason for hiding this commentThe 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): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.