Repository: OpenAssetIO/OpenAssetIO-Manager-BAL (language: python)

Issue:
`BAL_LIBRARY_PATH` ignored 
## What

Since we implemented the settings mechanism in BAL, we accidentally cause init to ignore the `BAL_LIBRARY_PATH` env var.

## Expected behaviour

When BAL is initialized, if the library path is set to an empty string, then it should follow the same initialization logic as if it was not set at all.




Discussion / hints:

Fix patch:
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
index 5846663..402161e 100644
--- a/RELEASE_NOTES.md
+++ b/RELEASE_NOTES.md
@@ -1,6 +1,16 @@
 Release Notes
 =============
 
+v1.0.0-alpha.x
+--------------
+
+### Bug fixes
+
+- The `$BAL_LIBRARY_PATH` environment variable is now correctly
+  considered when the `library_path` setting is not set/empty.
+  [#26](https://github.com/OpenAssetIO/OpenAssetIO-Manager-BAL/issues/26)
+
+
 v1.0.0-alpha.5
 --------------
 
diff --git a/plugin/openassetio_manager_bal/BasicAssetLibraryInterface.py b/plugin/openassetio_manager_bal/BasicAssetLibraryInterface.py
index ada69bc..1fd7beb 100644
--- a/plugin/openassetio_manager_bal/BasicAssetLibraryInterface.py
+++ b/plugin/openassetio_manager_bal/BasicAssetLibraryInterface.py
@@ -72,28 +72,34 @@ def settings(self, hostSession):
 
     def initialize(self, managerSettings, hostSession):
         bal.validate_settings(managerSettings)
-        self.__settings.update(managerSettings)
 
-        self.__library = {}
+        # Settings updates can be partial, so make sure we keep any
+        # existing path.
+        existing_library_path = self.__settings.get("library_path")
+        library_path = managerSettings.get("library_path", existing_library_path)
 
-        if self.__settings.get("library_path") is None:
+        if not library_path:
             hostSession.logger().log(
                 hostSession.logger().Severity.kDebug,
-                f"'library_path' not in settings, checking {self.__lib_path_envvar_name}",
+                "'library_path' not in settings or is empty, checking "
+                f"{self.__lib_path_envvar_name}",
             )
-            self.__settings["library_path"] = os.environ.get(self.__lib_path_envvar_name)
+            library_path = os.environ.get(self.__lib_path_envvar_name)
+
+        if not library_path:
+            raise PluginError(f"'library_path'/{self.__lib_path_envvar_name} not set or is empty")
 
-        if self.__settings.get("library_path") is None:
-            raise PluginError("'library_path' not set")
+        self.__settings.update(managerSettings)
+        self.__settings["library_path"] = library_path
 
+        self.__library = {}
         hostSession.logger().log(
             hostSession.logger().Severity.kDebug,
-            f"Loading library from {self.__settings['library_path']}",
+            f"Loading library from '{library_path}'",
         )
-        self.__library = bal.load_library(self.__settings["library_path"])
+        self.__library = bal.load_library(library_path)
 
     def managementPolicy(self, traitSets, context, hostSession):
-
         access = "read" if context.isForRead() else "write"
         return [
             self.__dict_to_traits_data(bal.management_policy(trait_set, access, self.__library))
diff --git a/pyproject.toml b/pyproject.toml
index 7b2fc90..e051331 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -6,7 +6,7 @@ name = "openassetio-manager-bal"
 version = "1.0.0a5"
 requires-python = ">=3.7"
 dependencies = [
-    "openassetio == 1.0.0a8"
+    "openassetio == 1.0.0a9"
 ]
 
 authors = [


Test patch:
diff --git a/tests/bal_business_logic_suite.py b/tests/bal_business_logic_suite.py
index 61bc7e3..e46b199 100644
--- a/tests/bal_business_logic_suite.py
+++ b/tests/bal_business_logic_suite.py
@@ -23,15 +23,19 @@
 import operator
 import os
 
+from unittest import mock
+
 from openassetio import Context, TraitsData
+from openassetio.exceptions import PluginError
 from openassetio.test.manager.harness import FixtureAugmentedTestCase
 
-from unittest import mock
-
 
 __all__ = []
 
 
+LIBRARY_PATH_VARNAME = "BAL_LIBRARY_PATH"
+
+
 class LibraryOverrideTestCase(FixtureAugmentedTestCase):
     """
     Utility base class to load a custom BAL library for the duration of
@@ -58,6 +62,50 @@ def cleanUp(self):
         self._manager.initialize(self.__old_settings)
 
 
+class Test_initialize_library_path(FixtureAugmentedTestCase):
+    shareManager = False
+
+    alt_lib_path = os.path.join(os.path.dirname(__file__), "resources", "library_empty.json")
+
+    @mock.patch.dict(os.environ)
+    def test_when_setting_and_env_not_set_then_PluginError_raised(self):
+        if LIBRARY_PATH_VARNAME in os.environ:
+            del os.environ[LIBRARY_PATH_VARNAME]
+        with self.assertRaises(PluginError):
+            self._manager.initialize({})
+
+    @mock.patch.dict(os.environ)
+    def test_when_setting_not_set_and_env_set_then_env_used(self):
+        os.environ[LIBRARY_PATH_VARNAME] = self.alt_lib_path
+        self._manager.initialize({})
+        self.assertEqual(self._manager.settings()["library_path"], self.alt_lib_path)
+
+    @mock.patch.dict(os.environ)
+    def test_when_setting_set_and_env_not_set_then_setting_used(self):
+        if LIBRARY_PATH_VARNAME in os.environ:
+            del os.environ[LIBRARY_PATH_VARNAME]
+        self._manager.initialize({"library_path": self.alt_lib_path})
+        self.assertEqual(self._manager.settings()["library_path"], self.alt_lib_path)
+
+    @mock.patch.dict(os.environ)
+    def test_when_setting_and_env_set_then_setting_used(self):
+        os.environ[LIBRARY_PATH_VARNAME] = "I do not exist"
+        self._manager.initialize({"library_path": self.alt_lib_path})
+        self.assertEqual(self._manager.settings()["library_path"], self.alt_lib_path)
+
+    @mock.patch.dict(os.environ)
+    def test_when_setting_and_env_blank_then_PluginError_raised(self):
+        os.environ[LIBRARY_PATH_VARNAME] = ""
+        with self.assertRaises(PluginError):
+            self._manager.initialize({"library_path": ""})
+
+    @mock.patch.dict(os.environ)
+    def test_when_setting_blank_and_env_set_then_env_used(self):
+        os.environ[LIBRARY_PATH_VARNAME] = self.alt_lib_path
+        self._manager.initialize({"library_path": ""})
+        self.assertEqual(self._manager.settings()["library_path"], self.alt_lib_path)
+
+
 class Test_managementPolicy_missing_completely(LibraryOverrideTestCase):
     """
     Tests error case when  BAL library managementPolicy is missing.
@@ -219,7 +267,6 @@ def error_cb(idx, batchElementError):
 
 
 class Test_resolve_trait_property_expansion(LibraryOverrideTestCase):
-
     _library = "library_business_logic_suite_var_expansion.json"
 
     @mock.patch.dict(os.environ, {"CUSTOM": "the value from a custom"})
@@ -391,7 +438,6 @@ def __create_test_entity(self, ref, data, context):
 
 
 class Test_getRelatedRefrences(LibraryOverrideTestCase):
-
     # @TODO(tc) Switch to fixtures once covered by the OpenAssetIO
     # apiComplianceSuite as this should really be tested there. This
     # will be added when the C++ port happens along with the switch to
diff --git a/tests/resources/library_empty.json b/tests/resources/library_empty.json
new file mode 100644
index 0000000..0967ef4
--- /dev/null
+++ b/tests/resources/library_empty.json
@@ -0,0 +1,1 @@
+{}