BinaryData: raise instead of discarding data

Before, packed BinaryData would be empty if given a non-byte value.
E.g.: BinaryData('string').pack() == b''. Now, we raise a ValueError.

Author: cemsbr

pyof/foundation/basic_types.py

@@ -411,8 +411,13 @@ def __init__(self, value=b''):  # noqa
         """The constructor takes the parameter below.
 
         Args:
-            value (bytes): The binary data. Defaults to an empty value.
+            value (bytes, str): The binary data. Defaults to an empty value.
+
+        Raises:
+            ValueError: If given value is not bytes.
         """
+        if not isinstance(value, bytes):
+            raise ValueError('BinaryData must contain bytes.')
         super().__init__(value)
 
     def pack(self, value=None):
@@ -430,8 +435,10 @@ def pack(self, value=None):
         if value is None:
             value = self._value
 
-        if isinstance(value, bytes) and value:
-            return value
+        if value:
+            if isinstance(value, bytes):
+                return value
+            raise ValueError('BinaryData must contain bytes.')
 
         return b''
 

tests/test_foundation/test_basic_types.py

@@ -2,6 +2,7 @@
 import unittest
 
 from pyof.foundation import basic_types
+from pyof.foundation.basic_types import BinaryData
 
 
 class TestUBInt8(unittest.TestCase):
@@ -160,3 +161,33 @@ def test_get_size(self):
         """Testing get_size from IPAddress."""
         ip_addr = basic_types.IPAddress('192.168.0.1/24')
         self.assertEqual(ip_addr.get_size(), 4)
+
+
+class TestBinaryData(unittest.TestCase):
+    """Test Binary data type."""
+
+    def test_default_value(self):
+        """Default packed value should be an empty byte."""
+        expected = b''
+        actual = BinaryData().pack()
+        self.assertEqual(expected, actual)
+
+    def test_pack_bytes(self):
+        """Test packing some bytes."""
+        expected = b'forty two'
+        actual = BinaryData(expected).pack()
+        self.assertEqual(expected, actual)
+
+    def test_pack_empty_bytes(self):
+        """Test packing empty bytes."""
+        expected = b''
+        actual = BinaryData(expected).pack()
+        self.assertEqual(expected, actual)
+
+    def test_unexpected_value(self):
+        """Should raise ValueError if constructor value is not bytes."""
+        self.assertRaises(ValueError, BinaryData, "can't be string")
+
+    def test_unexpected_value_as_parameter(self):
+        """Should raise ValueError if pack value is not bytes."""
+        self.assertRaises(ValueError, BinaryData().pack, "can't be string")

tests/v0x01/test_asynchronous/test_error_msg.py

@@ -17,5 +17,5 @@ def setUpClass(cls):
         super().set_raw_dump_object(ErrorMsg, xid=12,
                                     error_type=ErrorType.OFPET_BAD_REQUEST,
                                     code=BadRequestCode.OFPBRC_BAD_STAT,
-                                    data=BinaryData('object_test'))
+                                    data=BinaryData(b'object_test'))
         super().set_minimum_size(12)