commit 72c561b9adb7e3963aa14da98b8d9bc77df533dd Author: Damian Johnson atagar@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)