Add python-coverage configuration and fully cover the code

	$ make coverage
	…
	Name         Stmts   Miss Branch BrPart  Cover   Missing
	--------------------------------------------------------
	pygtrie.py     395      0    142      0   100%

As a result of this change, some dead code has been eliminated so that’s
another win.

.coveragerc

@@ -0,0 +1,12 @@
+# Configuration for python-coverage.  Run ‘make coverage’ to check coverage.
+
+[run]
+omit = test.py
+branch = on
+
+[report]
+exclude_lines =
+    raise NotImplementedError
+    .* # Python 2 compatibility
+show_missing = true
+fail_under = 95

.gitignore

@@ -1,4 +1,5 @@
 *.pyc
+.coverage
 .version
 MANIFEST
 build/

Makefile

@@ -1,4 +1,4 @@
-all: test lint docs
+all: test lint coverage docs
 
 test: test2 test3
 
@@ -12,7 +12,12 @@ lint: .pylintrc pygtrie.py test.py example.py
 	lint=$$(which pylint3 2>/dev/null) || lint=$$(which pylint) && \
 	"$$lint" --rcfile $^
 
+coverage: test.py pygtrie.py
+	cov=$$(which python3-coverage 2>/dev/null) || \
+	cov=$$(which python-coverage) && \
+	"$$cov" run test.py && "$$cov" report -m
+
 docs:
 	sphinx-build . html
 
-.PHONY: all test test2 test3 lint docs
+.PHONY: all test test2 test3 lint coverage docs

pygtrie.py

@@ -46,11 +46,11 @@
 import collections as _collections
 try:
     import collections.abc as _abc
-except ImportError:
+except ImportError:  # Python 2 compatibility
     _abc = _collections
 
-# Python 2.x and 3.x compatibility stuff; pylint: disable=invalid-name
-if hasattr(dict, 'iteritems'):
+# pylint: disable=invalid-name
+if hasattr(dict, 'iteritems'):  # Python 2 compatibility
     _iteritems = lambda d: d.iteritems()
     _iterkeys = lambda d: d.iterkeys()
     def _sorted_iteritems(d):
@@ -65,7 +65,7 @@ def _sorted_iteritems(d):
 
 try:
     _basestring = basestring
-except NameError:
+except NameError:  # Python 2 compatibility
     _basestring = str
 # pylint: enable=invalid-name
 
@@ -171,16 +171,14 @@ def __eq__(self, other):
             while True:
                 try:
                     key, a = next(stack[-1][0])
-                    b = stack[-1][1].get(key)
-                    if b is None:
-                        return False
+                    b = stack[-1][1][key]
                     break
                 except StopIteration:
                     stack.pop()
                 except IndexError:
                     return True
-
-        return self.value == other.value and self.children == other.children
+                except KeyError:
+                    return False
 
     def __ne__(self, other):
         return not self.__eq__(other)
@@ -241,29 +239,27 @@ def __getstate__(self):
             stack.append(_iteritems(node.children))
 
             while True:
-                try:
-                    step, node = next(stack[-1])
-                except StopIteration:
-                    if last_cmd < 0:
-                        state[-1] -= 1
-                    else:
-                        last_cmd = -1
-                        state.append(-1)
-                    stack.pop()
-                    continue
-                except IndexError:
-                    if last_cmd < 0:
-                        state.pop()
-                    return state
+                step, node = next(stack[-1], (None, None))
+                if node is not None:
+                    break
 
-                if last_cmd > 0:
-                    last_cmd += 1
-                    state[-last_cmd] += 1
+                if last_cmd < 0:
+                    state[-1] -= 1
                 else:
-                    last_cmd = 1
-                    state.append(1)
-                state.append(step)
-                break
+                    last_cmd = -1
+                    state.append(-1)
+                stack.pop()
+                if not stack:
+                    state.pop()  # Final -n command is not necessary
+                    return state
+
+            if last_cmd > 0:
+                last_cmd += 1
+                state[-last_cmd] += 1
+            else:
+                last_cmd = 1
+                state.append(1)
+            state.append(step)
 
     def __setstate__(self, state):
         """Unpickles node.  See :func:`_Node.__getstate__`."""
@@ -761,30 +757,26 @@ def _cleanup_trace(trace):
             del parent.children[step]
             step, node = parent_step, parent
 
-    def _pop_from_node(self, node, trace, default=_SENTINEL):
+    def _pop_from_node(self, node, trace):
         """Removes a value from given node.
 
         Args:
             node: Node to get value of.
             trace: Trace to that node as returned by :func:`Trie._get_node`.
-            default: A default value to return if node has no value set.
 
         Returns:
-            Value of the node or ``default``.
+            Value of the node.
 
         Raises:
             ShortKeyError: If the node has no value associated with it and
                 ``default`` has not been given.
         """
-        if node.value is not _SENTINEL:
-            value = node.value
-            node.value = _SENTINEL
-            self._cleanup_trace(trace)
-            return value
-        elif default is _SENTINEL:
+        value = node.value
+        if value is _SENTINEL:
             raise ShortKeyError()
-        else:
-            return default
+        node.value = _SENTINEL
+        self._cleanup_trace(trace)
+        return value
 
     def pop(self, key, default=_SENTINEL):
         """Deletes value associated with given key and returns it.

test.py

@@ -70,6 +70,9 @@ class TrieTestCase(unittest.TestCase):
 
     # A key to set
     _SHORT_KEY = 'foo'
+    # Another key which has all but the last component equal to the ones in
+    # _SHORT_KEY.
+    _SHORT_KEY2 = 'foO'
     # Another key to set such that _SHORT_KEY is it's prefix
     _LONG_KEY = _SHORT_KEY + 'bar'
     # A key that is not set but _LONG_KEY is it's prefix
@@ -94,6 +97,15 @@ def key_from_path(cls, path):
 
     # End of stuff that needs to be overwritten by subclasses
 
+    def __init__(self, *args, **kw):
+        super(TrieTestCase, self).__init__(*args, **kw)
+        # Python 2 compatibility.  Noisy code to confuse pylint so it does not
+        # issue deprecated-method warning. pylint: disable=invalid-name
+        for new, old in (('assertRegex', 'assertRegexpMatches'),
+                         ('assertRaisesRegex', 'assertRaisesRegexp')):
+            if not hasattr(self, new):
+                setattr(self, new, getattr(self, old))
+
     def key_from_key(self, key):
         """Turns a key into a form that the Trie will return e.g. in keys()."""
         return self.key_from_path(self.path_from_key(key))
@@ -163,6 +175,9 @@ def assertShortTrie(self, t, value=42):
             self.assertNodeState(t, key)
         self.assertNodeState(t, self._SHORT_KEY, value=value)
 
+        self.assertRegex(str(t), r'Trie\([^:]*: [^:]*\)')
+        self.assertRegex(repr(t), r'Trie\(\(\(.*, .*\),\)\)')
+
     def assertEmptyTrie(self, t):
         """Asserts a trie is empty."""
         self.assertEqual(0, len(t), '%r should be empty: %d' % (t, len(t)))
@@ -202,6 +217,31 @@ def _do_test_basics(self, trie_factory):
         self.assertEqual((self.key_from_key(self._SHORT_KEY), 24), t.popitem())
         self.assertEmptyTrie(t)
 
+        t[self._LONG_KEY] = '42'
+        self.assertRaisesRegex(pygtrie.ShortKeyError, str(self._SHORT_KEY),
+                               t.__delitem__, self._SHORT_KEY)
+
+    def _do_test_invalid_arguments(self, trie_factory):
+        """Test various methods check for invalid arguments."""
+        d = dict.fromkeys((self._SHORT_KEY, self._LONG_KEY), 42)
+        t = trie_factory(self._TRIE_CLS, d)
+
+        self.assertRaisesRegex(
+            ValueError, 'update.. takes at most one positional argument,',
+            t.update, (self._LONG_KEY, 42), (self._VERY_LONG_KEY, 42))
+
+        self.assertRaisesRegex(TypeError, r'slice\(.*, None\)',
+                               lambda: t[self._SHORT_KEY:self._LONG_KEY])
+        self.assertRaisesRegex(TypeError, r"slice\(.*, 'foo'\)",
+                               lambda: t[self._SHORT_KEY:self._LONG_KEY:'foo'])
+    def _do_test_clear(self, trie_factory):
+        """Test clear method."""
+        d = dict.fromkeys((self._SHORT_KEY, self._LONG_KEY), 42)
+        t = trie_factory(self._TRIE_CLS, d)
+        self.assertFullTrie(t)
+        t.clear()
+        self.assertEmptyTrie(t)
+
     def _do_test_iterator(self, trie_factory):
         """Trie iterator tests."""
         d = dict.fromkeys((self._SHORT_KEY, self._LONG_KEY), 42)
@@ -295,6 +335,9 @@ def assert_pair(expected, got):
                 self.assertTrue(got)
             else:
                 self.assertFalse(got)
+                self.assertFalse(got.is_set)
+                self.assertFalse(got.has_subtrie)
+                self.assertIsNone(got.get())
 
         assert_pair(short_pair, t.shortest_prefix(self._VERY_LONG_KEY))
         assert_pair(short_pair, t.shortest_prefix(self._LONG_KEY))
@@ -450,24 +493,15 @@ def check_state(p):
 
     def test_equality(self):
         """Tests equality comparison."""
-        d = dict.fromkeys((self._SHORT_KEY, self._LONG_KEY), 42)
-        # pylint: disable=redefined-outer-name
-        tries = [factory(self._TRIE_CLS, d) for _, factory in _TRIE_FACTORIES]
-
-        for i in range(1, len(tries)):
-            self.assertEqual(
-                tries[i-1], tries[i],
-                '%r (factory: %s) should equal %r (factory: %s)' %
-                (tries[i-1], _TRIE_FACTORIES[i-1][0],
-                 tries[i], _TRIE_FACTORIES[i][0]))
-
-        for i in range(1, len(tries)):
-            tries[i-1][self._OTHER_KEY] = 42
-            self.assertNotEqual(
-                tries[i-1], tries[i],
-                '%r (factory: %s) should not be equal %r (factory: %s)' %
-                (tries[i-1], _TRIE_FACTORIES[i-1][0],
-                 tries[i], _TRIE_FACTORIES[i][0]))
+        a = self._TRIE_CLS({self._SHORT_KEY: 42})
+        b = self._TRIE_CLS({self._SHORT_KEY: 42})
+        c = self._TRIE_CLS({self._SHORT_KEY: '42'})
+        d = self._TRIE_CLS({self._SHORT_KEY2: 42})
+
+        self.assertEqual(a, a)
+        self.assertEqual(a, b)
+        self.assertNotEqual(a, c)
+        self.assertNotEqual(a, d)
 
     _PICKLED_PROTO_0 = (
         'Y2NvcHlfcmVnCl9yZWNvbnN0cnVjdG9yCnAwCihjcHlndHJpZQpUcmllCnAxCmNfX2J1aW'
@@ -563,6 +597,7 @@ class StringTrieTestCase(TrieTestCase):
     _TRIE_CLS = pygtrie.StringTrie
 
     _SHORT_KEY = '/home/foo'
+    _SHORT_KEY2 = '/home/FOO'
     _LONG_KEY = _SHORT_KEY + '/bar/baz'
     _VERY_LONG_KEY = _LONG_KEY + '/qux'
     _OTHER_KEY = '/hom'