From 5bb40926b8f18956a27f29a5858d65e84da24b8a Mon Sep 17 00:00:00 2001 From: Facetoe Date: Sun, 7 Aug 2016 01:16:27 +0800 Subject: [PATCH 1/5] Ensure required settings are defined correctly. Possible fix for #424. If a setting has not been overidden in a subclass or set in the user's config then raise an exception. --- i3pystatus/core/settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/i3pystatus/core/settings.py b/i3pystatus/core/settings.py index 75095e8..4bc1d86 100644 --- a/i3pystatus/core/settings.py +++ b/i3pystatus/core/settings.py @@ -33,7 +33,7 @@ class SettingsBaseMeta(type): # required anymore. for base in inspect.getmro(cls): for r in list(required): - if hasattr(base, r): + if hasattr(base, r) and getattr(base, r) != getattr(cls, r): required.remove(r) return unique(settings), required From 86fddf8e22b97ab4b57a1e33de1fe21c89c4af58 Mon Sep 17 00:00:00 2001 From: Facetoe Date: Tue, 9 Aug 2016 18:14:25 +0800 Subject: [PATCH 2/5] Add tests for required settings. --- tests/test_core_modules.py | 50 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/tests/test_core_modules.py b/tests/test_core_modules.py index 0ca7897..d3e7982 100644 --- a/tests/test_core_modules.py +++ b/tests/test_core_modules.py @@ -2,9 +2,9 @@ import time from unittest.mock import MagicMock import pytest - from i3pystatus import IntervalModule -from i3pystatus.core.modules import is_method_of +from i3pystatus.core.exceptions import ConfigMissingError +from i3pystatus.core.modules import is_method_of, Module left_click = 1 right_click = 3 @@ -145,3 +145,49 @@ def test_is_method_of(): assert not is_method_of(source_object.assigned_function, object) assert not is_method_of(source_object.member, object) assert not is_method_of(source_object.string_member, object) + + +def test_required_raises(): + """ Ensure undefined required settings raise a ConfigMissingError """ + + class TestRequired(Module): + settings = ( + ("some_setting",), + ) + required = ('some_setting',) + + with pytest.raises(ConfigMissingError): + TestRequired() + + TestRequired(some_setting='foo') + + +def test_required_defined_raises(): + """ Ensure defined but unmodified required settings raise a ConfigMissingError """ + + class TestRequiredDefined(Module): + settings = ( + ("some_setting",), + ) + required = ('some_setting',) + some_setting = None + + with pytest.raises(ConfigMissingError): + TestRequiredDefined() + + TestRequiredDefined(some_setting='foo') + + +def test_required_subclass_overide(): + """ Ensure required settings defined in subclasses do not raise a ConfigMissingError """ + + class TestRequiredDefined(Module): + settings = ( + ("some_setting",), + ) + required = ('some_setting',) + + class TestSubClass(TestRequiredDefined): + some_setting = 'foo' + + TestSubClass() From c30365338b67a9ad9c64125d7452597b851c9208 Mon Sep 17 00:00:00 2001 From: Facetoe Date: Tue, 9 Aug 2016 18:20:40 +0800 Subject: [PATCH 3/5] Refactor conditional to pass required setting tests. If a setting is defined in a subclass and is not None do not raise config error. --- i3pystatus/core/settings.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/i3pystatus/core/settings.py b/i3pystatus/core/settings.py index 4bc1d86..b2d6e3d 100644 --- a/i3pystatus/core/settings.py +++ b/i3pystatus/core/settings.py @@ -33,8 +33,10 @@ class SettingsBaseMeta(type): # required anymore. for base in inspect.getmro(cls): for r in list(required): - if hasattr(base, r) and getattr(base, r) != getattr(cls, r): + if hasattr(base, r) and getattr(base, r) != getattr(cls, r) \ + or hasattr(cls, r) and getattr(cls, r) is not None: required.remove(r) + return unique(settings), required From 8d8c0b681247cc34711998759d30968325fa06ce Mon Sep 17 00:00:00 2001 From: Facetoe Date: Tue, 9 Aug 2016 18:33:28 +0800 Subject: [PATCH 4/5] Update comment --- i3pystatus/core/settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/i3pystatus/core/settings.py b/i3pystatus/core/settings.py index b2d6e3d..e84fd3f 100644 --- a/i3pystatus/core/settings.py +++ b/i3pystatus/core/settings.py @@ -30,7 +30,7 @@ class SettingsBaseMeta(type): settings += tuple(getattr(base, "settings", [])) required |= set(getattr(base, "required", [])) # if a derived class defines a default for a setting it is not - # required anymore. + # required anymore, provided that default is not set to None. for base in inspect.getmro(cls): for r in list(required): if hasattr(base, r) and getattr(base, r) != getattr(cls, r) \ From 3d48213834cc0635acf2c57b5d3522583899c855 Mon Sep 17 00:00:00 2001 From: Facetoe Date: Tue, 9 Aug 2016 18:33:35 +0800 Subject: [PATCH 5/5] Add test for case where subclass defines a setting and sets it to None --- tests/test_core_modules.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/test_core_modules.py b/tests/test_core_modules.py index d3e7982..3d838fe 100644 --- a/tests/test_core_modules.py +++ b/tests/test_core_modules.py @@ -178,6 +178,23 @@ def test_required_defined_raises(): TestRequiredDefined(some_setting='foo') +def test_required_subclass_none_raises(): + """ Ensure required settings defined in subclasses raise a ConfigMissingError if they are set to None""" + + class TestRequiredDefined(Module): + settings = ( + ("some_setting",), + ) + required = ('some_setting',) + + class TestSubClass(TestRequiredDefined): + some_setting = None + + with pytest.raises(ConfigMissingError): + TestRequiredDefined() + TestSubClass(some_setting='foo') + + def test_required_subclass_overide(): """ Ensure required settings defined in subclasses do not raise a ConfigMissingError """