From 3875f9b17855b1435f298b7716fbe65b62d2fb1f Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Fri, 6 Nov 2020 14:40:04 +0000 Subject: [PATCH 1/9] Add test that checks the keys in the saved preferences --- .../tests/test_preferences_helper.py | 41 +++++++++++-- apptools/preferences/ui/tests/__init__.py | 0 .../ui/tests/test_preferences_page.py | 58 +++++++++++++++++++ 3 files changed, 95 insertions(+), 4 deletions(-) create mode 100644 apptools/preferences/ui/tests/__init__.py create mode 100644 apptools/preferences/ui/tests/test_preferences_page.py diff --git a/apptools/preferences/tests/test_preferences_helper.py b/apptools/preferences/tests/test_preferences_helper.py index 72037e46b..cf6b1a0d0 100644 --- a/apptools/preferences/tests/test_preferences_helper.py +++ b/apptools/preferences/tests/test_preferences_helper.py @@ -15,7 +15,10 @@ from apptools.preferences.api import Preferences, PreferencesHelper from apptools.preferences.api import ScopedPreferences from apptools.preferences.api import set_default_preferences -from traits.api import Any, Bool, HasTraits, Int, Float, List, Str +from traits.api import ( + Any, Bool, cached_property, Event, HasTraits, Int, Float, List, Str, + Property, +) def width_listener(obj, trait_name, old, new): @@ -59,6 +62,9 @@ def setUp(self): # A temporary directory that can safely be written to. self.tmpdir = tempfile.mkdtemp() + # Path to a temporary file + self.tmpfile = os.path.join(self.tmpdir, "tmp.ini") + def tearDown(self): """ Called immediately after each test method has been called. """ @@ -245,16 +251,43 @@ class AcmeUIPreferencesHelper(PreferencesHelper): self.assertEqual(second_unicode_str, p.get('acme.ui.description')) # Save it to another file. - tmp = os.path.join(self.tmpdir, 'tmp.ini') - p.save(tmp) + p.save(self.tmpfile) # Load it into a new node. p = Preferences() - p.load(tmp) + p.load(self.tmpfile) self.assertEqual(second_unicode_str, p.get('acme.ui.description')) self.assertEqual(u'True', p.get('acme.ui.visible')) self.assertTrue(helper.visible) + def test_items_trait_not_saved(self): + """ Invisible and noneditable traits are not saved in preferences.""" + # Regression test for enthought/apptools#129 + + class MyPreferencesHelper(PreferencesHelper): + preferences_path = Str('my_section') + + list_of_str = List(Str) + + an_event = Event() + + a_property = Property() + + @cached_property + def _get_a_property(self): + return 1 + + helper = MyPreferencesHelper(list_of_str=["1"]) + + # Now modify the list to fire _items event + helper.list_of_str.append("2") + self.preferences.save(self.tmpfile) + + new_preferences = Preferences() + new_preferences.load(self.tmpfile) + + self.assertEqual(new_preferences.keys("my_section"), ["list_of_str"]) + def test_no_preferences_path(self): """ no preferences path """ diff --git a/apptools/preferences/ui/tests/__init__.py b/apptools/preferences/ui/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/apptools/preferences/ui/tests/test_preferences_page.py b/apptools/preferences/ui/tests/test_preferences_page.py new file mode 100644 index 000000000..6572790ea --- /dev/null +++ b/apptools/preferences/ui/tests/test_preferences_page.py @@ -0,0 +1,58 @@ +""" Tests for the preferences page. """ + +import unittest + +from traits.api import Enum +from traitsui.api import Group, Item, View + +from apptools.preferences.api import Preferences +from apptools.preferences.ui.api import PreferencesPage + + +class TestPreferencesPage(unittest.TestCase): + """ Non-GUI Tests for PreferencesPage.""" + + def test_preferences_page_apply(self): + """ Test applying the preferences """ + + # this sets up imitate Mayavi usage. + + class MyPreferencesPage(PreferencesPage): + + # the following set default values for class traits + category = "Application" + + help_id = "" + + name = "Note" + + preferences_path = "my_ref.pref" + + # custom preferences + + backend = Enum("auto", "simple", "test") + + traits_view = View(Group(Item("backend"))) + + preferences = Preferences() + pref_page = MyPreferencesPage( + preferences=preferences, + category="Another Application", + help_id="this_wont_be_saved", + name="Different Note", + # custom preferences + backend="simple", + ) + pref_page.apply() + + self.assertEqual(preferences.get("my_ref.pref.backend"), "simple") + self.assertEqual(preferences.keys("my_ref.pref"), ["backend"]) + + # this is not saved by virtue of it being static and never assigned to + self.assertIsNone(preferences.get("my_ref.pref.traits_view")) + + # These are skipped because this trait is defined on the + # PreferencesPage. + self.assertIsNone(preferences.get("my_ref.pref.help_id")) + self.assertIsNone(preferences.get("my_ref.pref.category")) + self.assertIsNone(preferences.get("my_ref.pref.name")) \ No newline at end of file From 24e0345afc9e796c62e88c1af11fe8bb2eae7571 Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Fri, 6 Nov 2020 14:47:09 +0000 Subject: [PATCH 2/9] Expand test for PreferencesPage --- .../ui/tests/test_preferences_page.py | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/apptools/preferences/ui/tests/test_preferences_page.py b/apptools/preferences/ui/tests/test_preferences_page.py index 6572790ea..f17d6bc92 100644 --- a/apptools/preferences/ui/tests/test_preferences_page.py +++ b/apptools/preferences/ui/tests/test_preferences_page.py @@ -2,7 +2,7 @@ import unittest -from traits.api import Enum +from traits.api import Enum, List, Str from traitsui.api import Group, Item, View from apptools.preferences.api import Preferences @@ -55,4 +55,22 @@ class MyPreferencesPage(PreferencesPage): # PreferencesPage. self.assertIsNone(preferences.get("my_ref.pref.help_id")) self.assertIsNone(preferences.get("my_ref.pref.category")) - self.assertIsNone(preferences.get("my_ref.pref.name")) \ No newline at end of file + self.assertIsNone(preferences.get("my_ref.pref.name")) + + def test_preferences_page_apply_skip_items_traits(self): + """ Test _items traits from List mutation are skipped. """ + # Regression test for enthought/apptools#129 + + class MyPreferencesPage(PreferencesPage): + preferences_path = "my_ref.pref" + names = List(Str()) + + preferences = Preferences() + pref_page = MyPreferencesPage( + preferences=preferences, + names=["1"], + ) + pref_page.names.append("2") + pref_page.apply() + + self.assertEqual(preferences.keys("my_ref.pref"), ["names"]) From 1c548cad19820adc387f295ff0a4f89d6b83e26d Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Fri, 6 Nov 2020 14:54:03 +0000 Subject: [PATCH 3/9] Correct test --- apptools/preferences/tests/test_preferences_helper.py | 9 +++++++-- apptools/preferences/ui/tests/test_preferences_page.py | 1 + 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/apptools/preferences/tests/test_preferences_helper.py b/apptools/preferences/tests/test_preferences_helper.py index cf6b1a0d0..c1f9f433a 100644 --- a/apptools/preferences/tests/test_preferences_helper.py +++ b/apptools/preferences/tests/test_preferences_helper.py @@ -260,8 +260,10 @@ class AcmeUIPreferencesHelper(PreferencesHelper): self.assertEqual(u'True', p.get('acme.ui.visible')) self.assertTrue(helper.visible) - def test_items_trait_not_saved(self): - """ Invisible and noneditable traits are not saved in preferences.""" + def test_mutate_list_of_values(self): + """ Mutated list should be saved and _items events not to be + saved in the preferences. + """ # Regression test for enthought/apptools#129 class MyPreferencesHelper(PreferencesHelper): @@ -286,6 +288,9 @@ def _get_a_property(self): new_preferences = Preferences() new_preferences.load(self.tmpfile) + self.assertEqual( + new_preferences.get("my_section.list_of_str"), str(["1", "2"]) + ) self.assertEqual(new_preferences.keys("my_section"), ["list_of_str"]) def test_no_preferences_path(self): diff --git a/apptools/preferences/ui/tests/test_preferences_page.py b/apptools/preferences/ui/tests/test_preferences_page.py index f17d6bc92..f1e01c183 100644 --- a/apptools/preferences/ui/tests/test_preferences_page.py +++ b/apptools/preferences/ui/tests/test_preferences_page.py @@ -73,4 +73,5 @@ class MyPreferencesPage(PreferencesPage): pref_page.names.append("2") pref_page.apply() + self.assertEqual(preferences.get("my_ref.pref.names"), str(["1", "2"])) self.assertEqual(preferences.keys("my_ref.pref"), ["names"]) From 4b918bc80dfaa342c65f564213b21fad4215faf0 Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Fri, 6 Nov 2020 17:01:08 +0000 Subject: [PATCH 4/9] Fix list items not being synchronized --- apptools/preferences/preferences_helper.py | 16 +++++++++++++++- .../preferences/tests/test_preferences_helper.py | 5 ++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/apptools/preferences/preferences_helper.py b/apptools/preferences/preferences_helper.py index 4967c9f00..54689162b 100644 --- a/apptools/preferences/preferences_helper.py +++ b/apptools/preferences/preferences_helper.py @@ -65,9 +65,23 @@ def _preferences_default(self): def _anytrait_changed(self, trait_name, old, new): """ Static trait change handler. """ + if self.preferences is None: + return + + # If the trait was a list or dict '_items' trait then just treat it as + # if the entire list or dict was changed. + if trait_name.endswith('_items'): + trait_name = trait_name[:-6] + if self._is_preference_trait(trait_name): + self.preferences.set( + '%s.%s' % (self._get_path(), trait_name), + getattr(self, trait_name) + ) + return + # If we were the one that set the trait (because the underlying # preferences node changed) then do nothing. - if self.preferences and self._is_preference_trait(trait_name): + if self._is_preference_trait(trait_name): self.preferences.set('%s.%s' % (self._get_path(), trait_name), new) return diff --git a/apptools/preferences/tests/test_preferences_helper.py b/apptools/preferences/tests/test_preferences_helper.py index c1f9f433a..d43df7917 100644 --- a/apptools/preferences/tests/test_preferences_helper.py +++ b/apptools/preferences/tests/test_preferences_helper.py @@ -17,7 +17,7 @@ from apptools.preferences.api import set_default_preferences from traits.api import ( Any, Bool, cached_property, Event, HasTraits, Int, Float, List, Str, - Property, + Property, push_exception_handler, pop_exception_handler, ) @@ -65,6 +65,9 @@ def setUp(self): # Path to a temporary file self.tmpfile = os.path.join(self.tmpdir, "tmp.ini") + push_exception_handler(reraise_exceptions=True) + self.addCleanup(pop_exception_handler) + def tearDown(self): """ Called immediately after each test method has been called. """ From f201aaae9177b780db4055091c617c2295fc9c84 Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Fri, 6 Nov 2020 17:11:58 +0000 Subject: [PATCH 5/9] Expand class docstring to mention known limitation --- apptools/preferences/preferences_helper.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/apptools/preferences/preferences_helper.py b/apptools/preferences/preferences_helper.py index 54689162b..388dc6b18 100644 --- a/apptools/preferences/preferences_helper.py +++ b/apptools/preferences/preferences_helper.py @@ -17,7 +17,14 @@ class PreferencesHelper(HasTraits): - """ An object that can be initialized from a preferences node. """ + """ A base class for objects that can be initialized from a preferences + node. + + Additional traits defined on subclasses will be listened to. Changes + are then synchronized with the preferences. Note that mutations on nested + containers e.g. List(List(Str)) cannot be synchronized and should be + avoided. + """ #### 'PreferencesHelper' interface ######################################## From d99ca8cf9404610f1d30c8b63d52608f60d403cd Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Fri, 6 Nov 2020 17:12:28 +0000 Subject: [PATCH 6/9] Add a test for the known limitation --- .../tests/test_preferences_helper.py | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/apptools/preferences/tests/test_preferences_helper.py b/apptools/preferences/tests/test_preferences_helper.py index d43df7917..fed792a98 100644 --- a/apptools/preferences/tests/test_preferences_helper.py +++ b/apptools/preferences/tests/test_preferences_helper.py @@ -296,6 +296,35 @@ def _get_a_property(self): ) self.assertEqual(new_preferences.keys("my_section"), ["list_of_str"]) + def test_nested_container_mutation_not_supported(self): + """ Known limitation: mutation on nested containers are not + synchronized + """ + + class MyPreferencesHelper(PreferencesHelper): + preferences_path = Str('my_section') + list_of_list_of_str = List(List(Str)) + + helper = MyPreferencesHelper(list_of_list_of_str=[["1"]]) + helper.list_of_list_of_str[0].append("9") + + self.preferences.save(self.tmpfile) + + new_preferences = Preferences() + new_preferences.load(self.tmpfile) + + # The event trait is not saved. + self.assertEqual( + new_preferences.keys("my_section"), ["list_of_list_of_str"] + ) + + # The values are not synchronized + with self.assertRaises(AssertionError): + self.assertEqual( + new_preferences.get("my_section.list_of_list_of_str"), + str(helper.list_of_list_of_str) + ) + def test_no_preferences_path(self): """ no preferences path """ From dcfcf5760a7379d9b8a7e271834a3c063de3cb05 Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Fri, 6 Nov 2020 17:17:29 +0000 Subject: [PATCH 7/9] Add an issue reference --- apptools/preferences/tests/test_preferences_helper.py | 1 + 1 file changed, 1 insertion(+) diff --git a/apptools/preferences/tests/test_preferences_helper.py b/apptools/preferences/tests/test_preferences_helper.py index fed792a98..80246fa36 100644 --- a/apptools/preferences/tests/test_preferences_helper.py +++ b/apptools/preferences/tests/test_preferences_helper.py @@ -299,6 +299,7 @@ def _get_a_property(self): def test_nested_container_mutation_not_supported(self): """ Known limitation: mutation on nested containers are not synchronized + See enthought/apptools#194 """ class MyPreferencesHelper(PreferencesHelper): From 2ec9cdfe47c2e87e8c0f9753bc1775a1d5038cd0 Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Fri, 6 Nov 2020 17:20:39 +0000 Subject: [PATCH 8/9] Remove unrelated setup --- .../preferences/tests/test_preferences_helper.py | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/apptools/preferences/tests/test_preferences_helper.py b/apptools/preferences/tests/test_preferences_helper.py index 80246fa36..7f3cedbd8 100644 --- a/apptools/preferences/tests/test_preferences_helper.py +++ b/apptools/preferences/tests/test_preferences_helper.py @@ -16,8 +16,8 @@ from apptools.preferences.api import ScopedPreferences from apptools.preferences.api import set_default_preferences from traits.api import ( - Any, Bool, cached_property, Event, HasTraits, Int, Float, List, Str, - Property, push_exception_handler, pop_exception_handler, + Any, Bool, HasTraits, Int, Float, List, Str, + push_exception_handler, pop_exception_handler, ) @@ -274,14 +274,6 @@ class MyPreferencesHelper(PreferencesHelper): list_of_str = List(Str) - an_event = Event() - - a_property = Property() - - @cached_property - def _get_a_property(self): - return 1 - helper = MyPreferencesHelper(list_of_str=["1"]) # Now modify the list to fire _items event From 70f3111d16a0747f3d70ab9d657b6a6bcabcf869 Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Fri, 6 Nov 2020 18:03:25 +0000 Subject: [PATCH 9/9] Add a news fragment --- docs/releases/upcoming/196.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 docs/releases/upcoming/196.bugfix.rst diff --git a/docs/releases/upcoming/196.bugfix.rst b/docs/releases/upcoming/196.bugfix.rst new file mode 100644 index 000000000..0c12dfeb0 --- /dev/null +++ b/docs/releases/upcoming/196.bugfix.rst @@ -0,0 +1 @@ +Fix container items change event being saved in preferences (#196) \ No newline at end of file