From a0dfabdcf0e6656b8add42a7bba35b77f7c8391f Mon Sep 17 00:00:00 2001 From: Fredrik Strand Oseberg Date: Wed, 29 Nov 2023 12:57:46 +0100 Subject: [PATCH] Fix/double execution result (#294) The change in this pull request proposes to remove the double call to Strategy.execute(). Co-authored-by: ClementDelgrange Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- UnleashClient/features/Feature.py | 12 ++++++------ tests/unit_tests/test_client.py | 23 +++++++++++++++++++++++ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/UnleashClient/features/Feature.py b/UnleashClient/features/Feature.py index 0687d17d..053bfa88 100644 --- a/UnleashClient/features/Feature.py +++ b/UnleashClient/features/Feature.py @@ -2,7 +2,7 @@ from typing import Dict, Optional, cast from UnleashClient.constants import DISABLED_VARIATION -from UnleashClient.strategies import EvaluationResult, Strategy +from UnleashClient.strategies import EvaluationResult from UnleashClient.utils import LOGGER from UnleashClient.variants import Variants @@ -127,11 +127,11 @@ def _get_evaluation_result( if self.enabled: try: if self.strategies: - enabled_strategy: Strategy = next( - (x for x in self.strategies if x.execute(context)), None - ) - if enabled_strategy is not None: - strategy_result = enabled_strategy.get_result(context) + for strategy in self.strategies: + r = strategy.get_result(context) + if r.enabled: + strategy_result = r + break else: # If no strategies are present, should default to true. This isn't possible via UI. diff --git a/tests/unit_tests/test_client.py b/tests/unit_tests/test_client.py index f2bcda83..657123c5 100644 --- a/tests/unit_tests/test_client.py +++ b/tests/unit_tests/test_client.py @@ -239,6 +239,29 @@ def test_uc_is_enabled(unleash_client): assert unleash_client.is_enabled("testFlag") +@responses.activate +def test_consistent_results(unleash_client): + responses.add(responses.POST, URL + REGISTER_URL, json={}, status=202) + responses.add( + responses.GET, URL + FEATURES_URL, json=MOCK_FEATURE_RESPONSE, status=200 + ) + responses.add(responses.POST, URL + METRICS_URL, json={}, status=202) + unleash_client.initialize_client() + + results = [unleash_client.is_enabled("testFlag2") for i in range(1000)] + true_count = results.count(True) + false_count = results.count(False) + + # Due to murmur hash variations on smaller datasets, we allow a 10% discrepancy + discrepancy = 100 # 10% of 1000 + assert ( + 500 - discrepancy <= true_count <= 500 + discrepancy + ), "True count is outside acceptable range" + assert ( + 500 - discrepancy <= false_count <= 500 + discrepancy + ), "False count is outside acceptable range" + + @responses.activate def test_uc_project(unleash_client_project): unleash_client = unleash_client_project