Non-None default in toggle_state to mean "toggle"

charms/reactive/relations.py

@@ -30,7 +30,9 @@
 from charms.reactive.bus import StateList
 
 
-ALL = '__ALL_SERVICES__'
+# arbitrary obj instances to use as defaults instead of None
+ALL = object()
+TOGGLE = object()
 
 
 class scopes(object):
@@ -296,13 +298,13 @@ def is_state(self, state, scope=None):
         """
         return self.conversation(scope).is_state(state)
 
-    def toggle_state(self, state, active=None, scope=None):
+    def toggle_state(self, state, active=TOGGLE, scope=None):
         """
         Toggle the state for the :class:`Conversation` with the given scope.
 
         In Python, this is equivalent to::
 
-            relation.conversation(scope).toggle_state(state)
+            relation.conversation(scope).toggle_state(state, active)
 
         See :meth:`conversation` and :meth:`Conversation.toggle_state`.
         """
@@ -549,7 +551,7 @@ def is_state(self, state):
             return False
         return self.key in value['conversations']
 
-    def toggle_state(self, state, active=None):
+    def toggle_state(self, state, active=TOGGLE):
         """
         Toggle the given state for this conversation.
 
@@ -565,7 +567,7 @@ def toggle_state(self, state, active=None):
 
         This will set the state if ``value`` is equal to ``foo``.
         """
-        if active is None:
+        if active is TOGGLE:
             active = not self.is_state(state)
         if active:
             self.set_state(state)

tests/test_relations.py

@@ -171,6 +171,11 @@ def test_toggle_state(self):
         rb.conversation.assert_called_once_with('scope')
         conv.toggle_state.assert_called_once_with('state', 'active')
 
+        conv.toggle_state.reset_mock()
+        rb.toggle_state('state')
+        conv.toggle_state.assert_called_once_with('state',
+                                                  relations.TOGGLE)
+
     def test_set_remote(self):
         conv = mock.Mock(name='conv')
         rb = relations.RelationBase('relname', 'unit')
@@ -391,12 +396,14 @@ def test_toggle_state(self):
 
         conv.toggle_state('foo')
         self.assertEqual(conv.remove_state.call_count, 1)
+        conv.toggle_state('foo', None)
+        self.assertEqual(conv.remove_state.call_count, 2)
         conv.toggle_state('foo')
         self.assertEqual(conv.set_state.call_count, 1)
         conv.toggle_state('foo', True)
         self.assertEqual(conv.set_state.call_count, 2)
         conv.toggle_state('foo', False)
-        self.assertEqual(conv.remove_state.call_count, 2)
+        self.assertEqual(conv.remove_state.call_count, 3)
 
     @mock.patch.object(relations.hookenv, 'relation_set')
     @mock.patch.object(relations.Conversation, 'relation_ids', ['rel:1', 'rel:2'])