From 875c2f89232f484df1c98e606f0df604d7179c24 Mon Sep 17 00:00:00 2001 From: Andrew Helsby Date: Wed, 19 Oct 2022 14:17:52 +0400 Subject: [PATCH 01/10] fix/unit-tests: Improve unit test coverage to ensure spec is followed Signed-off-by: Andrew Helsby Signed-off-by: Andrew Helsby --- tests/{provider => }/conftest.py | 0 tests/provider/test_no_op_provider.py | 29 ++--- tests/test_open_feature_api.py | 56 +++++++++ tests/test_open_feature_client.py | 108 ++++++++++++++++++ tests/test_open_feature_evaluation_context.py | 33 ++++++ 5 files changed, 206 insertions(+), 20 deletions(-) rename tests/{provider => }/conftest.py (100%) create mode 100644 tests/test_open_feature_api.py create mode 100644 tests/test_open_feature_client.py create mode 100644 tests/test_open_feature_evaluation_context.py diff --git a/tests/provider/conftest.py b/tests/conftest.py similarity index 100% rename from tests/provider/conftest.py rename to tests/conftest.py diff --git a/tests/provider/test_no_op_provider.py b/tests/provider/test_no_op_provider.py index aed960ae..1e1e4b96 100644 --- a/tests/provider/test_no_op_provider.py +++ b/tests/provider/test_no_op_provider.py @@ -1,16 +1,9 @@ from numbers import Number -from open_feature import open_feature_api as api from open_feature.provider.no_op_provider import NoOpProvider -def setup(): - api.set_provider(NoOpProvider()) - provider = api.get_provider() - assert isinstance(provider, NoOpProvider) - - -def test_should_return_no_op_provider_metadata(no_op_provider_client): +def test_should_return_no_op_provider_metadata(): # Given # When metadata = NoOpProvider().get_metadata() @@ -20,39 +13,37 @@ def test_should_return_no_op_provider_metadata(no_op_provider_client): assert metadata.is_default_provider -def test_should_get_boolean_flag_from_no_op(no_op_provider_client): +def test_should_get_boolean_flag_from_no_op(): # Given # When - flag = no_op_provider_client.get_boolean_details(flag_key="Key", default_value=True) + flag = NoOpProvider().get_boolean_details(flag_key="Key", default_value=True) # Then assert flag is not None assert flag.value assert isinstance(flag.value, bool) -def test_should_get_number_flag_from_no_op(no_op_provider_client): +def test_should_get_number_flag_from_no_op(): # Given # When - flag = no_op_provider_client.get_number_details(flag_key="Key", default_value=100) + flag = NoOpProvider().get_number_details(flag_key="Key", default_value=100) # Then assert flag is not None assert flag.value == 100 assert isinstance(flag.value, Number) -def test_should_get_string_flag_from_no_op(no_op_provider_client): +def test_should_get_string_flag_from_no_op(): # Given # When - flag = no_op_provider_client.get_string_details( - flag_key="Key", default_value="String" - ) + flag = NoOpProvider().get_string_details(flag_key="Key", default_value="String") # Then assert flag is not None assert flag.value == "String" assert isinstance(flag.value, str) -def test_should_get_object_flag_from_no_op(no_op_provider_client): +def test_should_get_object_flag_from_no_op(): # Given return_value = { "String": "string", @@ -60,9 +51,7 @@ def test_should_get_object_flag_from_no_op(no_op_provider_client): "Boolean": True, } # When - flag = no_op_provider_client.get_string_details( - flag_key="Key", default_value=return_value - ) + flag = NoOpProvider().get_object_details(flag_key="Key", default_value=return_value) # Then assert flag is not None assert flag.value == return_value diff --git a/tests/test_open_feature_api.py b/tests/test_open_feature_api.py new file mode 100644 index 00000000..c3bee46d --- /dev/null +++ b/tests/test_open_feature_api.py @@ -0,0 +1,56 @@ +import pytest + +from open_feature.exception.exceptions import GeneralError +from open_feature.flag_evaluation.error_code import ErrorCode +from open_feature.open_feature_api import get_client, get_provider, set_provider +from open_feature.provider.no_op_provider import NoOpProvider + + +def test_should_raise_exception_with_nop_client(): + # Given + # When + with pytest.raises(GeneralError) as ge: + get_client() + # Then + assert ge.value + assert ( + ge.value.error_message + == "Provider not set. Call set_provider before using get_client" + ) + assert ge.value.error_code == ErrorCode.GENERAL + + +def test_should_return_open_feature_client_when_configured_correctly(): + # Given + set_provider(NoOpProvider()) + + # When + client = get_client(name="No-op Provider", version="1.0") + + # Then + assert client.name == "No-op Provider" + assert client.version == "1.0" + assert isinstance(client.provider, NoOpProvider) + + +def test_should_try_set_provider_and_fail_if_none_provided(): + # Given + # When + with pytest.raises(GeneralError) as ge: + set_provider(provider=None) + + # Then + assert ge.value.error_message == "No provider" + assert ge.value.error_code == ErrorCode.GENERAL + + +def test_should_return_a_provider_if_setup_correctly(): + # Given + set_provider(NoOpProvider()) + + # When + provider = get_provider() + + # Then + assert provider + assert isinstance(provider, NoOpProvider) diff --git a/tests/test_open_feature_client.py b/tests/test_open_feature_client.py new file mode 100644 index 00000000..ea087254 --- /dev/null +++ b/tests/test_open_feature_client.py @@ -0,0 +1,108 @@ +from numbers import Number + +from open_feature import open_feature_api as api +from open_feature.provider.no_op_provider import NoOpProvider + + +def setup(): + api.set_provider(NoOpProvider()) + provider = api.get_provider() + assert isinstance(provider, NoOpProvider) + + +def test_should_get_boolean_flag(no_op_provider_client): + # Given + # When + flag = no_op_provider_client.get_boolean_details(flag_key="Key", default_value=True) + # Then + assert flag is not None + assert flag.value + assert isinstance(flag.value, bool) + + +def test_should_get_boolean_flag_value(no_op_provider_client): + # Given + # When + flag = no_op_provider_client.get_boolean_value(flag_key="Key", default_value=True) + # Then + assert flag is not None + assert flag + assert isinstance(flag, bool) + + +def test_should_get_number_flag(no_op_provider_client): + # Given + # When + flag = no_op_provider_client.get_number_details(flag_key="Key", default_value=100) + # Then + assert flag is not None + assert flag.value == 100 + assert isinstance(flag.value, Number) + + +def test_should_get_number_flag_value(no_op_provider_client): + # Given + # When + flag = no_op_provider_client.get_number_value(flag_key="Key", default_value=100) + # Then + assert flag is not None + assert flag == 100 + assert isinstance(flag, Number) + + +def test_should_get_string_flag(no_op_provider_client): + # Given + # When + flag = no_op_provider_client.get_string_details( + flag_key="Key", default_value="String" + ) + # Then + assert flag is not None + assert flag.value == "String" + assert isinstance(flag.value, str) + + +def test_should_get_string_flag_value(no_op_provider_client): + # Given + # When + flag = no_op_provider_client.get_string_value( + flag_key="Key", default_value="String" + ) + # Then + assert flag is not None + assert flag == "String" + assert isinstance(flag, str) + + +def test_should_get_object_flag(no_op_provider_client): + # Given + return_value = { + "String": "string", + "Number": 2, + "Boolean": True, + } + # When + flag = no_op_provider_client.get_object_details( + flag_key="Key", default_value=return_value + ) + # Then + assert flag is not None + assert flag.value == return_value + assert isinstance(flag.value, dict) + + +def test_should_get_object_flag_value(no_op_provider_client): + # Given + return_value = { + "String": "string", + "Number": 2, + "Boolean": True, + } + # When + flag = no_op_provider_client.get_object_value( + flag_key="Key", default_value=return_value + ) + # Then + assert flag is not None + assert flag == return_value + assert isinstance(flag, dict) diff --git a/tests/test_open_feature_evaluation_context.py b/tests/test_open_feature_evaluation_context.py new file mode 100644 index 00000000..fc353d96 --- /dev/null +++ b/tests/test_open_feature_evaluation_context.py @@ -0,0 +1,33 @@ +import pytest + +from open_feature.evaluation_context.evaluation_context import EvaluationContext +from open_feature.exception.exceptions import GeneralError +from open_feature.flag_evaluation.error_code import ErrorCode +from open_feature.open_feature_evaluation_context import ( + api_evaluation_context, + set_api_evaluation_context, +) + + +def test_should_raise_an_exception_if_no_evaluation_context_set(): + # Given + with pytest.raises(GeneralError) as ge: + set_api_evaluation_context(evaluation_context=None) + # Then + assert ge.value + assert ge.value.error_message == "No api level evaluation context" + assert ge.value.error_code == ErrorCode.GENERAL + + +def test_should_successfully_set_evaluation_context_for_api(): + # Given + evaluation_context = EvaluationContext("targeting_key", {"attr1": "val1"}) + + # When + set_api_evaluation_context(evaluation_context) + global_evaluation_context = api_evaluation_context() + + # Then + assert global_evaluation_context + assert global_evaluation_context.targeting_key == evaluation_context.targeting_key + assert global_evaluation_context.attributes == evaluation_context.attributes From 6d9a0440a2d8ecaac5d159f8519f615afe34bc20 Mon Sep 17 00:00:00 2001 From: Andrew Helsby Date: Wed, 19 Oct 2022 15:10:05 +0400 Subject: [PATCH 02/10] fix/unit-tests: Add test coverage around hooks Signed-off-by: Andrew Helsby Signed-off-by: Andrew Helsby --- open_feature/hooks/hook_support.py | 8 ++-- open_feature/open_feature_client.py | 10 ++--- tests/conftest.py | 45 ++++++++++++++++++++ tests/test_open_feature_client.py | 65 +++++++++++++++++++++++++++++ 4 files changed, 119 insertions(+), 9 deletions(-) diff --git a/open_feature/hooks/hook_support.py b/open_feature/hooks/hook_support.py index 119359d0..11115044 100644 --- a/open_feature/hooks/hook_support.py +++ b/open_feature/hooks/hook_support.py @@ -17,7 +17,7 @@ def error_hooks( hooks: typing.List[Hook], hints: dict, ): - kwargs = {"ctx": hook_context, "exception": exception, "hints": hints} + kwargs = {"hook_context": hook_context, "exception": exception, "hints": hints} _execute_hooks( flag_type=flag_type, hooks=hooks, hook_method=HookType.ERROR, **kwargs ) @@ -29,7 +29,7 @@ def after_all_hooks( hooks: typing.List[Hook], hints: dict, ): - kwargs = {"ctx": hook_context, "hints": hints} + kwargs = {"hook_context": hook_context, "hints": hints} _execute_hooks( flag_type=flag_type, hooks=hooks, hook_method=HookType.FINALLY_AFTER, **kwargs ) @@ -42,7 +42,7 @@ def after_hooks( hooks: typing.List[Hook], hints: dict, ): - kwargs = {"ctx": hook_context, "details": details, "hints": hints} + kwargs = {"hook_context": hook_context, "details": details, "hints": hints} _execute_hooks_unchecked( flag_type=flag_type, hooks=hooks, hook_method=HookType.AFTER, **kwargs ) @@ -54,7 +54,7 @@ def before_hooks( hooks: typing.List[Hook], hints: dict, ) -> EvaluationContext: - kwargs = {"ctx": hook_context, "hints": hints} + kwargs = {"hook_context": hook_context, "hints": hints} executed_hooks = _execute_hooks_unchecked( flag_type=flag_type, hooks=hooks, hook_method=HookType.BEFORE, **kwargs ) diff --git a/open_feature/open_feature_client.py b/open_feature/open_feature_client.py index a685f4b7..8be455ef 100644 --- a/open_feature/open_feature_client.py +++ b/open_feature/open_feature_client.py @@ -3,7 +3,7 @@ from numbers import Number from open_feature.evaluation_context.evaluation_context import EvaluationContext -from open_feature.exception.exceptions import GeneralError +from open_feature.exception.exceptions import GeneralError, OpenFeatureError from open_feature.flag_evaluation.error_code import ErrorCode from open_feature.flag_evaluation.flag_evaluation_details import FlagEvaluationDetails from open_feature.flag_evaluation.flag_type import FlagType @@ -190,7 +190,7 @@ def evaluate_flag_details( client_metadata=None, provider_metadata=None, ) - merged_hooks = [] + merged_hooks = self.hooks try: # https://github.com/open-feature/spec/blob/main/specification/sections/03-evaluation-context.md @@ -206,7 +206,7 @@ def evaluate_flag_details( api_evaluation_context().merge(self.context).merge(invocation_context) ) - flag_evaluation = self.create_provider_evaluation( + flag_evaluation = self._create_provider_evaluation( flag_type, flag_key, default_value, @@ -217,7 +217,7 @@ def evaluate_flag_details( return flag_evaluation - except OpenFeatureError as e: # noqa + except OpenFeatureError as e: error_hooks(flag_type, hook_context, e, merged_hooks, None) return FlagEvaluationDetails( flag_key=flag_key, @@ -242,7 +242,7 @@ def evaluate_flag_details( finally: after_all_hooks(flag_type, hook_context, merged_hooks, None) - def create_provider_evaluation( + def _create_provider_evaluation( self, flag_type: FlagType, flag_key: str, diff --git a/tests/conftest.py b/tests/conftest.py index b2161c68..5e5b3b0c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,6 +1,13 @@ import pytest from open_feature import open_feature_api as api +from open_feature.evaluation_context.evaluation_context import EvaluationContext +from open_feature.exception.exceptions import OpenFeatureError +from open_feature.flag_evaluation.error_code import ErrorCode +from open_feature.flag_evaluation.flag_evaluation_details import FlagEvaluationDetails +from open_feature.flag_evaluation.flag_type import FlagType +from open_feature.hooks.hook import Hook +from open_feature.hooks.hook_context import HookContext from open_feature.provider.no_op_provider import NoOpProvider @@ -8,3 +15,41 @@ def no_op_provider_client(): api.set_provider(NoOpProvider()) return api.get_client() + + +class TestExceptionHook(Hook): + def before(self, hook_context: HookContext, hints: dict) -> EvaluationContext: + pass + + def after( + self, hook_context: HookContext, details: FlagEvaluationDetails, hints: dict + ): + raise Exception("Generic exception raised") + + def error(self, hook_context: HookContext, exception: Exception, hints: dict): + pass + + def finally_after(self, hook_context: HookContext, hints: dict): + pass + + def supports_flag_value_type(self, flag_type: FlagType) -> bool: + return True + + +class TestOpenFeatureErrorHook(Hook): + def before(self, hook_context: HookContext, hints: dict) -> EvaluationContext: + pass + + def after( + self, hook_context: HookContext, details: FlagEvaluationDetails, hints: dict + ): + raise OpenFeatureError("error_message", ErrorCode.GENERAL) + + def error(self, hook_context: HookContext, exception: Exception, hints: dict): + pass + + def finally_after(self, hook_context: HookContext, hints: dict): + pass + + def supports_flag_value_type(self, flag_type: FlagType) -> bool: + return True diff --git a/tests/test_open_feature_client.py b/tests/test_open_feature_client.py index ea087254..125f47be 100644 --- a/tests/test_open_feature_client.py +++ b/tests/test_open_feature_client.py @@ -1,7 +1,15 @@ from numbers import Number +import pytest + from open_feature import open_feature_api as api +from open_feature.exception.exceptions import GeneralError +from open_feature.flag_evaluation.error_code import ErrorCode +from open_feature.flag_evaluation.flag_type import FlagType +from open_feature.flag_evaluation.reason import Reason +from open_feature.open_feature_client import OpenFeatureClient from open_feature.provider.no_op_provider import NoOpProvider +from tests.conftest import TestExceptionHook, TestOpenFeatureErrorHook def setup(): @@ -10,6 +18,31 @@ def setup(): assert isinstance(provider, NoOpProvider) +def test_should_use_no_op_provider_if_none_provided(): + # Given + # When + flag = OpenFeatureClient("No provider", "1.0")._create_provider_evaluation( + flag_type=FlagType.BOOLEAN, flag_key="Key", default_value=True + ) + # Then + assert flag is not None + assert flag.value + assert isinstance(flag.value, bool) + + +def test_should_raise_exception_when_invalid_flag_type_provided(): + # Given + # When + with pytest.raises(GeneralError) as ge: + OpenFeatureClient("No provider", "1.0")._create_provider_evaluation( + flag_type=None, flag_key="Key", default_value=True + ) + # Then + assert ge.value + assert ge.value.error_message == "Unknown flag type" + assert ge.value.error_code == ErrorCode.GENERAL + + def test_should_get_boolean_flag(no_op_provider_client): # Given # When @@ -106,3 +139,35 @@ def test_should_get_object_flag_value(no_op_provider_client): assert flag is not None assert flag == return_value assert isinstance(flag, dict) + + +def test_should_handle_a_generic_exception_thrown_by_a_provider(no_op_provider_client): + # Given + no_op_provider_client.add_hooks([TestExceptionHook()]) + # When + flag_details = no_op_provider_client.get_boolean_details( + flag_key="Key", default_value=True + ) + # Then + assert flag_details is not None + assert flag_details.value + assert isinstance(flag_details.value, bool) + assert flag_details.reason == Reason.ERROR + assert flag_details.error_message == "Generic exception raised" + + +def test_should_handle_an_open_feature_exception_thrown_by_a_provider( + no_op_provider_client, +): + # Given + no_op_provider_client.add_hooks([TestOpenFeatureErrorHook()]) + # When + flag_details = no_op_provider_client.get_boolean_details( + flag_key="Key", default_value=True + ) + # Then + assert flag_details is not None + assert flag_details.value + assert isinstance(flag_details.value, bool) + assert flag_details.reason == Reason.ERROR + assert flag_details.error_message == "error_message" From 5da054eb0bfbdc38fc40b0925f9428a3447525c4 Mon Sep 17 00:00:00 2001 From: Andrew Helsby Date: Wed, 2 Nov 2022 10:58:01 +0400 Subject: [PATCH 03/10] fix/unit-tests: Parametrize test_should_use_no_op_provider_if_none_provided Signed-off-by: Andrew Helsby Signed-off-by: Andrew Helsby --- tests/test_open_feature_client.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/test_open_feature_client.py b/tests/test_open_feature_client.py index 125f47be..33e93cbf 100644 --- a/tests/test_open_feature_client.py +++ b/tests/test_open_feature_client.py @@ -18,11 +18,20 @@ def setup(): assert isinstance(provider, NoOpProvider) -def test_should_use_no_op_provider_if_none_provided(): +@pytest.mark.parametrize( + "flag_type, default_value", + ( + (FlagType.BOOLEAN, True), + (FlagType.STRING, True), + (FlagType.NUMBER, True), + (FlagType.OBJECT, True), + ), +) +def test_should_use_no_op_provider_if_none_provided(flag_type, default_value): # Given # When flag = OpenFeatureClient("No provider", "1.0")._create_provider_evaluation( - flag_type=FlagType.BOOLEAN, flag_key="Key", default_value=True + flag_type=flag_type, flag_key="Key", default_value=default_value ) # Then assert flag is not None From b91d636cbd3edd22acdf389c9a76d6e5c1776bfa Mon Sep 17 00:00:00 2001 From: Andrew Helsby Date: Wed, 2 Nov 2022 12:17:56 +0400 Subject: [PATCH 04/10] fix/unit-tests: Parametrize all flag type methods in open feature client Signed-off-by: Andrew Helsby Signed-off-by: Andrew Helsby --- tests/test_open_feature_api.py | 6 ++ tests/test_open_feature_client.py | 160 ++++++++++-------------------- 2 files changed, 57 insertions(+), 109 deletions(-) diff --git a/tests/test_open_feature_api.py b/tests/test_open_feature_api.py index c3bee46d..c695d35c 100644 --- a/tests/test_open_feature_api.py +++ b/tests/test_open_feature_api.py @@ -31,6 +31,8 @@ def test_should_return_open_feature_client_when_configured_correctly(): assert client.name == "No-op Provider" assert client.version == "1.0" assert isinstance(client.provider, NoOpProvider) + # Setting private provider back to None to ensure other tests aren't affected + _provider = None # noqa: F841 def test_should_try_set_provider_and_fail_if_none_provided(): @@ -42,6 +44,8 @@ def test_should_try_set_provider_and_fail_if_none_provided(): # Then assert ge.value.error_message == "No provider" assert ge.value.error_code == ErrorCode.GENERAL + # Setting private provider back to None to ensure other tests aren't affected + _provider = None # noqa: F841 def test_should_return_a_provider_if_setup_correctly(): @@ -54,3 +58,5 @@ def test_should_return_a_provider_if_setup_correctly(): # Then assert provider assert isinstance(provider, NoOpProvider) + # Setting private provider back to None to ensure other tests aren't affected + _provider = None # noqa: F841 diff --git a/tests/test_open_feature_client.py b/tests/test_open_feature_client.py index 33e93cbf..1ea6ae65 100644 --- a/tests/test_open_feature_client.py +++ b/tests/test_open_feature_client.py @@ -5,7 +5,6 @@ from open_feature import open_feature_api as api from open_feature.exception.exceptions import GeneralError from open_feature.flag_evaluation.error_code import ErrorCode -from open_feature.flag_evaluation.flag_type import FlagType from open_feature.flag_evaluation.reason import Reason from open_feature.open_feature_client import OpenFeatureClient from open_feature.provider.no_op_provider import NoOpProvider @@ -19,24 +18,65 @@ def setup(): @pytest.mark.parametrize( - "flag_type, default_value", + "flag_type, default_value, get_method", ( - (FlagType.BOOLEAN, True), - (FlagType.STRING, True), - (FlagType.NUMBER, True), - (FlagType.OBJECT, True), + (bool, True, "get_boolean_value"), + (str, "String", "get_string_value"), + (Number, 100, "get_number_value"), + ( + dict, + { + "String": "string", + "Number": 2, + "Boolean": True, + }, + "get_object_value", + ), ), ) -def test_should_use_no_op_provider_if_none_provided(flag_type, default_value): +def test_should_get_flag_value_based_on_method_type( + flag_type, default_value, get_method, no_op_provider_client +): # Given # When - flag = OpenFeatureClient("No provider", "1.0")._create_provider_evaluation( - flag_type=flag_type, flag_key="Key", default_value=default_value + flag = getattr(no_op_provider_client, get_method)( + flag_key="Key", default_value=default_value ) # Then assert flag is not None - assert flag.value - assert isinstance(flag.value, bool) + assert flag == default_value + assert isinstance(flag, flag_type) + + +@pytest.mark.parametrize( + "flag_type, default_value, get_method", + ( + (bool, True, "get_boolean_detail"), + (str, "String", "get_string_detail"), + (Number, 100, "get_number_detail"), + ( + dict, + { + "String": "string", + "Number": 2, + "Boolean": True, + }, + "get_object_detail", + ), + ), +) +def test_should_get_flag_detail_based_on_method_type( + flag_type, default_value, get_method, no_op_provider_client +): + # Given + # When + flag = getattr(no_op_provider_client, get_method)( + flag_key="Key", default_value=default_value + ) + # Then + assert flag is not None + assert flag.value == default_value + assert isinstance(flag.value, flag_type) def test_should_raise_exception_when_invalid_flag_type_provided(): @@ -52,104 +92,6 @@ def test_should_raise_exception_when_invalid_flag_type_provided(): assert ge.value.error_code == ErrorCode.GENERAL -def test_should_get_boolean_flag(no_op_provider_client): - # Given - # When - flag = no_op_provider_client.get_boolean_details(flag_key="Key", default_value=True) - # Then - assert flag is not None - assert flag.value - assert isinstance(flag.value, bool) - - -def test_should_get_boolean_flag_value(no_op_provider_client): - # Given - # When - flag = no_op_provider_client.get_boolean_value(flag_key="Key", default_value=True) - # Then - assert flag is not None - assert flag - assert isinstance(flag, bool) - - -def test_should_get_number_flag(no_op_provider_client): - # Given - # When - flag = no_op_provider_client.get_number_details(flag_key="Key", default_value=100) - # Then - assert flag is not None - assert flag.value == 100 - assert isinstance(flag.value, Number) - - -def test_should_get_number_flag_value(no_op_provider_client): - # Given - # When - flag = no_op_provider_client.get_number_value(flag_key="Key", default_value=100) - # Then - assert flag is not None - assert flag == 100 - assert isinstance(flag, Number) - - -def test_should_get_string_flag(no_op_provider_client): - # Given - # When - flag = no_op_provider_client.get_string_details( - flag_key="Key", default_value="String" - ) - # Then - assert flag is not None - assert flag.value == "String" - assert isinstance(flag.value, str) - - -def test_should_get_string_flag_value(no_op_provider_client): - # Given - # When - flag = no_op_provider_client.get_string_value( - flag_key="Key", default_value="String" - ) - # Then - assert flag is not None - assert flag == "String" - assert isinstance(flag, str) - - -def test_should_get_object_flag(no_op_provider_client): - # Given - return_value = { - "String": "string", - "Number": 2, - "Boolean": True, - } - # When - flag = no_op_provider_client.get_object_details( - flag_key="Key", default_value=return_value - ) - # Then - assert flag is not None - assert flag.value == return_value - assert isinstance(flag.value, dict) - - -def test_should_get_object_flag_value(no_op_provider_client): - # Given - return_value = { - "String": "string", - "Number": 2, - "Boolean": True, - } - # When - flag = no_op_provider_client.get_object_value( - flag_key="Key", default_value=return_value - ) - # Then - assert flag is not None - assert flag == return_value - assert isinstance(flag, dict) - - def test_should_handle_a_generic_exception_thrown_by_a_provider(no_op_provider_client): # Given no_op_provider_client.add_hooks([TestExceptionHook()]) From 03b0412756017027b3b869750677ed6f0044c6e7 Mon Sep 17 00:00:00 2001 From: Andrew Helsby Date: Wed, 2 Nov 2022 12:44:32 +0400 Subject: [PATCH 05/10] fix/unit-tests: Parametrize all flag type methods in open feature client Signed-off-by: Andrew Helsby Signed-off-by: Andrew Helsby --- tests/test_open_feature_client.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_open_feature_client.py b/tests/test_open_feature_client.py index 1ea6ae65..c2d05cc0 100644 --- a/tests/test_open_feature_client.py +++ b/tests/test_open_feature_client.py @@ -51,9 +51,9 @@ def test_should_get_flag_value_based_on_method_type( @pytest.mark.parametrize( "flag_type, default_value, get_method", ( - (bool, True, "get_boolean_detail"), - (str, "String", "get_string_detail"), - (Number, 100, "get_number_detail"), + (bool, True, "get_boolean_details"), + (str, "String", "get_string_details"), + (Number, 100, "get_number_details"), ( dict, { @@ -61,7 +61,7 @@ def test_should_get_flag_value_based_on_method_type( "Number": 2, "Boolean": True, }, - "get_object_detail", + "get_object_details", ), ), ) From 42aafc5ab6d11c7562ace9ddbf542c0b97cd961f Mon Sep 17 00:00:00 2001 From: Andrew Helsby Date: Wed, 2 Nov 2022 13:47:35 +0400 Subject: [PATCH 06/10] fix/unit-tests: Add fixture to clear global provider after each tests Signed-off-by: Andrew Helsby Signed-off-by: Andrew Helsby --- tests/conftest.py | 7 +++++++ tests/test_open_feature_api.py | 6 ------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 5e5b3b0c..2e6fec10 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -11,6 +11,13 @@ from open_feature.provider.no_op_provider import NoOpProvider +@pytest.fixture(autouse=True) +def clear_provider(): + yield + # Setting private provider back to None to ensure other tests aren't affected + _provider = None # noqa: F841 + + @pytest.fixture() def no_op_provider_client(): api.set_provider(NoOpProvider()) diff --git a/tests/test_open_feature_api.py b/tests/test_open_feature_api.py index c695d35c..c3bee46d 100644 --- a/tests/test_open_feature_api.py +++ b/tests/test_open_feature_api.py @@ -31,8 +31,6 @@ def test_should_return_open_feature_client_when_configured_correctly(): assert client.name == "No-op Provider" assert client.version == "1.0" assert isinstance(client.provider, NoOpProvider) - # Setting private provider back to None to ensure other tests aren't affected - _provider = None # noqa: F841 def test_should_try_set_provider_and_fail_if_none_provided(): @@ -44,8 +42,6 @@ def test_should_try_set_provider_and_fail_if_none_provided(): # Then assert ge.value.error_message == "No provider" assert ge.value.error_code == ErrorCode.GENERAL - # Setting private provider back to None to ensure other tests aren't affected - _provider = None # noqa: F841 def test_should_return_a_provider_if_setup_correctly(): @@ -58,5 +54,3 @@ def test_should_return_a_provider_if_setup_correctly(): # Then assert provider assert isinstance(provider, NoOpProvider) - # Setting private provider back to None to ensure other tests aren't affected - _provider = None # noqa: F841 From d8245fdcb037395bf866b44ec7bdef256b7c8479 Mon Sep 17 00:00:00 2001 From: Andrew Helsby Date: Wed, 2 Nov 2022 15:36:45 +0400 Subject: [PATCH 07/10] fix/unit-tests: Mock hooks in tests Signed-off-by: Andrew Helsby Signed-off-by: Andrew Helsby --- tests/conftest.py | 45 ------------------------------- tests/test_open_feature_client.py | 16 ++++++++--- 2 files changed, 12 insertions(+), 49 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 2e6fec10..5081675d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,13 +1,6 @@ import pytest from open_feature import open_feature_api as api -from open_feature.evaluation_context.evaluation_context import EvaluationContext -from open_feature.exception.exceptions import OpenFeatureError -from open_feature.flag_evaluation.error_code import ErrorCode -from open_feature.flag_evaluation.flag_evaluation_details import FlagEvaluationDetails -from open_feature.flag_evaluation.flag_type import FlagType -from open_feature.hooks.hook import Hook -from open_feature.hooks.hook_context import HookContext from open_feature.provider.no_op_provider import NoOpProvider @@ -22,41 +15,3 @@ def clear_provider(): def no_op_provider_client(): api.set_provider(NoOpProvider()) return api.get_client() - - -class TestExceptionHook(Hook): - def before(self, hook_context: HookContext, hints: dict) -> EvaluationContext: - pass - - def after( - self, hook_context: HookContext, details: FlagEvaluationDetails, hints: dict - ): - raise Exception("Generic exception raised") - - def error(self, hook_context: HookContext, exception: Exception, hints: dict): - pass - - def finally_after(self, hook_context: HookContext, hints: dict): - pass - - def supports_flag_value_type(self, flag_type: FlagType) -> bool: - return True - - -class TestOpenFeatureErrorHook(Hook): - def before(self, hook_context: HookContext, hints: dict) -> EvaluationContext: - pass - - def after( - self, hook_context: HookContext, details: FlagEvaluationDetails, hints: dict - ): - raise OpenFeatureError("error_message", ErrorCode.GENERAL) - - def error(self, hook_context: HookContext, exception: Exception, hints: dict): - pass - - def finally_after(self, hook_context: HookContext, hints: dict): - pass - - def supports_flag_value_type(self, flag_type: FlagType) -> bool: - return True diff --git a/tests/test_open_feature_client.py b/tests/test_open_feature_client.py index c2d05cc0..2031cf9d 100644 --- a/tests/test_open_feature_client.py +++ b/tests/test_open_feature_client.py @@ -1,14 +1,15 @@ from numbers import Number +from unittest.mock import MagicMock import pytest from open_feature import open_feature_api as api -from open_feature.exception.exceptions import GeneralError +from open_feature.exception.exceptions import GeneralError, OpenFeatureError from open_feature.flag_evaluation.error_code import ErrorCode from open_feature.flag_evaluation.reason import Reason +from open_feature.hooks.hook import Hook from open_feature.open_feature_client import OpenFeatureClient from open_feature.provider.no_op_provider import NoOpProvider -from tests.conftest import TestExceptionHook, TestOpenFeatureErrorHook def setup(): @@ -94,7 +95,9 @@ def test_should_raise_exception_when_invalid_flag_type_provided(): def test_should_handle_a_generic_exception_thrown_by_a_provider(no_op_provider_client): # Given - no_op_provider_client.add_hooks([TestExceptionHook()]) + exception_hook = MagicMock(spec=Hook) + exception_hook.after.side_effect = Exception("Generic exception raised") + no_op_provider_client.add_hooks([exception_hook]) # When flag_details = no_op_provider_client.get_boolean_details( flag_key="Key", default_value=True @@ -111,7 +114,12 @@ def test_should_handle_an_open_feature_exception_thrown_by_a_provider( no_op_provider_client, ): # Given - no_op_provider_client.add_hooks([TestOpenFeatureErrorHook()]) + exception_hook = MagicMock(spec=Hook) + exception_hook.after.side_effect = OpenFeatureError( + "error_message", ErrorCode.GENERAL + ) + no_op_provider_client.add_hooks([exception_hook]) + # When flag_details = no_op_provider_client.get_boolean_details( flag_key="Key", default_value=True From 27d2ad821c755db2194ff6347b8557df667abd6c Mon Sep 17 00:00:00 2001 From: Andrew Helsby Date: Wed, 2 Nov 2022 15:40:26 +0400 Subject: [PATCH 08/10] fix/unit-tests: Update docstring on clear_provider method Signed-off-by: Andrew Helsby Signed-off-by: Andrew Helsby --- tests/conftest.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index 5081675d..af8467ff 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -6,8 +6,11 @@ @pytest.fixture(autouse=True) def clear_provider(): + """ + For tests that use set_provider(), we need to clear the provider to avoid issues + in other tests. + """ yield - # Setting private provider back to None to ensure other tests aren't affected _provider = None # noqa: F841 From d12011c985d10de5806c99e6ce07e3eac794c442 Mon Sep 17 00:00:00 2001 From: Andrew Helsby Date: Wed, 2 Nov 2022 15:45:56 +0400 Subject: [PATCH 09/10] fix/unit-tests: remove setup method in client tests Signed-off-by: Andrew Helsby Signed-off-by: Andrew Helsby --- tests/test_open_feature_client.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/test_open_feature_client.py b/tests/test_open_feature_client.py index 2031cf9d..2c0b5750 100644 --- a/tests/test_open_feature_client.py +++ b/tests/test_open_feature_client.py @@ -3,19 +3,11 @@ import pytest -from open_feature import open_feature_api as api from open_feature.exception.exceptions import GeneralError, OpenFeatureError from open_feature.flag_evaluation.error_code import ErrorCode from open_feature.flag_evaluation.reason import Reason from open_feature.hooks.hook import Hook from open_feature.open_feature_client import OpenFeatureClient -from open_feature.provider.no_op_provider import NoOpProvider - - -def setup(): - api.set_provider(NoOpProvider()) - provider = api.get_provider() - assert isinstance(provider, NoOpProvider) @pytest.mark.parametrize( From f61604c640f7a6684a0cb66b78093a8ce85b4e05 Mon Sep 17 00:00:00 2001 From: Andrew Helsby Date: Wed, 2 Nov 2022 16:18:55 +0400 Subject: [PATCH 10/10] fix/unit-tests: Remove invalid flag type check in private method _create_provider_evaluation and move to public method Signed-off-by: Andrew Helsby Signed-off-by: Andrew Helsby --- tests/test_open_feature_client.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/tests/test_open_feature_client.py b/tests/test_open_feature_client.py index 2c0b5750..9e7fb5ee 100644 --- a/tests/test_open_feature_client.py +++ b/tests/test_open_feature_client.py @@ -3,11 +3,10 @@ import pytest -from open_feature.exception.exceptions import GeneralError, OpenFeatureError +from open_feature.exception.exceptions import OpenFeatureError from open_feature.flag_evaluation.error_code import ErrorCode from open_feature.flag_evaluation.reason import Reason from open_feature.hooks.hook import Hook -from open_feature.open_feature_client import OpenFeatureClient @pytest.mark.parametrize( @@ -72,17 +71,17 @@ def test_should_get_flag_detail_based_on_method_type( assert isinstance(flag.value, flag_type) -def test_should_raise_exception_when_invalid_flag_type_provided(): +def test_should_raise_exception_when_invalid_flag_type_provided(no_op_provider_client): # Given # When - with pytest.raises(GeneralError) as ge: - OpenFeatureClient("No provider", "1.0")._create_provider_evaluation( - flag_type=None, flag_key="Key", default_value=True - ) + flag = no_op_provider_client.evaluate_flag_details( + flag_type=None, flag_key="Key", default_value=True + ) # Then - assert ge.value - assert ge.value.error_message == "Unknown flag type" - assert ge.value.error_code == ErrorCode.GENERAL + assert flag.value + assert flag.error_message == "Unknown flag type" + assert flag.error_code == ErrorCode.GENERAL + assert flag.reason == Reason.ERROR def test_should_handle_a_generic_exception_thrown_by_a_provider(no_op_provider_client):