Merge pull request #1 from evernym/code-cleanup

removing commented code, todos from test package and obsolete files
hyperledger · Feb 17, 2016 · c27a8c9 · c27a8c9
2 parents e033c55 + ad78dab
commit c27a8c9
Show file tree

Hide file tree

Showing 26 changed files with 20 additions and 276 deletions.
diff --git a/zeno/__init__.py b/zeno/__init__.py
@@ -10,12 +10,4 @@
 
 
 import importlib
-
-#__all__ = ['core']
-
-#_modules = ['core']
-
-#for m in _modules:
-#    importlib.import_module(".{0}".format(m), package='zeno')
-
 from .__metadata__ import *
diff --git a/zeno/server/node.py b/zeno/server/node.py
@@ -824,12 +824,6 @@ async def processPropagate(self, msg: Propagate, frm):
         reqDict = msg.request
         request = Request(**reqDict)
 
-        # TODO it appears this signature validation is redundant
-        # try:
-        #     self.verifySignature(reqDict)
-        # except Exception as ex:
-        #     raise SuspiciousNode from ex
-
         self.requests.addPropagate(request, frm)
 
         self.propagate(request)

diff --git a/zeno/test/client/test_faulty_client_with_faulty_node.py b/zeno/test/client/test_faulty_client_with_faulty_node.py
@@ -18,9 +18,6 @@ def nodeChangesRequest(nodeSet):
     alpha = nodeSet.Alpha
 
     # Alpha should not be blacklisted for Invalid Signature by all other nodes
-    # for node in nodeSet:
-    #     if node != alpha:
-    #         node.whitelistNode(alpha.name, 100)
     whitelistNode(alpha.name,
                   [node for node in nodeSet if node != alpha],
                   100)

diff --git a/zeno/test/eventually.py b/zeno/test/eventually.py
@@ -87,7 +87,6 @@ async def eventually(coroFunc: FlexFunc,
 
     if acceptableExceptions and not isinstance(acceptableExceptions, Iterable):
             acceptableExceptions = [acceptableExceptions]
-    # TODO make smarter. Have an optional initial_wait. ratchet up to timeout
     start = time.perf_counter()
 
     ratchet = Ratchet.fromGoalDuration(retryWait, ratchetSteps, timeout).gen() if ratchetSteps else None

diff --git a/zeno/test/helper.py b/zeno/test/helper.py
@@ -403,17 +403,6 @@ def addNode(self, name: str) -> TestNode:
         assert name in self.nodeReg
         ha, cliname, cliha = self.nodeReg[name]
 
-        # nstack = dict(name=name,
-        #               ha=ha,
-        #               main=True,
-        #               auto=auto if auto is not None else AutoMode.always)
-        #
-        # # TODO Should the `auto` for client stack always be `AutoMode.always`
-        # cstack = dict(name=cliname,
-        #               ha=cliha,
-        #               main=True,
-        #               auto=AutoMode.always)  # client stack is promiscuous, for now
-
         node = self.enter_context(
                 TestNode(name=name,
                          ha=ha,
@@ -566,7 +555,6 @@ def expectedWait(nodeCount):
     return w
 
 
-# TODO should be using StackTester.ensureConnectedToNodes for stuff like this
 async def checkNodesConnected(stacks: Iterable[NodeStacked],
                               expectedRemoteState=None,
                               overrideTimeout=None):
@@ -584,31 +572,6 @@ async def checkNodesConnected(stacks: Iterable[NodeStacked],
                         acceptableExceptions=[AssertionError, RemoteNotFound])
 
 
-# async def connectAll(nodes: TestNodeSet, expectedRemoteState: RemoteState=None):
-#     raise DeprecationWarning("Don't use connectAll any more. "
-#                              "Just run Looper.startAll and the "
-#                              "nodes should connect automatically. "
-#                              "use checkNodesConnected")
-#     # nodes.connectAll()
-#     # await checkNodesConnected(nodes, expectedRemoteState)
-
-
-# async def startAll(loop: BaseEventLoop, *nodes: Union[Node, Iterable[Node]]):
-#     raise DeprecationWarning("Don't use connectAll any more. "
-#                              "Just run Looper.startAll and the "
-#                              "nodes should connect automatically. "
-#                              "use checkNodesConnected")
-#     # ns = []
-#     # for n in nodes:  # type: Node
-#     #     if isinstance(n, Iterable):
-#     #         await startAll(loop, *n)
-#     #     else:
-#     #         n.start(loop)
-#     #         ns.append(n)
-#     # if ns:
-#     #     await checkNodesConnected(ns)
-
-
 def checkNodeRemotes(node: TestNode, states: Dict[str, RemoteState] = None,
                      state: RemoteState = None):
     assert states or state, "either state or states is required"
@@ -740,7 +703,6 @@ def checkAtMostOnePrim(node):
                               timeout=timeout))
 
 
-# TODO Change this to a better name like `checkIfPoolIsValid` or `checkIfValidPoolValid`
 def checkProtocolInstanceSetup(looper: Looper, nodes: Sequence[TestNode],
                                retryWait: float = 1,
                                timeout: float = None):
@@ -882,16 +844,6 @@ def getNext(self):
 genHa = HaGen().getNext
 
 
-# curPort = -1 + 8
-# def genHa(offset=None):
-#     global curPort
-#
-#     if offset is None:
-#         curPort += 1
-#         offset = curPort
-#     return HA("127.0.0.1", 7532 + offset)
-
-
 def genTestClient(nodes: TestNodeSet = None,
                   nodeReg=None,
                   tmpdir=None,
@@ -1132,16 +1084,12 @@ def checkPrePrepareReqSent(replica: TestReplica, req: Request):
 
 def checkPrePrepareReqRecvd(replicas: Iterable[TestReplica],
                             expectedRequest: PrePrepare):
-    # TODO Check whether the PrePrepare request was received by non-primary
-    # nodes by putting spy on eatPrePrepare
     for replica in replicas:
         params = getAllArgs(replica, replica.canProcessPrePrepare)
         assert expectedRequest in [p['pp'] for p in params]
 
 
 def checkPrepareReqSent(replica: TestReplica, clientId: str, reqId: int):
-    # TODO Check whether the Prepare request was received by nodes by putting
-    # spy on eatPrePrepare
     paramsList = getAllArgs(replica, replica.canSendPrepare)
     rv = getReturnValsForAllCallsToAMethod(replica,
                                            replica.canSendPrepare)

diff --git a/zeno/test/instances/test_instance_cannot_become_active_with_less_than_four_servers.py b/zeno/test/instances/test_instance_cannot_become_active_with_less_than_four_servers.py
@@ -75,10 +75,3 @@ def addNodeBackAndCheck(nodeIdx: int, expectedStatus: Status):
                 else:
                     expectedStatus = Status.started
                 addNodeBackAndCheck(i, expectedStatus)
-
-            # TODO RAET complains if a node A tries to connectF to node B, and B is not available, but then B
-            #     tries to connect to A, and then gets an error because A is still trying to connect. A needs to
-            #     respond to that by trying to connect to B again.
-
-            # TODO What if status is hungry, not started_hungry, and a
-            #     previously key-shared node comes up? We still need to be able to connect.
diff --git a/zeno/test/instances/test_multiple_commit.py b/zeno/test/instances/test_multiple_commit.py
@@ -10,9 +10,9 @@
 from zeno.test.helper import getNonPrimaryReplicas, getPrimaryReplica, \
     getNodeSuspicions, whitelistNode
 
-# TODO: Copy pasted code, use fixtures
 from zeno.test.testing_utils import adict
 
+
 whitelist = [Suspicions.DUPLICATE_CM_SENT.reason,
              'cannot process incoming COMMIT']
 

diff --git a/zeno/test/instances/test_multiple_pre_prepare.py b/zeno/test/instances/test_multiple_pre_prepare.py
@@ -33,11 +33,6 @@ def setup(nodeSet, up):
     # The node of the primary replica above should not be blacklisted by any
     # other node since we are simulating multiple PRE-PREPARE messages and
     # want to check for a particular suspicion
-    # for node in nodeSet:
-    #     if node != primaryRep.node:
-    #         node.whitelistNode(primaryRep.node.name,
-    #                            Suspicions.DUPLICATE_PPR_SENT.code)
-
 
     return adict(primaryRep=primaryRep, nonPrimaryReps=nonPrimaryReps)
 

diff --git a/zeno/test/node_request/test_pre_prepare/test_num_of_pre_prepare_with_f_plus_one_faults.py b/zeno/test/node_request/test_pre_prepare/test_num_of_pre_prepare_with_f_plus_one_faults.py
@@ -15,6 +15,7 @@
              'discarding message',
              'cannot process incoming PREPARE']
 
+
 @pytest.fixture(scope="module")
 def setup(startedNodes):
     A = startedNodes.Alpha

diff --git a/zeno/test/primary_election/test_node_election.py b/zeno/test/primary_election/test_node_election.py
diff --git a/zeno/test/primary_election/test_primary_election_case1.py b/zeno/test/primary_election/test_primary_election_case1.py
@@ -16,6 +16,7 @@
 whitelist = ['already got nomination',
              'doing nothing for now']
 
+
 @pytest.fixture()
 def case1Setup(startedNodes: TestNodeSet):
     nodes = startedNodes
@@ -59,11 +60,6 @@ def testPrimaryElectionCase1(case1Setup, looper, keySharedNodes):
     """
     nodes = keySharedNodes
     nodeA, nodeB, nodeC, nodeD = [nodes.getNode(nm) for nm in nodes.nodeNames]
-    # nodeNames = nodeSet.nodeNames
-    # nodeA = nodeSet.getNode(nodeNames[0])
-    # nodeB = nodeSet.getNode(nodeNames[1])
-    # nodeC = nodeSet.getNode(nodeNames[2])
-    # nodeD = nodeSet.getNode(nodeNames[3])
 
     # Doesn't matter if nodes reach the ready state or not. Just start them
     looper.run(checkNodesConnected(nodes))

diff --git a/zeno/test/primary_election/test_primary_election_contested.py b/zeno/test/primary_election/test_primary_election_contested.py
@@ -46,9 +46,6 @@ def testPrimaryElectionContested(electContFixture, looper, keySharedNodes):
     All see the others have sent Primary A, and then the nodes record who is the Primary.
     """
 
-    # TODO what if not all send out Primary(A)?
-    # TODO what if there are big delays in messages getting delivered?
-
     nodeSet = keySharedNodes
     A, B, C, D = nodeSet.nodes.values()
 

diff --git a/zeno/test/primary_election/test_primary_election_with_clear_winner.py b/zeno/test/primary_election/test_primary_election_with_clear_winner.py
@@ -54,11 +54,6 @@ def testPrimaryElectionWithAClearWinner(electContFixture, looper, keySharedNodes
     A, B, C, D = nodeSet.nodes.values()
     nodesBCD = [B, C, D]
 
-    # attempting to use a raet stack delay... not successful
-    # nodeBPort = nodesBCD[0].stack.ha[1]
-    # delayRef = RaetDelay(TrnsKind.alive, PcktKind.ack, nodeBPort)
-    # nodeA.stack.delay(4, delayRef)
-
     checkPoolReady(looper, nodeSet)
 
     # Checking whether one of the replicas of Node A nominated itself

diff --git a/zeno/test/primary_election/test_primary_election_with_tie.py b/zeno/test/primary_election/test_primary_election_with_tie.py
@@ -83,29 +83,5 @@ def testPrimaryElectionWithTie(electTieFixture, looper, keySharedNodes):
     for node in nodeSet.nodes.values():
         node.resetDelays()
 
-    # TODO Check for spylog of `eatReelection` after putting spy on eaters too
-
     checkProtocolInstanceSetup(looper=looper, nodes=nodeSet, retryWait=1,
                                timeout=60)
-
-    # TODO testPrimaryElectionWithTieButOneNodeDoesntKnowIt
-    # TODO add E back in after election is complete, or even before it's
-    # complete (E jumps in right in the middle of an election)
-    #       When E joins, it needs to be able to ping the others for the
-    #       current state (A could send to E the votes of the other nodes,
-    #       so E would know if B is being maliciious), and the others respond,
-    #       and E can make a determination based on those responses, even if
-    #       one is malicious.
-
-    # TODO We need to trap for the case when a node is being inconsistent. For example:
-    """
-        A is malicious
-        A sends NOMINATE(id, C) to B
-        A sends NOMINATE(id, B) to C and D
-        B forwards A's signed NOMINATE(id, C) to C and D
-        C forwards A's signed NOMINATE(id, B) to B and D
-        D forwards A's signed NOMINATE(id, B) to B and C
-        C and D and B can all see that A is being inconsistent, and they all blacklist A
-
-        If A is using different ID's for each record, then we can see that as well.
-    """
diff --git a/zeno/test/test_bootstrapping.py b/zeno/test/test_bootstrapping.py
@@ -53,7 +53,3 @@ def testConnectWithoutKeySharingFails(tdir_for_func):
                         if n == ex.args[0]]
             except Exception:
                 raise
-
-# TODO Test Bootstrapping Nodes
-
-# TODO Test Bootstrapping Clients
diff --git a/zeno/test/test_client.py b/zeno/test/test_client.py
@@ -54,8 +54,6 @@ def testClientShouldNotBeAbleToConnectToNodesNodeStack(pool):
     """
 
     async def go(ctx):
-        # TODO need to make default behavior of node stack autoMode.never
-
         for n in ctx.nodeset:
             n.nodestack.keep.auto = AutoMode.never
 
@@ -204,30 +202,4 @@ def testReplyWhenRequestAlreadyExecuted(looper, nodeSet, client1, sent1):
                                  orignalRquestResponsesLen +
                                  duplicateRequestRepliesLen),
             retryWait=.25,
-            timeout=20))
-
-
-# TODO Need to implement this and need tests for this
-# Implement client request retransmission on request timeout(Client sends a
-# request but does not receive f+1 replies
-#  in a predeclared time interval). Take care of cases where the client
-# retransmits request but the previous request
-#  is still being processed. Consider edge cases around request sequence
-# numbers.
-
-# TODO Need tests for these
-"""
-1. Client sends 3 requests with sequence numbers s1, s2, s3 in order. For s2
-client does not get the reply in time
-    so it retransmits the request with the sequence number s2.
-    a. Request with sequence number s2 reached only a faulty node(which did
-    some tampering with the request,
-    probably changing the request sequence number) and was later rejected by
-    the other nodes so it was not processed
-    b. Processing request with sequence number s2 is taking time probably
-    because nodes are busy under a high load
-    or the request has an expensive operation to perform.
-
-2. What if client get f+1 replies but with different results from different
-replicas?
-"""
+            timeout=20))
diff --git a/zeno/test/test_crypto.py b/zeno/test/test_crypto.py
@@ -126,6 +126,3 @@ def testSimpleSigningWithSimpleKeys():
     assert len(pubkey) == 32
     msg = b'1234'
     cmsg = crypto_sign(msg, prikey)
-    # TODO would like to know the details as to why the lines below don't work
-    # dmsg = crypto_sign_open(cmsg, pubkey)
-    # assert msg == dmsg  # why can't we sign with a "regular" key pair?
diff --git a/zeno/test/test_delay.py b/zeno/test/test_delay.py
@@ -64,8 +64,8 @@ def testSelfNominationDelay(tdir_for_func):
                 # nodesBCD.append(nodeSet.addNode(name, i+1, AutoMode.never))
                 nodesBCD.append(addNodeBack(nodeSet, looper, name))
 
-            # Ensuring that NodeA is started before any other node to demonstrate that it is delaying
-            # self nomination
+            # Ensuring that NodeA is started before any other node to demonstrate
+            # that it is delaying self nomination
             looper.run(
                     eventually(lambda: assertExp(nodeA.isReady()), retryWait=1,
                                timeout=5))
@@ -80,7 +80,8 @@ def testSelfNominationDelay(tdir_for_func):
                                retryWait=1,
                                timeout=10))
 
-            # Make sure that after at the most 30 seconds, nodeA's `startElection` is called
+            # Make sure that after at the most 30 seconds, nodeA's
+            # `startElection` is called
             looper.run(eventually(lambda: assertExp(
                     len(nodeA.spylog.getAll(
                             Node.decidePrimaries.__name__)) > 0),

diff --git a/zeno/test/test_node_connection.py b/zeno/test/test_node_connection.py
@@ -122,6 +122,3 @@ def testNodeConnection():
             looper.runFor(4)
             B.start()
             looper.run(checkNodesConnected([A, B]))
-
-            # TODO need to vary the times between nodes coming up in order to
-            # TODO     see where issues are