More coverage

dcicutils/contribution_utils.py

@@ -368,14 +368,16 @@ def contributor_index_by_primary_name(cls, contributors_by_name: ContributorInde
         :return: a contributor index
         """
         seen = set()
+        nicknames_seen = set()
         contributor_items = []
         contributors = {}
         for name, contributor in contributors_by_name.items():
             if contributor not in seen:
                 for nickname in contributor.names:
-                    if nickname in seen:
+                    if nickname in nicknames_seen:
                         raise Exception(f"Name improperly shared between {contributor}"
                                         f" and {contributors_by_name[nickname]}")
+                    nicknames_seen.add(nickname)
                 contributor_items.append((contributor.primary_name, contributor))
                 seen.add(contributor)
         for name, contributor in sorted(contributor_items,
@@ -384,15 +386,16 @@ def contributor_index_by_primary_name(cls, contributors_by_name: ContributorInde
             contributors[name] = contributor
         return contributors
 
-    def traverse(self,
+    @classmethod
+    def traverse(cls,
                  root: Contributor,
-                 cursor: Contributor,
+                 cursor: Optional[Contributor],
                  contributors_by_email: ContributorIndex,
                  contributors_by_name: ContributorIndex,
                  seen: Optional[Set[Contributor]] = None):
         if seen is None:
             seen = set()
-        if cursor in seen:
+        if cursor in seen:  # It's slightly possible that a person has a name of None that slipped in. Ignore that.
             return
         seen.add(cursor)
         for name in list(cursor.names):
@@ -402,16 +405,16 @@ def traverse(self,
         for name in list(cursor.names):
             contributor = contributors_by_name.get(name)
             if contributor and contributor not in seen:
-                self.traverse(root=root, cursor=contributor, contributors_by_email=contributors_by_email,
-                              contributors_by_name=contributors_by_name, seen=seen)
+                cls.traverse(root=root, cursor=contributor, contributors_by_email=contributors_by_email,
+                             contributors_by_name=contributors_by_name, seen=seen)
         for email in list(cursor.emails):
             contributor = contributors_by_email.get(email)
             if contributor and contributor not in seen:
-                self.traverse(root=root, cursor=contributor, contributors_by_email=contributors_by_email,
-                              contributors_by_name=contributors_by_name, seen=seen)
+                cls.traverse(root=root, cursor=contributor, contributors_by_email=contributors_by_email,
+                             contributors_by_name=contributors_by_name, seen=seen)
 
     @classmethod
-    def contributor_values_as_dicts(cls, contributor_index: ContributorIndex):
+    def contributor_values_as_dicts(cls, contributor_index: Optional[ContributorIndex]):
         if contributor_index is None:
             return None
         else:
@@ -421,7 +424,7 @@ def contributor_values_as_dicts(cls, contributor_index: ContributorIndex):
             }
 
     @classmethod
-    def contributor_values_as_objects(cls, contributor_index: Dict):
+    def contributor_values_as_objects(cls, contributor_index: Optional[Dict]):
         if contributor_index is None:
             return None
         else:

test/test_contribution_utils.py

@@ -313,6 +313,33 @@ def test_contributor_as_dict():
     }
 
 
+def test_contributor_values_as_objects():
+
+    assert Contributions.contributor_values_as_objects(None) is None
+
+    idx_json = {'jdoe': {'names': ['jdoe'], 'emails': ['jdoe@somewhere']}}
+    idx = Contributions.contributor_values_as_objects(idx_json)
+    jdoe = idx['jdoe']
+
+    assert isinstance(jdoe, Contributor)
+    assert jdoe.names == set(idx_json['jdoe']['names'])
+    assert jdoe.emails == set(idx_json['jdoe']['emails'])
+
+
+def test_contributor_values_as_dicts():
+
+    assert Contributions.contributor_values_as_dicts(None) is None
+
+    jdoe = Contributor(names={'jdoe'}, emails={'jdoe@somewhere'})
+    idx = {'jdoe': jdoe}
+    idx_json = Contributions.contributor_values_as_dicts(idx)
+    jdoe_json = idx_json['jdoe']
+
+    assert isinstance(jdoe_json, dict)
+    assert set(jdoe_json['names']) == jdoe.names
+    assert set(jdoe_json['emails']) == jdoe.emails
+
+
 def test_contributor_index_by_primary_name():
 
     john = Contributor(names={"John Doe", "jdoe"}, emails={"jdoe@a.foo"})
@@ -340,6 +367,18 @@ def test_contributor_index_by_primary_name():
         ("John Doe", john),
     ]
 
+    idx3 = {
+        "John Doe": john,
+        "jdoe": john.copy(),  # if the info is here twice, it must be in a shared pointer, not a separate object
+        "jsmith": jane,
+    }
+
+    with pytest.raises(Exception) as exc:
+        # This should notice the improper duplication.
+        Contributions.contributor_index_by_primary_name(idx3)
+
+    assert str(exc.value).startswith("Name improperly shared")
+
 
 def test_contributions_by_name_from_by_email():
 
@@ -418,3 +457,18 @@ def test_contributions_by_name_from_by_email():
     with pytest.raises(Exception) as exc:
         Contributions.by_email_from_by_name(by_name_json_with_dups)
     assert str(exc.value) == "email address ajones@somewhere is used more than once."
+
+
+def test_contributions_traverse_terminal_node():
+
+    mariela = Contributor(names={'mari', 'mariela'}, emails={'mariela@somewhere', 'mari@elsewhere'})
+    mari = Contributor(names={'mari'}, emails={'mariela@somewhere', 'mari@elsewhere'})
+
+    seen = {mari, mariela}
+    originally_seen = seen.copy()
+
+    Contributions.traverse(root=mariela, contributors_by_name={}, contributors_by_email={},
+                           # We're testing the cursor=None case
+                           seen=seen, cursor=mari)
+
+    assert seen == originally_seen