Merge pull request #369 from cemsbr/binarydata

BinaryData: raise instead of discarding data

Author: beraldoleal

CHANGELOG.rst

@@ -3,14 +3,15 @@ Change log
 ##########
 All notable changes to the python-openflow project is documented in this file.
 
-[UNRELEASED] - Under development
-********************************
+UNRELEASED - [2017.1b3] - "bethania" beta3 - 2017-06-1X
+*******************************************************
 Added
 =====
 - IPv4 packet pack/unpack support.
 
 Changed
 =======
+- Raise ValueError if not using bytes (e.g. string) in BinaryData.
 
 Deprecated
 ==========

pyof/foundation/basic_types.py

@@ -412,7 +412,12 @@ def __init__(self, value=b''):  # noqa
 
         Args:
             value (bytes): 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

@@ -16,7 +16,6 @@ def setUpClass(cls):
                                     error_type=ErrorType.OFPET_BAD_REQUEST,
                                     code=BadRequestCode.OFPBRC_BAD_STAT,
                                     data=b'')
-
         super().set_minimum_size(12)
 
     def test_unpack_error_msg(self):