[tor-commits] [stem/master] Unit tests for parse_file() and router entries

[atagar at torproject.org]

commit 72c561b9adb7e3963aa14da98b8d9bc77df533dd
Author: Damian Johnson <atagar at torproject.org>
Date:   Sat Sep 22 17:57:49 2012 -0700

    Unit tests for parse_file() and router entries
    
    On reflection the reason that the prior changes passed the unit tests so easily
    was because the parse_file() function and inclusion of router status entries
    was completely untested by my unit tests. The RouterStatusEntry class itself it
    tested, but not its inclusion in a document. Integ tests would certainly cover
    this, but I want the unit tests to exercise everything too.
    
    Adding the missing tests and some fixes for issues that they revealed.
---
 stem/descriptor/networkstatus.py               |   16 ++++++-
 test/unit/descriptor/networkstatus/document.py |   57 ++++++++++++++++++++++-
 2 files changed, 68 insertions(+), 5 deletions(-)

diff --git a/stem/descriptor/networkstatus.py b/stem/descriptor/networkstatus.py
index 8774b95..2514a62 100644
--- a/stem/descriptor/networkstatus.py
+++ b/stem/descriptor/networkstatus.py
@@ -140,7 +140,7 @@ def parse_file(document_file, validate = True, is_microdescriptor = False):
   routers_end = document_file.tell()
   
   footer = document_file.readlines()
-  document_content = header + footer
+  document_content = "".join(header + footer)
   
   if not is_microdescriptor:
     document = NetworkStatusDocument(document_content, validate)
@@ -200,7 +200,7 @@ def _get_entries(document_file, validate, entry_class, entry_keyword, start_posi
   document_file.seek(start_position)
   while document_file.tell() < end_position:
     desc_content = "".join(_read_until_keywords(entry_keyword, document_file, ignore_first = True, end_position = end_position))
-    yield router_type(desc_content, validate, *extra_args)
+    yield entry_class(desc_content, validate, *extra_args)
 
 class NetworkStatusDocument(stem.descriptor.Descriptor):
   """
@@ -296,6 +296,12 @@ class NetworkStatusDocument(stem.descriptor.Descriptor):
   
   def get_unrecognized_lines(self):
     return list(self._unrecognized_lines)
+  
+  def __cmp__(self, other):
+    if not isinstance(other, NetworkStatusDocument):
+      return 1
+    
+    return str(self) > str(other)
 
 class _DocumentHeader(object):
   def __init__(self, document_file, validate, default_params):
@@ -994,6 +1000,12 @@ class RouterStatusEntry(stem.descriptor.Descriptor):
     """
     
     return list(self._unrecognized_lines)
+  
+  def __cmp__(self, other):
+    if not isinstance(other, RouterStatusEntry):
+      return 1
+    
+    return str(self) > str(other)
 
 class MicrodescriptorConsensus(NetworkStatusDocument):
   """
diff --git a/test/unit/descriptor/networkstatus/document.py b/test/unit/descriptor/networkstatus/document.py
index ad79dde..7570e8c 100644
--- a/test/unit/descriptor/networkstatus/document.py
+++ b/test/unit/descriptor/networkstatus/document.py
@@ -4,10 +4,12 @@ Unit tests for the NetworkStatusDocument of stem.descriptor.networkstatus.
 
 import datetime
 import unittest
+import StringIO
 
 import stem.version
 from stem.descriptor import Flag
-from stem.descriptor.networkstatus import HEADER_STATUS_DOCUMENT_FIELDS, FOOTER_STATUS_DOCUMENT_FIELDS, DEFAULT_PARAMS, BANDWIDTH_WEIGHT_ENTRIES, NetworkStatusDocument, DocumentSignature
+from stem.descriptor.networkstatus import HEADER_STATUS_DOCUMENT_FIELDS, FOOTER_STATUS_DOCUMENT_FIELDS, DEFAULT_PARAMS, BANDWIDTH_WEIGHT_ENTRIES, RouterStatusEntry, NetworkStatusDocument, DocumentSignature, parse_file
+from test.unit.descriptor.networkstatus.entry import get_router_status_entry
 
 sig_block = """\
 -----BEGIN SIGNATURE-----
@@ -33,7 +35,6 @@ NETWORK_STATUS_DOCUMENT_ATTR = {
   "directory-signature": "%s %s\n%s" % (SIG.identity, SIG.key_digest, SIG.signature),
 }
 
-
 def get_network_status_document(attr = None, exclude = None, routers = None):
   """
   Constructs a minimal network status document with the given attributes. This
@@ -89,7 +90,13 @@ def get_network_status_document(attr = None, exclude = None, routers = None):
     if attr_value: attr_value = " %s" % attr_value
     remainder.append(attr_keyword + attr_value)
   
-  return "\n".join(header_content + remainder + routers + footer_content)
+  # join the routers into a single block, then split it into lines
+  if routers:
+    router_lines = ("\n".join([str(r) for r in routers])).split("\n")
+  else:
+    router_lines = []
+  
+  return "\n".join(header_content + remainder + router_lines + footer_content)
 
 class TestNetworkStatusDocument(unittest.TestCase):
   def test_minimal_consensus(self):
@@ -156,6 +163,27 @@ class TestNetworkStatusDocument(unittest.TestCase):
     self.assertEqual([SIG], document.signatures)
     self.assertEqual([], document.get_unrecognized_lines())
   
+  def test_parse_file(self):
+    """
+    Try parsing a document via the parse_file() function.
+    """
+    
+    entry1 = RouterStatusEntry(get_router_status_entry({'s': "Fast"}))
+    entry2 = RouterStatusEntry(get_router_status_entry({'s': "Valid"}))
+    content = get_network_status_document(routers = (entry1, entry2))
+    
+    # the document that the entries refer to should actually be the minimal
+    # descriptor (ie, without the entries)
+    
+    expected_document = NetworkStatusDocument(get_network_status_document())
+    
+    descriptor_file = StringIO.StringIO(content)
+    entries = list(parse_file(descriptor_file))
+    
+    self.assertEquals(entry1, entries[0])
+    self.assertEquals(entry2, entries[1])
+    self.assertEquals(expected_document, entries[0].document)
+  
   def test_missing_fields(self):
     """
     Excludes mandatory fields from both a vote and consensus document.
@@ -680,4 +708,27 @@ class TestNetworkStatusDocument(unittest.TestCase):
         content = get_network_status_document({"directory-signature": "%s %s\n%s" % tuple(attrs)})
         self.assertRaises(ValueError, NetworkStatusDocument, content)
         NetworkStatusDocument(content, False) # checks that it's still parseable without validation
+  
+  def test_with_router_status_entries(self):
+    """
+    Includes a router status entry within the document. This isn't to test the
+    RouterStatusEntry parsing but rather the inclusion of it within the
+    document.
+    """
+    
+    entry1 = RouterStatusEntry(get_router_status_entry({'s': "Fast"}))
+    entry2 = RouterStatusEntry(get_router_status_entry({'s': "Valid"}))
+    content = get_network_status_document(routers = (entry1, entry2))
+    
+    document = NetworkStatusDocument(content)
+    self.assertEquals((entry1, entry2), document.routers)
+    
+    # try with an invalid RouterStatusEntry
+    
+    entry3 = RouterStatusEntry(get_router_status_entry({'r': "ugabuga"}), False)
+    content = get_network_status_document(routers = (entry3,))
+    
+    self.assertRaises(ValueError, NetworkStatusDocument, content)
+    document = NetworkStatusDocument(content, False)
+    self.assertEquals((entry3,), document.routers)