Merge pull request #5 from godaddy/bugfixes

Fix load_library and str_to_buf

Author: jgowdy-godaddy

.gitignore

@@ -127,3 +127,8 @@ dmypy.json
 
 # Pyre type checker
 .pyre/
+
+# Editors
+.vscode/
+.idea/
+*.swp

CHANGELOG.md

@@ -1,8 +1,14 @@
+## [0.2.1] - 2022-03-04
+
+- Added `minimum_allocation` and `header_size` properties to the `Cobhan` class
+- Fixed `load_library` and added tests
+- Fixed allocation of empty buffers in `str_to_buf` and added tests
+
 ## [0.2.0] - 2022-03-03
 
-- Rename the `_load_*` methods to `load_*` to make them part of the public API,
+- Renamed the `_load_*` methods to `load_*` to make them part of the public API,
   rather than protected.
-- Add the `int_to_buf` and `buf_to_int` methods.
+- Added the `int_to_buf` and `buf_to_int` methods.
 
 ## [0.1.0] - 2022-02-28
 

cobhan/cobhan.py

@@ -27,6 +27,16 @@ def __init__(self):
             self.__sizeof_int32, byteorder="little", signed=True
         )
 
+    @property
+    def minimum_allocation(self):
+        """The minimum buffer size, in bytes, that will be allocated for a string"""
+        return self.__minimum_allocation
+
+    @property
+    def header_size(self):
+        """The size, in bytes, of a buffer's header"""
+        return self.__sizeof_header
+
     def load_library(self, library_path: str, library_name: str, cdefines: str) -> None:
         """Locate and load a library based on the current platform.
 
@@ -47,7 +57,7 @@ def load_library(self, library_path: str, library_name: str, cdefines: str) -> N
                 os_ext = "-musl.so"
                 need_chdir = True
             else:
-                os_path = ".so"
+                os_ext = ".so"
         elif system == "Darwin":
             os_ext = ".dylib"
         elif system == "Windows":
@@ -64,9 +74,7 @@ def load_library(self, library_path: str, library_name: str, cdefines: str) -> N
             raise UnsupportedOperation("Unsupported CPU")
 
         # Get absolute library path
-        resolved_library_path = pathlib.Path(
-            os.path.join(library_path, os_path, arch_part)
-        ).resolve()
+        resolved_library_path = pathlib.Path(library_path).resolve()
 
         # Build library path with file name
         library_file_path = os.path.join(
@@ -154,12 +162,14 @@ def bytearray_to_buf(self, payload: ByteString) -> CBuf:
         self.__set_payload(buf, payload, length)
         return buf
 
-    def str_to_buf(self, string: str) -> CBuf:
+    def str_to_buf(self, string: Optional[str]) -> CBuf:
         """Encode a string in utf8 and copy into a Cobhan buffer.
 
         :param string: The string to be copied
         :returns: A new Cobhan buffer containing the utf8 encoded string
         """
+        if not string:
+            return self.__ffi.new(f"char[{self.header_size}]")
         encoded_bytes = string.encode("utf8")
         length = len(encoded_bytes)
         buf = self.allocate_buf(length)

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "cobhan"
-version = "0.2.0"
+version = "0.2.1"
 description = "Cobhan FFI"
 authors = [
     "Jeremiah Gowdy <[email protected]>",

tests/test_cobhan.py

@@ -1,11 +1,109 @@
-"""Tests for the main Cobhan module"""
+# pylint: disable=missing-function-docstring,missing-class-docstring,missing-module-docstring
 
-from unittest import TestCase
+from pathlib import Path
+from unittest import mock, TestCase
 
+from cobhan import Cobhan
 
-class CobhanTest(TestCase):
-    """Tests for the main Cobhan class"""
 
-    def test_pass(self):
-        """Fake test to start off"""
-        self.assertEqual(True, True)
+class LoadLibraryTests(TestCase):
+    """Tests for Cobhan.load_library"""
+
+    def setUp(self) -> None:
+        self.ffi_patcher = mock.patch("cobhan.cobhan.FFI")
+        self.platform_patcher = mock.patch("cobhan.cobhan.platform")
+
+        self.mock_ffi = self.ffi_patcher.start()
+        self.mock_dlopen = self.mock_ffi.return_value.dlopen
+        self.mock_platform = self.platform_patcher.start()
+
+        self.addCleanup(self.ffi_patcher.stop)
+        self.addCleanup(self.platform_patcher.stop)
+
+        self.cobhan = Cobhan()
+        return super().setUp()
+
+    def test_load_linux_x64(self):
+        self.mock_platform.system.return_value = "Linux"
+        self.mock_platform.machine.return_value = "x86_64"
+
+        self.cobhan.load_library("libfoo", "libbar", "")
+        self.mock_dlopen.assert_called_once_with(
+            str(Path("libfoo/libbar-x64.so").resolve())
+        )
+
+    def test_load_linux_arm64(self):
+        self.mock_platform.system.return_value = "Linux"
+        self.mock_platform.machine.return_value = "arm64"
+
+        self.cobhan.load_library("libfoo", "libbar", "")
+        self.mock_dlopen.assert_called_once_with(
+            str(Path("libfoo/libbar-arm64.so").resolve())
+        )
+
+    def test_load_macos_x64(self):
+        self.mock_platform.system.return_value = "Darwin"
+        self.mock_platform.machine.return_value = "x86_64"
+
+        self.cobhan.load_library("libfoo", "libbar", "")
+        self.mock_dlopen.assert_called_once_with(
+            str(Path("libfoo/libbar-x64.dylib").resolve())
+        )
+
+    def test_load_macos_arm64(self):
+        self.mock_platform.system.return_value = "Darwin"
+        self.mock_platform.machine.return_value = "arm64"
+
+        self.cobhan.load_library("libfoo", "libbar", "")
+        self.mock_dlopen.assert_called_once_with(
+            str(Path("libfoo/libbar-arm64.dylib").resolve())
+        )
+
+    def test_load_windows_x64(self):
+        self.mock_platform.system.return_value = "Windows"
+        self.mock_platform.machine.return_value = "x86_64"
+
+        self.cobhan.load_library("libfoo", "libbar", "")
+        self.mock_dlopen.assert_called_once_with(
+            str(Path("libfoo/libbar-x64.dll").resolve())
+        )
+
+    def test_load_windows_arm64(self):
+        self.mock_platform.system.return_value = "Windows"
+        self.mock_platform.machine.return_value = "arm64"
+
+        self.cobhan.load_library("libfoo", "libbar", "")
+        self.mock_dlopen.assert_called_once_with(
+            str(Path("libfoo/libbar-arm64.dll").resolve())
+        )
+
+
+class StringTests(TestCase):
+    def setUp(self) -> None:
+
+        self.cobhan = Cobhan()
+        return super().setUp()
+
+    def test_minimum_allocation_is_enforced(self):
+        buf = self.cobhan.str_to_buf("foo")
+        self.assertEqual(
+            len(buf), (self.cobhan.minimum_allocation + self.cobhan.header_size)
+        )
+
+    def test_can_allocate_beyond_minimum(self):
+        long_str = "foobar" * 1000  # This will be 6k characters in length
+        buf = self.cobhan.str_to_buf(long_str)
+        self.assertEqual(len(buf), (len(long_str) + self.cobhan.header_size))
+
+    def test_two_way_conversion_maintains_string(self):
+        buf = self.cobhan.str_to_buf("foobar")
+        result = self.cobhan.buf_to_str(buf)
+        self.assertEqual(result, "foobar")
+
+    def test_empty_string_returns_empty_buffer(self):
+        buf = self.cobhan.str_to_buf("")
+        self.assertEqual(len(buf), self.cobhan.header_size)
+
+    def test_input_of_none_returns_empty_buffer(self):
+        buf = self.cobhan.str_to_buf(None)
+        self.assertEqual(len(buf), self.cobhan.header_size)