[tor-commits] [stem/master] Refactor link_protocol in test loops into test data (i.e. not hardcoded magic numbers)

atagar at torproject.org atagar at torproject.org
Thu Jul 12 18:53:10 UTC 2018


commit ccd2294c062adbdec599509c67328d69ff90870e
Author: Dave Rolek <dmr-x at riseup.net>
Date:   Mon Jul 9 19:47:52 2018 +0000

    Refactor link_protocol in test loops into test data (i.e. not hardcoded magic numbers)
    
    This removes most - but not all - use of magic numbers for link_protocol
    values.
---
 test/unit/client/cell.py | 114 +++++++++++++++++++++++------------------------
 1 file changed, 57 insertions(+), 57 deletions(-)

diff --git a/test/unit/client/cell.py b/test/unit/client/cell.py
index c7eb6f00..b706fcd6 100644
--- a/test/unit/client/cell.py
+++ b/test/unit/client/cell.py
@@ -30,28 +30,28 @@ RANDOM_PAYLOAD = os.urandom(FIXED_PAYLOAD_LEN)
 CHALLENGE = b'\x89Y\t\x99\xb2\x1e\xd9*V\xb6\x1bn\n\x05\xd8/\xe3QH\x85\x13Z\x17\xfc\x1c\x00{\xa9\xae\x83^K'
 
 PADDING_CELLS = {
-  b'\x00\x00\x00' + RANDOM_PAYLOAD: RANDOM_PAYLOAD,
+  b'\x00\x00\x00' + RANDOM_PAYLOAD: (RANDOM_PAYLOAD, 2),
 }
 
 RELAY_CELLS = {
-  b'\x00\x01\x03\r\x00\x00\x00\x01!\xa3?\xec' + ZERO * 500: ('RELAY_BEGIN_DIR', 13, 1, 1, b'', 564346860),
-  b'\x00\x01\x03\x02\x00\x00\x00\x01\x15:m\xe0\x00&GET /tor/server/authority HTTP/1.0\r\n\r\n' + ZERO * 460: ('RELAY_DATA', 2, 1, 1, b'GET /tor/server/authority HTTP/1.0\r\n\r\n', 356150752),
+  b'\x00\x01\x03\r\x00\x00\x00\x01!\xa3?\xec' + ZERO * 500: ('RELAY_BEGIN_DIR', 13, 1, 1, b'', 564346860, 2),
+  b'\x00\x01\x03\x02\x00\x00\x00\x01\x15:m\xe0\x00&GET /tor/server/authority HTTP/1.0\r\n\r\n' + ZERO * 460: ('RELAY_DATA', 2, 1, 1, b'GET /tor/server/authority HTTP/1.0\r\n\r\n', 356150752, 2),
 }
 
 DESTROY_CELLS = {
-  b'\x80\x00\x00\x00\x04\x00' + ZERO * 508: (2147483648, CloseReason.NONE, 0, ZERO * 508),
-  b'\x80\x00\x00\x00\x04\x03' + ZERO * 508: (2147483648, CloseReason.REQUESTED, 3, ZERO * 508),
-  b'\x80\x00\x00\x00\x04\x01' + b'\x01' + ZERO * 507: (2147483648, CloseReason.PROTOCOL, 1, b'\x01' + ZERO * 507),
+  b'\x80\x00\x00\x00\x04\x00' + ZERO * 508: (2147483648, CloseReason.NONE, 0, ZERO * 508, 5),
+  b'\x80\x00\x00\x00\x04\x03' + ZERO * 508: (2147483648, CloseReason.REQUESTED, 3, ZERO * 508, 5),
+  b'\x80\x00\x00\x00\x04\x01' + b'\x01' + ZERO * 507: (2147483648, CloseReason.PROTOCOL, 1, b'\x01' + ZERO * 507, 5),
 }
 
 CREATE_FAST_CELLS = {
-  (b'\x80\x00\x00\x00\x05\x92O\x0c\xcb\xa8\xac\xfb\xc9\x7f\xd0\rz\x1a\x03u\x91\xceas\xce' + ZERO * 489): (2147483648, b'\x92O\x0c\xcb\xa8\xac\xfb\xc9\x7f\xd0\rz\x1a\x03u\x91\xceas\xce'),
-  (b'\x80\x00\x00\x00\x05\x92O\x0c\xcb\xa8\xac\xfb\xc9\x7f\xd0\rz\x1a\x03u\x91\xceas\x00' + ZERO * 489): (2147483648, b'\x92O\x0c\xcb\xa8\xac\xfb\xc9\x7f\xd0\rz\x1a\x03u\x91\xceas\x00'),
+  (b'\x80\x00\x00\x00\x05\x92O\x0c\xcb\xa8\xac\xfb\xc9\x7f\xd0\rz\x1a\x03u\x91\xceas\xce' + ZERO * 489): (2147483648, b'\x92O\x0c\xcb\xa8\xac\xfb\xc9\x7f\xd0\rz\x1a\x03u\x91\xceas\xce', 5),
+  (b'\x80\x00\x00\x00\x05\x92O\x0c\xcb\xa8\xac\xfb\xc9\x7f\xd0\rz\x1a\x03u\x91\xceas\x00' + ZERO * 489): (2147483648, b'\x92O\x0c\xcb\xa8\xac\xfb\xc9\x7f\xd0\rz\x1a\x03u\x91\xceas\x00', 5),
 }
 
 CREATED_FAST_CELLS = {
-  (b'\x80\x00\x00\x00\x06\x92O\x0c\xcb\xa8\xac\xfb\xc9\x7f\xd0\rz\x1a\x03u\x91\xceas\xce\x13Z\x99\xb2\x1e\xb6\x05\x85\x17\xfc\x1c\x00{\xa9\xae\x83^K\x99\xb2' + ZERO * 469): (2147483648, b'\x92O\x0c\xcb\xa8\xac\xfb\xc9\x7f\xd0\rz\x1a\x03u\x91\xceas\xce', b'\x13Z\x99\xb2\x1e\xb6\x05\x85\x17\xfc\x1c\x00{\xa9\xae\x83^K\x99\xb2'),
-  (b'\x80\x00\x00\x00\x06\x92O\x0c\xcb\xa8\xac\xfb\xc9\x7f\xd0\rz\x1a\x03u\x91\xceas\xce\x13Z\x99\xb2\x1e\xb6\x05\x85\x17\xfc\x1c\x00{\xa9\xae\x83^K\x99\x00' + ZERO * 469): (2147483648, b'\x92O\x0c\xcb\xa8\xac\xfb\xc9\x7f\xd0\rz\x1a\x03u\x91\xceas\xce', b'\x13Z\x99\xb2\x1e\xb6\x05\x85\x17\xfc\x1c\x00{\xa9\xae\x83^K\x99\x00'),
+  (b'\x80\x00\x00\x00\x06\x92O\x0c\xcb\xa8\xac\xfb\xc9\x7f\xd0\rz\x1a\x03u\x91\xceas\xce\x13Z\x99\xb2\x1e\xb6\x05\x85\x17\xfc\x1c\x00{\xa9\xae\x83^K\x99\xb2' + ZERO * 469): (2147483648, b'\x92O\x0c\xcb\xa8\xac\xfb\xc9\x7f\xd0\rz\x1a\x03u\x91\xceas\xce', b'\x13Z\x99\xb2\x1e\xb6\x05\x85\x17\xfc\x1c\x00{\xa9\xae\x83^K\x99\xb2', 5),
+  (b'\x80\x00\x00\x00\x06\x92O\x0c\xcb\xa8\xac\xfb\xc9\x7f\xd0\rz\x1a\x03u\x91\xceas\xce\x13Z\x99\xb2\x1e\xb6\x05\x85\x17\xfc\x1c\x00{\xa9\xae\x83^K\x99\x00' + ZERO * 469): (2147483648, b'\x92O\x0c\xcb\xa8\xac\xfb\xc9\x7f\xd0\rz\x1a\x03u\x91\xceas\xce', b'\x13Z\x99\xb2\x1e\xb6\x05\x85\x17\xfc\x1c\x00{\xa9\xae\x83^K\x99\x00', 5),
 }
 
 VERSIONS_CELLS = {
@@ -62,26 +62,26 @@ VERSIONS_CELLS = {
 }
 
 NETINFO_CELLS = {
-  b'\x00\x00\x08ZZ\xb6\x90\x04\x04\x7f\x00\x00\x01\x01\x04\x04aq\x0f\x02' + ZERO * (FIXED_PAYLOAD_LEN - 17): (datetime.datetime(2018, 1, 14, 1, 46, 56), Address('127.0.0.1'), [Address('97.113.15.2')]),
+  b'\x00\x00\x08ZZ\xb6\x90\x04\x04\x7f\x00\x00\x01\x01\x04\x04aq\x0f\x02' + ZERO * (FIXED_PAYLOAD_LEN - 17): (datetime.datetime(2018, 1, 14, 1, 46, 56), Address('127.0.0.1'), [Address('97.113.15.2')], 2),
 }
 
 VPADDING_CELL_EMPTY_PACKED = b'\x00\x00\x80\x00\x00'
 
 VPADDING_CELLS = {
-  VPADDING_CELL_EMPTY_PACKED: b'',
-  b'\x00\x00\x80\x00\x01\x08': b'\x08',
-  b'\x00\x00\x80\x00\x02\x08\x11': b'\x08\x11',
-  b'\x00\x00\x80\x01\xfd' + RANDOM_PAYLOAD: RANDOM_PAYLOAD,
+  VPADDING_CELL_EMPTY_PACKED: (b'', 2),
+  b'\x00\x00\x80\x00\x01\x08': (b'\x08', 2),
+  b'\x00\x00\x80\x00\x02\x08\x11': (b'\x08\x11', 2),
+  b'\x00\x00\x80\x01\xfd' + RANDOM_PAYLOAD: (RANDOM_PAYLOAD, 2),
 }
 
 CERTS_CELLS = {
-  b'\x00\x00\x81\x00\x01\x00': [],
-  b'\x00\x00\x81\x00\x04\x01\x01\x00\x00': [Certificate(1, b'')],
-  b'\x00\x00\x81\x00\x05\x01\x01\x00\x01\x08': [Certificate(1, b'\x08')],
+  b'\x00\x00\x81\x00\x01\x00': ([], 2),
+  b'\x00\x00\x81\x00\x04\x01\x01\x00\x00': ([Certificate(1, b'')], 2),
+  b'\x00\x00\x81\x00\x05\x01\x01\x00\x01\x08': ([Certificate(1, b'\x08')], 2),
 }
 
 AUTH_CHALLENGE_CELLS = {
-  b'\x00\x00\x82\x00&' + CHALLENGE + b'\x00\x02\x00\x01\x00\x03': (CHALLENGE, [1, 3]),
+  b'\x00\x00\x82\x00&' + CHALLENGE + b'\x00\x02\x00\x01\x00\x03': (CHALLENGE, [1, 3], 2),
 }
 
 
@@ -175,20 +175,20 @@ class TestCell(unittest.TestCase):
     self.assertEqual(b'', content)  # check that we've consumed all of the bytes
 
   def test_padding_cell(self):
-    for cell_bytes, payload in PADDING_CELLS.items():
-      self.assertEqual(cell_bytes, PaddingCell(payload).pack(2))
+    for cell_bytes, (payload, link_protocol) in PADDING_CELLS.items():
+      self.assertEqual(cell_bytes, PaddingCell(payload).pack(link_protocol))
 
-      cell = Cell.pop(cell_bytes, 2)[0]
+      cell = Cell.pop(cell_bytes, link_protocol)[0]
       self.assertEqual(payload, cell.payload)
       self.assertEqual(b'', cell.unused)  # always empty
-      self.assertEqual(cell_bytes, cell.pack(2))
+      self.assertEqual(cell_bytes, cell.pack(link_protocol))
 
   def test_relay_cell(self):
-    for cell_bytes, (command, command_int, circ_id, stream_id, data, digest) in RELAY_CELLS.items():
-      self.assertEqual(cell_bytes, RelayCell(circ_id, command, data, digest, stream_id).pack(2))
-      self.assertEqual(cell_bytes, RelayCell(circ_id, command_int, data, digest, stream_id).pack(2))
+    for cell_bytes, (command, command_int, circ_id, stream_id, data, digest, link_protocol) in RELAY_CELLS.items():
+      self.assertEqual(cell_bytes, RelayCell(circ_id, command, data, digest, stream_id).pack(link_protocol))
+      self.assertEqual(cell_bytes, RelayCell(circ_id, command_int, data, digest, stream_id).pack(link_protocol))
 
-      cell = Cell.pop(cell_bytes, 2)[0]
+      cell = Cell.pop(cell_bytes, link_protocol)[0]
       self.assertEqual(circ_id, cell.circ_id)
       self.assertEqual(command, cell.command)
       self.assertEqual(command_int, cell.command_int)
@@ -196,7 +196,7 @@ class TestCell(unittest.TestCase):
       self.assertEqual(digest, cell.digest)
       self.assertEqual(stream_id, cell.stream_id)
       self.assertEqual(ZERO * (498 - len(cell.data)), cell.unused)
-      self.assertEqual(cell_bytes, cell.pack(2))
+      self.assertEqual(cell_bytes, cell.pack(link_protocol))
 
     digest = hashlib.sha1(b'hi')
     self.assertEqual(3257622417, RelayCell(5, 'RELAY_BEGIN_DIR', '', digest, 564346860).digest)
@@ -219,43 +219,43 @@ class TestCell(unittest.TestCase):
     self.assertRaisesRegexp(ValueError, '^%s$' % re.escape(expected_message), Cell.pop, mismatched_data_length_bytes, 2)
 
   def test_destroy_cell(self):
-    for cell_bytes, (circ_id, reason, reason_int, unused) in DESTROY_CELLS.items():
+    for cell_bytes, (circ_id, reason, reason_int, unused, link_protocol) in DESTROY_CELLS.items():
       # Packed cells always pad with zeros, so if we're testing something with
       # non-zero padding then skip this check.
 
       if not unused.strip(ZERO):
-        self.assertEqual(cell_bytes, DestroyCell(circ_id, reason).pack(5))
-        self.assertEqual(cell_bytes, DestroyCell(circ_id, reason_int).pack(5))
+        self.assertEqual(cell_bytes, DestroyCell(circ_id, reason).pack(link_protocol))
+        self.assertEqual(cell_bytes, DestroyCell(circ_id, reason_int).pack(link_protocol))
 
-      cell = Cell.pop(cell_bytes, 5)[0]
+      cell = Cell.pop(cell_bytes, link_protocol)[0]
       self.assertEqual(circ_id, cell.circ_id)
       self.assertEqual(reason, cell.reason)
       self.assertEqual(reason_int, cell.reason_int)
       self.assertEqual(unused, cell.unused)
-      self.assertEqual(cell_bytes, cell.pack(5))
+      self.assertEqual(cell_bytes, cell.pack(link_protocol))
 
   def test_create_fast_cell(self):
-    for cell_bytes, (circ_id, key_material) in CREATE_FAST_CELLS.items():
-      self.assertEqual(cell_bytes, CreateFastCell(circ_id, key_material).pack(5))
+    for cell_bytes, (circ_id, key_material, link_protocol) in CREATE_FAST_CELLS.items():
+      self.assertEqual(cell_bytes, CreateFastCell(circ_id, key_material).pack(link_protocol))
 
-      cell = Cell.pop(cell_bytes, 5)[0]
+      cell = Cell.pop(cell_bytes, link_protocol)[0]
       self.assertEqual(circ_id, cell.circ_id)
       self.assertEqual(key_material, cell.key_material)
       self.assertEqual(ZERO * 489, cell.unused)
-      self.assertEqual(cell_bytes, cell.pack(5))
+      self.assertEqual(cell_bytes, cell.pack(link_protocol))
 
     self.assertRaisesRegexp(ValueError, 'Key material should be 20 bytes, but was 3', CreateFastCell, 5, 'boo')
 
   def test_created_fast_cell(self):
-    for cell_bytes, (circ_id, key_material, derivative_key) in CREATED_FAST_CELLS.items():
-      self.assertEqual(cell_bytes, CreatedFastCell(circ_id, derivative_key, key_material).pack(5))
+    for cell_bytes, (circ_id, key_material, derivative_key, link_protocol) in CREATED_FAST_CELLS.items():
+      self.assertEqual(cell_bytes, CreatedFastCell(circ_id, derivative_key, key_material).pack(link_protocol))
 
-      cell = Cell.pop(cell_bytes, 5)[0]
+      cell = Cell.pop(cell_bytes, link_protocol)[0]
       self.assertEqual(circ_id, cell.circ_id)
       self.assertEqual(key_material, cell.key_material)
       self.assertEqual(derivative_key, cell.derivative_key)
       self.assertEqual(ZERO * 469, cell.unused)
-      self.assertEqual(cell_bytes, cell.pack(5))
+      self.assertEqual(cell_bytes, cell.pack(link_protocol))
 
     self.assertRaisesRegexp(ValueError, 'Key material should be 20 bytes, but was 3', CreateFastCell, 5, 'boo')
 
@@ -269,24 +269,24 @@ class TestCell(unittest.TestCase):
       self.assertEqual(cell_bytes, cell.pack(link_protocol))
 
   def test_netinfo_cell(self):
-    for cell_bytes, (timestamp, receiver_address, sender_addresses) in NETINFO_CELLS.items():
-      self.assertEqual(cell_bytes, NetinfoCell(receiver_address, sender_addresses, timestamp).pack(2))
+    for cell_bytes, (timestamp, receiver_address, sender_addresses, link_protocol) in NETINFO_CELLS.items():
+      self.assertEqual(cell_bytes, NetinfoCell(receiver_address, sender_addresses, timestamp).pack(link_protocol))
 
-      cell = Cell.pop(cell_bytes, 2)[0]
+      cell = Cell.pop(cell_bytes, link_protocol)[0]
       self.assertEqual(timestamp, cell.timestamp)
       self.assertEqual(receiver_address, cell.receiver_address)
       self.assertEqual(sender_addresses, cell.sender_addresses)
       self.assertEqual(ZERO * 492, cell.unused)
-      self.assertEqual(cell_bytes, cell.pack(2))
+      self.assertEqual(cell_bytes, cell.pack(link_protocol))
 
   def test_vpadding_cell(self):
-    for cell_bytes, payload in VPADDING_CELLS.items():
-      self.assertEqual(cell_bytes, VPaddingCell(payload = payload).pack(2))
+    for cell_bytes, (payload, link_protocol) in VPADDING_CELLS.items():
+      self.assertEqual(cell_bytes, VPaddingCell(payload = payload).pack(link_protocol))
 
-      cell = Cell.pop(cell_bytes, 2)[0]
+      cell = Cell.pop(cell_bytes, link_protocol)[0]
       self.assertEqual(payload, cell.payload)
       self.assertEqual(b'', cell.unused)  # always empty
-      self.assertEqual(cell_bytes, cell.pack(2))
+      self.assertEqual(cell_bytes, cell.pack(link_protocol))
 
     empty_constructed_cell = VPaddingCell(size = 0)
     self.assertEqual(VPADDING_CELL_EMPTY_PACKED, empty_constructed_cell.pack(2))
@@ -297,12 +297,12 @@ class TestCell(unittest.TestCase):
     self.assertRaisesRegexp(ValueError, re.escape('VPaddingCell constructor must specify payload or size'), VPaddingCell)
 
   def test_certs_cell(self):
-    for cell_bytes, certs in CERTS_CELLS.items():
-      self.assertEqual(cell_bytes, CertsCell(certs).pack(2))
+    for cell_bytes, (certs, link_protocol) in CERTS_CELLS.items():
+      self.assertEqual(cell_bytes, CertsCell(certs).pack(link_protocol))
 
-      cell = Cell.pop(cell_bytes, 2)[0]
+      cell = Cell.pop(cell_bytes, link_protocol)[0]
       self.assertEqual(certs, cell.certificates)
-      self.assertEqual(cell_bytes, cell.pack(2))
+      self.assertEqual(cell_bytes, cell.pack(link_protocol))
 
     # extra bytes after the last certificate should be ignored
 
@@ -314,14 +314,14 @@ class TestCell(unittest.TestCase):
     self.assertRaisesRegexp(ValueError, 'CERTS cell indicates it should have 2 certificates, but only contained 1', Cell.pop, b'\x00\x00\x81\x00\x05\x02\x01\x00\x01\x08', 2)
 
   def test_auth_challenge_cell(self):
-    for cell_bytes, (challenge, methods) in AUTH_CHALLENGE_CELLS.items():
-      self.assertEqual(cell_bytes, AuthChallengeCell(methods, challenge).pack(2))
+    for cell_bytes, (challenge, methods, link_protocol) in AUTH_CHALLENGE_CELLS.items():
+      self.assertEqual(cell_bytes, AuthChallengeCell(methods, challenge).pack(link_protocol))
 
-      cell = Cell.pop(cell_bytes, 2)[0]
+      cell = Cell.pop(cell_bytes, link_protocol)[0]
       self.assertEqual(challenge, cell.challenge)
       self.assertEqual(methods, cell.methods)
       self.assertEqual(b'', cell.unused)
-      self.assertEqual(cell_bytes, cell.pack(2))
+      self.assertEqual(cell_bytes, cell.pack(link_protocol))
 
     self.assertRaisesRegexp(ValueError, 'AUTH_CHALLENGE cell should have a payload of 38 bytes, but only had 16', Cell.pop, b'\x00\x00\x82\x00&' + CHALLENGE[:10] + b'\x00\x02\x00\x01\x00\x03', 2)
     self.assertRaisesRegexp(ValueError, 'AUTH_CHALLENGE should have 3 methods, but only had 4 bytes for it', Cell.pop, b'\x00\x00\x82\x00&' + CHALLENGE + b'\x00\x03\x00\x01\x00\x03', 2)





More information about the tor-commits mailing list