From d7e803152eb6f7ffb07d4ebafc7aa44e14d434e8 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Thu, 17 May 2018 14:51:22 +0100
Subject: [PATCH] Fixed ui hooks factory

---
 force_bdss/ui_hooks/base_ui_hooks_factory.py  | 20 ++++--
 .../tests/test_base_ui_hooks_factory.py       | 68 ++++++++++++-------
 .../tests/test_base_ui_hooks_manager.py       |  5 +-
 3 files changed, 60 insertions(+), 33 deletions(-)

diff --git a/force_bdss/ui_hooks/base_ui_hooks_factory.py b/force_bdss/ui_hooks/base_ui_hooks_factory.py
index 111a858..b9bc960 100644
--- a/force_bdss/ui_hooks/base_ui_hooks_factory.py
+++ b/force_bdss/ui_hooks/base_ui_hooks_factory.py
@@ -1,5 +1,5 @@
 import logging
-from traits.api import ABCHasStrictTraits, Instance, String, provides, Type
+from traits.api import ABCHasStrictTraits, Instance, Str, provides, Type
 from envisage.plugin import Plugin
 
 from force_bdss.ids import factory_id
@@ -16,14 +16,14 @@ class BaseUIHooksFactory(ABCHasStrictTraits):
     moments of the UI lifetime.
     """
     #: identifier of the factory
-    id = String()
+    id = Str()
 
     #: Name of the factory. User friendly for UI
-    name = String()
+    name = Str()
 
     #: The UI Hooks manager class to instantiate. Define this to your
     #: base hook managers.
-    ui_hooks_manager_class = Type(BaseUIHooksManager)
+    ui_hooks_manager_class = Type(BaseUIHooksManager, allow_none=False)
 
     #: A reference to the containing plugin
     plugin = Instance(Plugin)
@@ -42,7 +42,17 @@ class BaseUIHooksFactory(ABCHasStrictTraits):
         self.ui_hooks_manager_class = self.get_ui_hooks_manager_class()
         self.name = self.get_name()
         identifier = self.get_identifier()
-        self.id = factory_id(self.plugin.id, identifier)
+        try:
+            id = factory_id(self.plugin.id, identifier)
+        except ValueError:
+            raise ValueError(
+                "Invalid identifier {} returned by "
+                "{}.get_identifier()".format(
+                    identifier,
+                    self.__class__.__name__
+                )
+            )
+        self.id = id
 
     def get_ui_hooks_manager_class(self):
         raise NotImplementedError(
diff --git a/force_bdss/ui_hooks/tests/test_base_ui_hooks_factory.py b/force_bdss/ui_hooks/tests/test_base_ui_hooks_factory.py
index dd65ce8..9ee721d 100644
--- a/force_bdss/ui_hooks/tests/test_base_ui_hooks_factory.py
+++ b/force_bdss/ui_hooks/tests/test_base_ui_hooks_factory.py
@@ -1,7 +1,8 @@
 import unittest
 
-import testfixtures
+from traits.trait_errors import TraitError
 
+from force_bdss.ui_hooks.base_ui_hooks_factory import BaseUIHooksFactory
 from force_bdss.ui_hooks.tests.test_base_ui_hooks_manager import \
     DummyUIHooksManager
 
@@ -11,37 +12,52 @@ except ImportError:
     from unittest import mock
 
 from envisage.api import Plugin
-from ..base_ui_hooks_factory import BaseUIHooksFactory
 
 
 class DummyUIHooksFactory(BaseUIHooksFactory):
-    def create_ui_hooks_manager(self):
-        return DummyUIHooksManager(self)
+    def get_identifier(self):
+        return "foo"
 
+    def get_name(self):
+        return "bar"
 
-class DummyUIHooksFactoryFast(BaseUIHooksFactory):
-    ui_hooks_manager_class = DummyUIHooksManager
+    def get_ui_hooks_manager_class(self):
+        return DummyUIHooksManager
 
 
 class TestBaseUIHooksFactory(unittest.TestCase):
+    def setUp(self):
+        self.plugin = mock.Mock(spec=Plugin, id="pid")
+
     def test_initialize(self):
-        mock_plugin = mock.Mock(spec=Plugin)
-        factory = DummyUIHooksFactory(plugin=mock_plugin)
-        self.assertEqual(factory.plugin, mock_plugin)
-
-    def test_fast_definition(self):
-        mock_plugin = mock.Mock(spec=Plugin)
-        factory = DummyUIHooksFactoryFast(plugin=mock_plugin)
-
-        self.assertIsInstance(
-            factory.create_ui_hooks_manager(),
-            DummyUIHooksManager)
-
-    def test_fast_definition_errors(self):
-        mock_plugin = mock.Mock(spec=Plugin)
-        factory = DummyUIHooksFactoryFast(plugin=mock_plugin)
-        factory.ui_hooks_manager_class = None
-
-        with testfixtures.LogCapture():
-            with self.assertRaises(RuntimeError):
-                factory.create_ui_hooks_manager()
+        factory = DummyUIHooksFactory(plugin=self.plugin)
+        self.assertEqual(factory.plugin, self.plugin)
+        self.assertEqual(factory.id, "pid.factory.foo")
+        self.assertEqual(factory.name, "bar")
+        self.assertEqual(factory.ui_hooks_manager_class, DummyUIHooksManager)
+        self.assertIsInstance(factory.create_ui_hooks_manager(),
+                              DummyUIHooksManager)
+
+    def test_broken_get_identifier(self):
+        class Broken(DummyUIHooksFactory):
+            def get_identifier(self):
+                return None
+
+        with self.assertRaises(ValueError):
+            Broken(self.plugin)
+
+    def test_broken_get_name(self):
+        class Broken(DummyUIHooksFactory):
+            def get_name(self):
+                return None
+
+        with self.assertRaises(TraitError):
+            Broken(self.plugin)
+
+    def test_broken_get_ui_hooks_manager_class(self):
+        class Broken(DummyUIHooksFactory):
+            def get_ui_hooks_manager_class(self):
+                return None
+
+        with self.assertRaises(TraitError):
+            Broken(self.plugin)
diff --git a/force_bdss/ui_hooks/tests/test_base_ui_hooks_manager.py b/force_bdss/ui_hooks/tests/test_base_ui_hooks_manager.py
index 5e3afd4..f1b53dc 100644
--- a/force_bdss/ui_hooks/tests/test_base_ui_hooks_manager.py
+++ b/force_bdss/ui_hooks/tests/test_base_ui_hooks_manager.py
@@ -1,7 +1,8 @@
 import unittest
 
-from ..base_ui_hooks_manager import BaseUIHooksManager
-from ..base_ui_hooks_factory import BaseUIHooksFactory
+from force_bdss.ui_hooks.base_ui_hooks_factory import BaseUIHooksFactory
+from force_bdss.ui_hooks.base_ui_hooks_manager import BaseUIHooksManager
+
 try:
     import mock
 except ImportError:
-- 
GitLab