Fixed test failure from #72

charms/reactive/relations.py

@@ -16,7 +16,6 @@
 
 import os
 from inspect import isclass
-from subprocess import CalledProcessError
 
 from six import with_metaclass
 
@@ -610,22 +609,21 @@ def get_remote(self, key, default=None):
         converging to identical data.  Thus, this method returns the first
         value that it finds set by any of its units.
         """
+        cur_rid = hookenv.relation_id()
+        departing = hookenv.hook_name().endswith('-relation-departed')
         for relation_id in self.relation_ids:
-            for unit in self.units:
-                try:
-                    value = hookenv.relation_get(key, unit, relation_id)
-                    if value:
-                        return value
-                except CalledProcessError:
-                    # Our unit list might be inaccurate (perhaps a hook error
-                    # during -relation-departed that was `juju resolved`?) so
-                    # ignore units that fail to return data.  This could mask
-                    # a failure to connect to the state server, but in that
-                    # case, we have bigger problems, and it will at least be
-                    # logged.
-                    hookenv.log('Error getting relation data for {}; ignoring'
-                                '(did unit miss depart?)'.format(unit))
-                    pass
+            units = hookenv.related_units(relation_id)
+            if departing and cur_rid == relation_id:
+                # Work around the fact that Juju 2.0 doesn't include the
+                # departing unit in relation-list during the -departed hook,
+                # by adding it back in ourselves.
+                units.append(hookenv.remote_unit())
+            for unit in units:
+                if unit not in self.units:
+                    continue
+                value = hookenv.relation_get(key, unit, relation_id)
+                if value:
+                    return value
         return default
 
     def set_local(self, key=None, value=None, data=None, **kwdata):

tests/test_relations.py

@@ -443,15 +443,19 @@ def test_set_remote(self, relation_set):
 
     @mock.patch.object(relations.hookenv, 'relation_get')
     @mock.patch.object(relations.hookenv, 'related_units')
-    @mock.patch.object(relations.Conversation, 'relation_ids', ['rel:1', 'rel:2'])
+    @mock.patch.object(relations.Conversation, 'relation_ids', ['rel:1',
+                                                                'rel:2'])
     def test_get_remote(self, related_units, relation_get):
-        conv = relations.Conversation('rel', ['srv1/0', 'srv2/0', 'srv2/1'], 'scope')
+        conv = relations.Conversation('rel',
+                                      ['srv1/0', 'srv2/0', 'srv2/1'],
+                                      'scope')
 
         # set on at least one remote
         related_units.side_effect = [['srv1/0', 'srv1/1'], ['srv2/1']]
         relation_get.side_effect = [None, 'value']
         self.assertEqual(conv.get_remote('key', 'default'), 'value')
-        self.assertEqual(related_units.call_args_list, [mock.call('rel:1'), mock.call('rel:2')])
+        self.assertEqual(related_units.call_args_list, [mock.call('rel:1'),
+                                                        mock.call('rel:2')])
         self.assertEqual(relation_get.call_args_list, [
             mock.call('key', 'srv1/0', 'rel:1'),
             mock.call('key', 'srv2/1', 'rel:2'),
@@ -467,6 +471,28 @@ def test_get_remote(self, related_units, relation_get):
         relation_get.side_effect = AssertionError('relation_get should not be called')
         self.assertEqual(conv.get_remote('key', 'default'), 'default')
 
+    @mock.patch.object(relations.hookenv, 'relation_get')
+    @mock.patch.object(relations.hookenv, 'related_units')
+    @mock.patch.object(relations.Conversation, 'relation_ids', ['rel:1',
+                                                                'rel:2'])
+    @mock.patch.object(relations.hookenv, 'hook_name',
+                       lambda: 'rel-relation-departed')
+    @mock.patch.object(relations.hookenv, 'relation_id', lambda: 'rel:2')
+    @mock.patch.object(relations.hookenv, 'remote_unit', lambda: 'srv2/0')
+    def test_get_remote_departed(self, related_units, relation_get):
+        conv = relations.Conversation('rel',
+                                      ['srv1/0', 'srv2/0'],
+                                      'scope')
+
+        related_units.side_effect = [['srv1/0'], ['srv2/0']]
+        relation_get.side_effect = [None, 'value']
+        self.assertEqual(conv.get_remote('key', 'default'), 'value')
+        self.assertEqual(related_units.call_args_list, [mock.call('rel:1'),
+                                                        mock.call('rel:2')])
+        self.assertEqual(relation_get.call_args_list,
+                         [mock.call('key', 'srv1/0', 'rel:1'),
+                          mock.call('key', 'srv2/0', 'rel:2')])
+
     @mock.patch.object(relations.unitdata, 'kv')
     def test_set_local(self, kv):
         conv = relations.Conversation('rel', ['srv1/0', 'srv2/0', 'srv2/1'], 'scope')