[tor-commits] [stem/master] Use base Cell class for test_unimplemented_cell_methods

atagar at torproject.org atagar at torproject.org
Sun Jun 17 00:23:10 UTC 2018


commit 7711050619af1a2f8ecf4aa87f774baa5f367b3b
Author: Damian Johnson <atagar at torproject.org>
Date:   Sat Jun 16 16:49:21 2018 -0700

    Use base Cell class for test_unimplemented_cell_methods
    
    No need to subclass. Making a child doesn't hurt, but it also doesn't provide
    anything over just instantiating our abstract class.
    
    Also dropping the '^%s$' matching from our regex. This isn't wrong at all - in
    fact, it's a bit better. But it's also something applicable to ~80% of the
    raise assertions we make. Probably a quirk of mine, but kinda like to avoid the
    extra verbosity.
    
    It's kinda questionable if we should be making these assertions at all. The
    following makes a case against it...
    
      https://stackoverflow.com/questions/8672754/how-to-show-the-error-messages-caught-by-assertraises-in-unittest-in-python2-7/8673096#8673096
    
    Maybe that's why the unittest module doesn't provide it.
---
 test/unit/client/cell.py | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/test/unit/client/cell.py b/test/unit/client/cell.py
index d36500cc..3d054757 100644
--- a/test/unit/client/cell.py
+++ b/test/unit/client/cell.py
@@ -103,16 +103,10 @@ class TestCell(unittest.TestCase):
     self.assertRaises(ValueError, Cell.by_value, None)
 
   def test_unimplemented_cell_methods(self):
-    class UnimplementedCell(Cell):
-      NAME = 'UNIMPLEMENTED'
+    cell_instance = Cell()
 
-    instance = UnimplementedCell()
-
-    self.assertRaisesRegexp(NotImplementedError, '^%s$' % re.escape('Packing not yet implemented for UNIMPLEMENTED cells'), instance.pack, 2)
-
-    # ideally we'd test unpacking with Cell.pop(), but that would mean monkey-patching for Cell.pop() to see this UnimplementedCell
-    # that's probably not worth it; instead we'll use the instance method _unpack()
-    self.assertRaisesRegexp(NotImplementedError, '^%s$' % re.escape('Unpacking not yet implemented for UNIMPLEMENTED cells'), instance._unpack, b'dummy', 0, 2)
+    self.assertRaisesRegexp(NotImplementedError, re.escape('Packing not yet implemented for UNKNOWN cells'), cell_instance.pack, 2)
+    self.assertRaisesRegexp(NotImplementedError, re.escape('Unpacking not yet implemented for UNKNOWN cells'), cell_instance._unpack, b'dummy', 0, 2)
 
   def test_payload_too_large(self):
     class OversizedCell(Cell):
@@ -126,7 +120,7 @@ class TestCell(unittest.TestCase):
     instance = OversizedCell()
 
     expected_message = 'Cell of type OVERSIZED is too large (%i bytes), must not be more than %i. Check payload size (was %i bytes)' % (FIXED_PAYLOAD_LEN + 4, FIXED_PAYLOAD_LEN + 3, FIXED_PAYLOAD_LEN + 1)
-    self.assertRaisesRegexp(ValueError, '^%s$' % re.escape(expected_message), instance.pack, 2)
+    self.assertRaisesRegexp(ValueError, re.escape(expected_message), instance.pack, 2)
 
   def test_unpack_for_new_link(self):
     expected_certs = (
@@ -245,7 +239,7 @@ class TestCell(unittest.TestCase):
 
     self.assertRaisesRegexp(ValueError, 'VPaddingCell constructor specified both a size of 5 bytes and payload of 1 bytes', VPaddingCell, 5, '\x02')
     self.assertRaisesRegexp(ValueError, re.escape('VPaddingCell size (-15) cannot be negative'), VPaddingCell, -15)
-    self.assertRaisesRegexp(ValueError, '^%s$' % re.escape('VPaddingCell constructor must specify payload or size'), VPaddingCell)
+    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():





More information about the tor-commits mailing list