commit 7711050619af1a2f8ecf4aa87f774baa5f367b3b Author: Damian Johnson <atagar@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-c... 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():