follow review recommandations: remove redundant tests

blobquiet · Mar 17, 2023 · 958b682 · 958b682
1 parent 72ecf66
commit 958b682
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 24 deletions.
diff --git a/fedbiomed/common/utils/_secagg_utils.py b/fedbiomed/common/utils/_secagg_utils.py
@@ -20,11 +20,13 @@ def matching_parties_servkey(context: dict, parties: list) -> bool:
     # eg: [ 'un', 'deux', 'trois' ] compatible with [ 'un', 'trois', 'deux' ]
     # but not with [ 'deux', 'un', 'trois' ]
     return (
-        isinstance(context, dict) and
-        'parties' in context and 
-        isinstance(context['parties'], list) and
-        len(context['parties']) >= 1 and
-        isinstance(parties, list) and
+        # Commented tests can be assumed from calling functions
+        #
+        # isinstance(context, dict) and
+        # 'parties' in context and 
+        # isinstance(context['parties'], list) and
+        # len(context['parties']) >= 1 and
+        # isinstance(parties, list) and
         parties[0] == context['parties'][0] and
         set(parties[1:]) == set(context['parties'][1:]))
 
@@ -44,10 +46,13 @@ def matching_parties_biprime(context: dict, parties: list) -> bool:
     # - either the existing element is not attached to specific parties (None)
     # - or existing element was established for the same parties or a superset of the parties
     return (
-        isinstance(context, dict) and
-        'parties' in context and 
-        isinstance(parties, list) and (
+        # Commented tests can be assumed from calling functions
+        #
+        # isinstance(context, dict) and
+        # 'parties' in context and 
+        # isinstance(parties, list) and
+        (
             context['parties'] is None or (
-                isinstance(context['parties'], list) and
+                # isinstance(context['parties'], list) and
                 set(parties).issubset(set(context['parties']))
             )))
diff --git a/tests/test_secagg_node.py b/tests/test_secagg_node.py
@@ -199,9 +199,11 @@ def test_secagg_servkey_setup_03_setup(self):
             self.assertEqual(reply["success"], False)
 
         for get_value, return_value in (
-            (3, False),
-            ({}, False),
-            ({'parties': None}, False),
+            # Not tested by _matching_parties* 
+            #
+            # (3, False),
+            # ({}, False),
+            # ({'parties': None}, False),
             ({'parties': ['not', 'matching', 'current', 'parties']}, False),
             ({'parties': ['my researcher', environ["ID"], 'my node2', 'my node3']}, True),
             ({'parties': ['my researcher', environ["ID"], 'my node3', 'my node2']}, True),
@@ -261,8 +263,10 @@ def test_secagg_biprime_setup_02_setup(self):
         """Tests init """
 
         for get_value, return_value in (
-            (3, False),
-            ({}, False),
+            # Not tested by _matching_parties* 
+            #
+            # (3, False),
+            # ({}, False),
             ({'parties': None}, True),
             ({'parties': ['not', 'matching', 'current', 'parties']}, False),
             ({'parties': ['my researcher', environ["ID"], 'my node2', 'my node3']}, True),

diff --git a/tests/test_secagg_researcher.py b/tests/test_secagg_researcher.py
@@ -230,11 +230,13 @@ def test_secagg_02_payload(self, patch_payload_create):
 
         for return_value, context, value in (
             (None, dummy_context, dummy_status),
-            (3, 3, False),
-            ({}, {}, False),
-            ({'toto': 1}, {'toto': 1}, False),
-            ({'parties': 3}, {'parties': 3}, False),
-            ({'parties': ['only_one_party']}, {'parties': ['only_one_party']}, False),
+            # Not tested by _matching_parties* 
+            #
+            # (3, 3, False),
+            # ({}, {}, False),
+            # ({'toto': 1}, {'toto': 1}, False),
+            # ({'parties': 3}, {'parties': 3}, False),
+            # ({'parties': ['only_one_party']}, {'parties': ['only_one_party']}, False),
             ({'parties': [environ["ID"], 'party2', 'party3']},
                 {'parties': [environ["ID"], 'party2', 'party3']}, True),
             ({'parties': [environ["ID"], 'party3', 'party2']},
@@ -377,11 +379,13 @@ def test_secagg_02_payload(self, patch_payload_create):
 
         for return_value, context, value in (
             (None, dummy_context, dummy_status),
-            (3, 3, False),
-            ({}, {}, False),
-            ({'toto': 1}, {'toto': 1}, False),
-            ({'parties': 3}, {'parties': 3}, False),
-            ({'parties': ['only_one_party']}, {'parties': ['only_one_party']}, False),
+            # Not tested by _matching_parties* 
+            #
+            # (3, 3, False),
+            # ({}, {}, False),
+            # ({'toto': 1}, {'toto': 1}, False),
+            # ({'parties': 3}, {'parties': 3}, False),
+            # ({'parties': ['only_one_party']}, {'parties': ['only_one_party']}, False),
             ({'parties': [environ["ID"], 'party2', 'party3']},
                 {'parties': [environ["ID"], 'party2', 'party3']}, True),
             ({'parties': ['party4', environ["ID"], 'party2', 'party3']},