From 3992620c61ef75aac41d9827142f29b38fa788aa Mon Sep 17 00:00:00 2001 From: ozayr-zaviar Date: Fri, 23 Apr 2021 11:56:59 +0500 Subject: [PATCH 01/16] code chnages for get_variation_for_feature --- optimizely/decision_service.py | 27 +++++++-------------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index 52e9d02b..ae166e69 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -462,31 +462,18 @@ def get_variation_for_feature(self, project_config, feature, user_id, attributes decide_reasons = [] bucketing_id, reasons = self._get_bucketing_id(user_id, attributes) decide_reasons += reasons - # First check if the feature is in a mutex group - if feature.groupId: - group = project_config.get_group(feature.groupId) - if group: - experiment, reasons = self.get_experiment_in_group(project_config, group, bucketing_id) - decide_reasons += reasons - if experiment and experiment.id in feature.experimentIds: + + # Next check if the feature is being experimented on + if feature.experimentIds: + # the feature can only be associated with one experiment + for experiment in feature.experimentIds: + experiment = project_config.get_experiment_from_id(experiment) + if experiment: variation, variation_reasons = self.get_variation( project_config, experiment, user_id, attributes, ignore_user_profile) decide_reasons += variation_reasons if variation: return Decision(experiment, variation, enums.DecisionSources.FEATURE_TEST), decide_reasons - else: - self.logger.error(enums.Errors.INVALID_GROUP_ID.format('_get_variation_for_feature')) - - # Next check if the feature is being experimented on - elif feature.experimentIds: - # If an experiment is not in a group, then the feature can only be associated with one experiment - experiment = project_config.get_experiment_from_id(feature.experimentIds[0]) - if experiment: - variation, variation_reasons = self.get_variation( - project_config, experiment, user_id, attributes, ignore_user_profile) - decide_reasons += variation_reasons - if variation: - return Decision(experiment, variation, enums.DecisionSources.FEATURE_TEST), decide_reasons # Next check if user is part of a rollout if feature.rolloutId: From 62987b097e3e7df6ffe181fa69b3d0ade8b4f2e9 Mon Sep 17 00:00:00 2001 From: ozayr-zaviar Date: Mon, 26 Apr 2021 16:53:40 +0500 Subject: [PATCH 02/16] comments and testcases addressed --- optimizely/decision_service.py | 37 +--------- tests/test_decision_service.py | 131 ++++++++++----------------------- 2 files changed, 40 insertions(+), 128 deletions(-) diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index ae166e69..98060e8e 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -413,39 +413,6 @@ def get_variation_for_rollout(self, project_config, rollout, user_id, attributes return Decision(None, None, enums.DecisionSources.ROLLOUT), decide_reasons - def get_experiment_in_group(self, project_config, group, bucketing_id): - """ Determine which experiment in the group the user is bucketed into. - - Args: - project_config: Instance of ProjectConfig. - group: The group to bucket the user into. - bucketing_id: ID to be used for bucketing the user. - - Returns: - Experiment if the user is bucketed into an experiment in the specified group. None otherwise - and array of log messages representing decision making. - """ - decide_reasons = [] - experiment_id = self.bucketer.find_bucket( - project_config, bucketing_id, group.id, group.trafficAllocation) - if experiment_id: - experiment = project_config.get_experiment_from_id(experiment_id) - if experiment: - message = 'User with bucketing ID "%s" is in experiment %s of group %s.' % \ - (bucketing_id, experiment.key, group.id) - self.logger.info( - message - ) - decide_reasons.append(message) - return experiment, decide_reasons - message = 'User with bucketing ID "%s" is not in any experiments of group %s.' % (bucketing_id, group.id) - self.logger.info( - message - ) - decide_reasons.append(message) - - return None, decide_reasons - def get_variation_for_feature(self, project_config, feature, user_id, attributes=None, ignore_user_profile=False): """ Returns the experiment/variation the user is bucketed in for the given feature. @@ -463,9 +430,9 @@ def get_variation_for_feature(self, project_config, feature, user_id, attributes bucketing_id, reasons = self._get_bucketing_id(user_id, attributes) decide_reasons += reasons - # Next check if the feature is being experimented on + # Check if the feature flag is under an experiment and the the user is bucketed into one of these experiments if feature.experimentIds: - # the feature can only be associated with one experiment + # Evaluate each experiment ID and return the first bucketed experiment variation for experiment in feature.experimentIds: experiment = project_config.get_experiment_from_id(experiment) if experiment: diff --git a/tests/test_decision_service.py b/tests/test_decision_service.py index f4023d0a..0812203d 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -1309,41 +1309,26 @@ def test_get_variation_for_feature__returns_variation_if_user_not_in_experiment_ mock_decision_service_logging, ) - def test_get_variation_for_feature__returns_variation_for_feature_in_group(self): - """ Test that get_variation_for_feature returns the variation of - the experiment the user is bucketed in the feature's group. """ + def test_get_variation_for_feature__returns_none_for_user_not_in_experiment(self): + """ Test that get_variation_for_feature returns None for user not in the associated experiment. """ - feature = self.project_config.get_feature_from_key("test_feature_in_group") + feature = self.project_config.get_feature_from_key("test_feature_in_experiment") - expected_experiment = self.project_config.get_experiment_from_key("group_exp_1") - expected_variation = self.project_config.get_variation_from_id( - "group_exp_1", "28901" - ) with mock.patch( - "optimizely.decision_service.DecisionService.get_experiment_in_group", - return_value=(self.project_config.get_experiment_from_key("group_exp_1"), []), - ) as mock_get_experiment_in_group, mock.patch( "optimizely.decision_service.DecisionService.get_variation", - return_value=(expected_variation, []), + return_value=[None, []], ) as mock_decision: variation_received, _ = self.decision_service.get_variation_for_feature( self.project_config, feature, "test_user" ) self.assertEqual( - decision_service.Decision( - expected_experiment, - expected_variation, - enums.DecisionSources.FEATURE_TEST, - ), + decision_service.Decision(None, None, enums.DecisionSources.ROLLOUT), variation_received, ) - mock_get_experiment_in_group.assert_called_once_with( - self.project_config, self.project_config.get_group("19228"), 'test_user') - mock_decision.assert_called_once_with( self.project_config, - self.project_config.get_experiment_from_key("group_exp_1"), + self.project_config.get_experiment_from_key("test_experiment"), "test_user", None, False @@ -1351,16 +1336,14 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_group(self) def test_get_variation_for_feature__returns_none_for_user_not_in_group(self): """ Test that get_variation_for_feature returns None for - user not in group and the feature is not part of a rollout. """ + user not in group and the feature is not part of a rollout. """ feature = self.project_config.get_feature_from_key("test_feature_in_group") with mock.patch( - "optimizely.decision_service.DecisionService.get_experiment_in_group", - return_value=[None, []], - ) as mock_get_experiment_in_group, mock.patch( - "optimizely.decision_service.DecisionService.get_variation" - ) as mock_decision: + "optimizely.decision_service.DecisionService.get_variation", + return_value=[None, []], + ): variation_received, _ = self.decision_service.get_variation_for_feature( self.project_config, feature, "test_user" ) @@ -1369,65 +1352,50 @@ def test_get_variation_for_feature__returns_none_for_user_not_in_group(self): variation_received, ) - mock_get_experiment_in_group.assert_called_once_with( - self.project_config, self.project_config.get_group("19228"), "test_user") - - self.assertFalse(mock_decision.called) - - def test_get_variation_for_feature__returns_none_for_user_not_in_experiment(self): - """ Test that get_variation_for_feature returns None for user not in the associated experiment. """ + def test_get_variation_for_feature__returns_variation_for_feature_in_group(self): + """ Test that get_variation_for_feature returns the variation of + the experiment the user is bucketed in the feature's group. """ - feature = self.project_config.get_feature_from_key("test_feature_in_experiment") + feature = self.project_config.get_feature_from_key("test_feature_in_group") + expected_experiment = self.project_config.get_experiment_from_key("group_exp_1") + expected_variation = self.project_config.get_variation_from_id( + "group_exp_1", "28901" + ) with mock.patch( "optimizely.decision_service.DecisionService.get_variation", - return_value=[None, []], + return_value=(expected_variation, []), ) as mock_decision: variation_received, _ = self.decision_service.get_variation_for_feature( self.project_config, feature, "test_user" ) self.assertEqual( - decision_service.Decision(None, None, enums.DecisionSources.ROLLOUT), + decision_service.Decision( + expected_experiment, + expected_variation, + enums.DecisionSources.FEATURE_TEST, + ), variation_received, ) mock_decision.assert_called_once_with( self.project_config, - self.project_config.get_experiment_from_key("test_experiment"), + self.project_config.get_experiment_from_key("group_exp_1"), "test_user", None, False ) - def test_get_variation_for_feature__returns_none_for_invalid_group_id(self): - """ Test that get_variation_for_feature returns None for unknown group ID. """ - - feature = self.project_config.get_feature_from_key("test_feature_in_group") - feature.groupId = "aabbccdd" - - with self.mock_decision_logger as mock_decision_service_logging: - variation_received, _ = self.decision_service.get_variation_for_feature( - self.project_config, feature, "test_user" - ) - self.assertEqual( - decision_service.Decision(None, None, enums.DecisionSources.ROLLOUT), - variation_received, - ) - mock_decision_service_logging.error.assert_called_once_with( - enums.Errors.INVALID_GROUP_ID.format("_get_variation_for_feature") - ) - def test_get_variation_for_feature__returns_none_for_user_in_group_experiment_not_associated_with_feature( self, ): """ Test that if a user is in the mutex group but the experiment is - not targeting a feature, then None is returned. """ + not targeting a feature, then None is returned. """ feature = self.project_config.get_feature_from_key("test_feature_in_group") - with mock.patch( - "optimizely.decision_service.DecisionService.get_experiment_in_group", - return_value=[self.project_config.get_experiment_from_key("group_exp_2"), []], + "optimizely.decision_service.DecisionService.get_variation", + return_value=[None, []], ) as mock_decision: variation_received, _ = self.decision_service.get_variation_for_feature( self.project_config, feature, "test_user" @@ -1438,43 +1406,20 @@ def test_get_variation_for_feature__returns_none_for_user_in_group_experiment_no ) mock_decision.assert_called_once_with( - self.project_config, self.project_config.get_group("19228"), "test_user" + self.project_config, self.project_config.get_experiment_from_id("32222"), "test_user", None, False ) - def test_get_experiment_in_group(self): - """ Test that get_experiment_in_group returns the bucketed experiment for the user. """ + def test_get_variation_for_feature__returns_none_for_invalid_group_id(self): + """ Test that get_variation_for_feature returns None for unknown group ID. """ + + feature = self.project_config.get_feature_from_key("test_feature_in_group") + feature.groupId = "aabbccdd" - group = self.project_config.get_group("19228") - experiment = self.project_config.get_experiment_from_id("32222") - with mock.patch( - "optimizely.bucketer.Bucketer.find_bucket", return_value="32222" - ), self.mock_decision_logger as mock_decision_service_logging: - variation_received, _ = self.decision_service.get_experiment_in_group( - self.project_config, group, "test_user" + with self.mock_decision_logger as mock_decision_service_logging: + variation_received, _ = self.decision_service.get_variation_for_feature( + self.project_config, feature, "test_user" ) self.assertEqual( - experiment, + decision_service.Decision(None, None, enums.DecisionSources.ROLLOUT), variation_received, ) - - mock_decision_service_logging.info.assert_called_once_with( - 'User with bucketing ID "test_user" is in experiment group_exp_1 of group 19228.' - ) - - def test_get_experiment_in_group__returns_none_if_user_not_in_group(self): - """ Test that get_experiment_in_group returns None if the user is not bucketed into the group. """ - - group = self.project_config.get_group("19228") - with mock.patch( - "optimizely.bucketer.Bucketer.find_bucket", return_value=None - ), self.mock_decision_logger as mock_decision_service_logging: - variation_received, _ = self.decision_service.get_experiment_in_group( - self.project_config, group, "test_user" - ) - self.assertIsNone( - variation_received - ) - - mock_decision_service_logging.info.assert_called_once_with( - 'User with bucketing ID "test_user" is not in any experiments of group 19228.' - ) From 3655fc6f9d20cc1cbee654dbb68b70adf063f967 Mon Sep 17 00:00:00 2001 From: ozayr-zaviar Date: Mon, 26 Apr 2021 17:03:52 +0500 Subject: [PATCH 03/16] unused variable fixed --- tests/test_decision_service.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tests/test_decision_service.py b/tests/test_decision_service.py index 0812203d..74cef38f 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -1415,11 +1415,10 @@ def test_get_variation_for_feature__returns_none_for_invalid_group_id(self): feature = self.project_config.get_feature_from_key("test_feature_in_group") feature.groupId = "aabbccdd" - with self.mock_decision_logger as mock_decision_service_logging: - variation_received, _ = self.decision_service.get_variation_for_feature( - self.project_config, feature, "test_user" - ) - self.assertEqual( - decision_service.Decision(None, None, enums.DecisionSources.ROLLOUT), - variation_received, - ) + variation_received, _ = self.decision_service.get_variation_for_feature( + self.project_config, feature, "test_user" + ) + self.assertEqual( + decision_service.Decision(None, None, enums.DecisionSources.ROLLOUT), + variation_received, + ) From 10431d2e1001ffc8688cd0b3d6f53d3ea6180ede Mon Sep 17 00:00:00 2001 From: ozayr-zaviar Date: Tue, 27 Apr 2021 16:41:00 +0500 Subject: [PATCH 04/16] test cases order corrected --- tests/test_decision_service.py | 77 +++++++++++++++++----------------- 1 file changed, 38 insertions(+), 39 deletions(-) diff --git a/tests/test_decision_service.py b/tests/test_decision_service.py index 74cef38f..b3a5c068 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -1276,8 +1276,7 @@ def test_get_variation_for_feature__returns_variation_if_user_not_in_experiment_ "optimizely.helpers.audience.does_user_meet_audience_conditions", side_effect=[[False, []], [True, []]], ) as mock_audience_check, self.mock_decision_logger as mock_decision_service_logging, mock.patch( - "optimizely.bucketer.Bucketer.bucket", return_value=[expected_variation, []]): - + "optimizely.bucketer.Bucketer.bucket", return_value=[expected_variation, []]): decision, _ = self.decision_service.get_variation_for_feature( self.project_config, feature, "test_user" ) @@ -1309,26 +1308,35 @@ def test_get_variation_for_feature__returns_variation_if_user_not_in_experiment_ mock_decision_service_logging, ) - def test_get_variation_for_feature__returns_none_for_user_not_in_experiment(self): - """ Test that get_variation_for_feature returns None for user not in the associated experiment. """ + def test_get_variation_for_feature__returns_variation_for_feature_in_group(self): + """ Test that get_variation_for_feature returns the variation of + the experiment the user is bucketed in the feature's group. """ - feature = self.project_config.get_feature_from_key("test_feature_in_experiment") + feature = self.project_config.get_feature_from_key("test_feature_in_group") + expected_experiment = self.project_config.get_experiment_from_key("group_exp_1") + expected_variation = self.project_config.get_variation_from_id( + "group_exp_1", "28901" + ) with mock.patch( "optimizely.decision_service.DecisionService.get_variation", - return_value=[None, []], + return_value=(expected_variation, []), ) as mock_decision: variation_received, _ = self.decision_service.get_variation_for_feature( self.project_config, feature, "test_user" ) self.assertEqual( - decision_service.Decision(None, None, enums.DecisionSources.ROLLOUT), + decision_service.Decision( + expected_experiment, + expected_variation, + enums.DecisionSources.FEATURE_TEST, + ), variation_received, ) mock_decision.assert_called_once_with( self.project_config, - self.project_config.get_experiment_from_key("test_experiment"), + self.project_config.get_experiment_from_key("group_exp_1"), "test_user", None, False @@ -1336,7 +1344,7 @@ def test_get_variation_for_feature__returns_none_for_user_not_in_experiment(self def test_get_variation_for_feature__returns_none_for_user_not_in_group(self): """ Test that get_variation_for_feature returns None for - user not in group and the feature is not part of a rollout. """ + user not in group and the feature is not part of a rollout. """ feature = self.project_config.get_feature_from_key("test_feature_in_group") @@ -1352,45 +1360,50 @@ def test_get_variation_for_feature__returns_none_for_user_not_in_group(self): variation_received, ) - def test_get_variation_for_feature__returns_variation_for_feature_in_group(self): - """ Test that get_variation_for_feature returns the variation of - the experiment the user is bucketed in the feature's group. """ + def test_get_variation_for_feature__returns_none_for_user_not_in_experiment(self): + """ Test that get_variation_for_feature returns None for user not in the associated experiment. """ - feature = self.project_config.get_feature_from_key("test_feature_in_group") + feature = self.project_config.get_feature_from_key("test_feature_in_experiment") - expected_experiment = self.project_config.get_experiment_from_key("group_exp_1") - expected_variation = self.project_config.get_variation_from_id( - "group_exp_1", "28901" - ) with mock.patch( "optimizely.decision_service.DecisionService.get_variation", - return_value=(expected_variation, []), + return_value=[None, []], ) as mock_decision: variation_received, _ = self.decision_service.get_variation_for_feature( self.project_config, feature, "test_user" ) self.assertEqual( - decision_service.Decision( - expected_experiment, - expected_variation, - enums.DecisionSources.FEATURE_TEST, - ), + decision_service.Decision(None, None, enums.DecisionSources.ROLLOUT), variation_received, ) mock_decision.assert_called_once_with( self.project_config, - self.project_config.get_experiment_from_key("group_exp_1"), + self.project_config.get_experiment_from_key("test_experiment"), "test_user", None, False ) + def test_get_variation_for_feature__returns_none_for_invalid_group_id(self): + """ Test that get_variation_for_feature returns None for unknown group ID. """ + + feature = self.project_config.get_feature_from_key("test_feature_in_group") + feature.groupId = "aabbccdd" + + variation_received, _ = self.decision_service.get_variation_for_feature( + self.project_config, feature, "test_user" + ) + self.assertEqual( + decision_service.Decision(None, None, enums.DecisionSources.ROLLOUT), + variation_received, + ) + def test_get_variation_for_feature__returns_none_for_user_in_group_experiment_not_associated_with_feature( self, ): """ Test that if a user is in the mutex group but the experiment is - not targeting a feature, then None is returned. """ + not targeting a feature, then None is returned. """ feature = self.project_config.get_feature_from_key("test_feature_in_group") with mock.patch( @@ -1408,17 +1421,3 @@ def test_get_variation_for_feature__returns_none_for_user_in_group_experiment_no mock_decision.assert_called_once_with( self.project_config, self.project_config.get_experiment_from_id("32222"), "test_user", None, False ) - - def test_get_variation_for_feature__returns_none_for_invalid_group_id(self): - """ Test that get_variation_for_feature returns None for unknown group ID. """ - - feature = self.project_config.get_feature_from_key("test_feature_in_group") - feature.groupId = "aabbccdd" - - variation_received, _ = self.decision_service.get_variation_for_feature( - self.project_config, feature, "test_user" - ) - self.assertEqual( - decision_service.Decision(None, None, enums.DecisionSources.ROLLOUT), - variation_received, - ) From 8d69a0210bfc029c7f458c94e670091855128232 Mon Sep 17 00:00:00 2001 From: ozayr-zaviar Date: Tue, 27 Apr 2021 16:53:26 +0500 Subject: [PATCH 05/16] linting fixed --- tests/test_decision_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_decision_service.py b/tests/test_decision_service.py index b3a5c068..0697806f 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -1276,7 +1276,7 @@ def test_get_variation_for_feature__returns_variation_if_user_not_in_experiment_ "optimizely.helpers.audience.does_user_meet_audience_conditions", side_effect=[[False, []], [True, []]], ) as mock_audience_check, self.mock_decision_logger as mock_decision_service_logging, mock.patch( - "optimizely.bucketer.Bucketer.bucket", return_value=[expected_variation, []]): + "optimizely.bucketer.Bucketer.bucket", return_value=[expected_variation, []]): decision, _ = self.decision_service.get_variation_for_feature( self.project_config, feature, "test_user" ) From f4c9e7e95f1171774f07b0e555a5e46701bfd33c Mon Sep 17 00:00:00 2001 From: ozayr-zaviar Date: Thu, 29 Apr 2021 17:48:45 +0500 Subject: [PATCH 06/16] data entered for new test cases --- tests/base.py | 154 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 154 insertions(+) diff --git a/tests/base.py b/tests/base.py index 254be7c5..784225aa 100644 --- a/tests/base.py +++ b/tests/base.py @@ -196,6 +196,75 @@ def setUp(self, config_dict='config_dict'): }, ], }, + { + 'key': 'test_experiment3', + 'status': 'Running', + 'layerId': '6', + 'audienceIds': [], + 'id': '111134', + 'forcedVariations': {}, + 'trafficAllocation': [ + { + 'entityId': '222239', 'endOfRange': 2500, + 'entityId': '', 'endOfRange': 5000, + 'entityId': '', 'endOfRange': 7500, + 'entityId': '', 'endOfRange': 10000 + } + ], + 'variations': [ + { + 'id': '222239', + 'key': 'control', + 'variables': [], + } + ], + }, + { + 'key': 'test_experiment4', + 'status': 'Running', + 'layerId': '7', + 'audienceIds': [], + 'id': '111135', + 'forcedVariations': {}, + 'trafficAllocation': [ + { + 'entityId': '', 'endOfRange': 2500, + 'entityId': '222240', 'endOfRange': 5000, + 'entityId': '', 'endOfRange': 7500, + 'entityId': '', 'endOfRange': 10000 + } + ], + 'variations': [ + { + 'id': '222240', + 'key': 'control', + 'variables': [], + } + ], + }, + { + 'key': 'test_experiment5', + 'status': 'Running', + 'layerId': '8', + 'audienceIds': [], + 'id': '111136', + 'forcedVariations': {}, + 'trafficAllocation': [ + { + 'entityId': '', 'endOfRange': 2500, + 'entityId': '', 'endOfRange': 5000, + 'entityId': '222241', 'endOfRange': 7500, + 'entityId': '', 'endOfRange': 10000 + } + ], + 'variations': [ + { + 'id': '222241', + 'key': 'control', + 'variables': [], + } + ], + }, ], 'groups': [ { @@ -239,6 +308,72 @@ def setUp(self, config_dict='config_dict'): {'entityId': '32222', "endOfRange": 3000}, {'entityId': '32223', 'endOfRange': 7500}, ], + }, + { + 'id': '19229', + 'policy': 'random', + 'experiments': [ + { + 'id': '42222', + 'key': 'group_2_exp_1', + 'status': 'Running', + "audienceConditions": [ + "or", + "11160" + ], + 'audienceIds': ['11160'], + 'layerId': '211183', + 'variations': [ + {'key': 'var_1', 'id': '38901'}, + ], + 'forcedVariations': {}, + 'trafficAllocation': [ + {'entityId': '38901', 'endOfRange': 10000} + ], + }, + { + 'id': '42223', + 'key': 'group_exp_2', + 'status': 'Running', + "audienceConditions": [ + "or", + "11160" + ], + 'audienceIds': ['11160'], + 'layerId': '211184', + 'variations': [ + {'key': 'var_1', 'id': '38905'} + ], + 'forcedVariations': {}, + 'trafficAllocation': [ + {'entityId': '38905', 'endOfRange': 10000} + ], + }, + { + 'id': '42224', + 'key': 'group_exp_2', + 'status': 'Running', + "audienceConditions": [ + "or", + "11160" + ], + 'audienceIds': ['11160'], + 'layerId': '211185', + 'variations': [ + {'key': 'var_1', 'id': '38906'} + ], + 'forcedVariations': {}, + 'trafficAllocation': [ + {'entityId': '38906', 'endOfRange': 10000} + ], + } + ], + 'trafficAllocation': [ + {'entityId': '42222', "endOfRange": 2500}, + {'entityId': '42223', 'endOfRange': 5000}, + {'entityId': '42224', "endOfRange": 7500}, + {'entityId': '', 'endOfRange': 10000}, + ], } ], 'attributes': [{'key': 'test_attribute', 'id': '111094'}], @@ -255,6 +390,11 @@ def setUp(self, config_dict='config_dict'): '{"name": "test_attribute", "type": "custom_attribute", "value": "test_value_2"}]]]', 'id': '11159', }, + { + 'name': 'Test attribute users 3', + 'conditions': "[\"and\", [\"or\", [\"or\", {\"match\": \"exact\", \"name\": \"experiment_attr\", \"type\": \"custom_attribute\", \"value\": \"group_experiment\"}]]]", + 'id': '11160', + } ], 'rollouts': [ {'id': '201111', 'experiments': []}, @@ -364,6 +504,20 @@ def setUp(self, config_dict='config_dict'): 'rolloutId': '211111', 'variables': [], }, + { + 'id': '91115', + 'key': 'test_feature_in_exclusion_group', + 'experimentIds': ['42222', '42223', '42224'], + 'rolloutId': '211111', + 'variables': [], + }, + { + 'id': '91116', + 'key': 'test_feature_in_multiple_groups', + 'experimentIds': ['111134', '111135', '111136'], + 'rolloutId': '211111', + 'variables': [], + }, ], } From 5ef8e268e4f53562f69562521c36c6d464c84c02 Mon Sep 17 00:00:00 2001 From: ozayr-zaviar Date: Thu, 29 Apr 2021 18:01:14 +0500 Subject: [PATCH 07/16] linting fixed --- tests/base.py | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/tests/base.py b/tests/base.py index 784225aa..ecb532ad 100644 --- a/tests/base.py +++ b/tests/base.py @@ -204,12 +204,10 @@ def setUp(self, config_dict='config_dict'): 'id': '111134', 'forcedVariations': {}, 'trafficAllocation': [ - { - 'entityId': '222239', 'endOfRange': 2500, - 'entityId': '', 'endOfRange': 5000, - 'entityId': '', 'endOfRange': 7500, - 'entityId': '', 'endOfRange': 10000 - } + {'entityId': '222239', 'endOfRange': 2500} + {'entityId': '', 'endOfRange': 5000}, + {'entityId': '', 'endOfRange': 7500}, + {'entityId': '', 'endOfRange': 10000} ], 'variations': [ { @@ -227,12 +225,10 @@ def setUp(self, config_dict='config_dict'): 'id': '111135', 'forcedVariations': {}, 'trafficAllocation': [ - { - 'entityId': '', 'endOfRange': 2500, - 'entityId': '222240', 'endOfRange': 5000, - 'entityId': '', 'endOfRange': 7500, - 'entityId': '', 'endOfRange': 10000 - } + {'entityId': '', 'endOfRange': 2500}, + {'entityId': '222240', 'endOfRange': 5000}, + {'entityId': '', 'endOfRange': 7500}, + {'entityId': '', 'endOfRange': 10000} ], 'variations': [ { @@ -250,12 +246,10 @@ def setUp(self, config_dict='config_dict'): 'id': '111136', 'forcedVariations': {}, 'trafficAllocation': [ - { - 'entityId': '', 'endOfRange': 2500, - 'entityId': '', 'endOfRange': 5000, - 'entityId': '222241', 'endOfRange': 7500, - 'entityId': '', 'endOfRange': 10000 - } + {'entityId': '', 'endOfRange': 2500}, + {'entityId': '', 'endOfRange': 5000}, + {'entityId': '222241', 'endOfRange': 7500}, + {'entityId': '', 'endOfRange': 10000} ], 'variations': [ { @@ -392,7 +386,8 @@ def setUp(self, config_dict='config_dict'): }, { 'name': 'Test attribute users 3', - 'conditions': "[\"and\", [\"or\", [\"or\", {\"match\": \"exact\", \"name\": \"experiment_attr\", \"type\": \"custom_attribute\", \"value\": \"group_experiment\"}]]]", + 'conditions': "[\"and\", [\"or\", [\"or\", {\"match\": \"exact\", \"name\": \ + \"experiment_attr\", \"type\": \"custom_attribute\", \"value\": \"group_experiment\"}]]]", 'id': '11160', } ], From cd59fa824c9ecfb1c530d7541a55f67cf70ba1f8 Mon Sep 17 00:00:00 2001 From: ozayr-zaviar Date: Thu, 29 Apr 2021 18:05:27 +0500 Subject: [PATCH 08/16] fixes --- tests/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/base.py b/tests/base.py index ecb532ad..0efc8c1c 100644 --- a/tests/base.py +++ b/tests/base.py @@ -204,7 +204,7 @@ def setUp(self, config_dict='config_dict'): 'id': '111134', 'forcedVariations': {}, 'trafficAllocation': [ - {'entityId': '222239', 'endOfRange': 2500} + {'entityId': '222239', 'endOfRange': 2500}, {'entityId': '', 'endOfRange': 5000}, {'entityId': '', 'endOfRange': 7500}, {'entityId': '', 'endOfRange': 10000} From 569720adaf0e2b2ff88c295fbafeb3bc7a548f86 Mon Sep 17 00:00:00 2001 From: ozayr-zaviar Date: Thu, 29 Apr 2021 18:20:35 +0500 Subject: [PATCH 09/16] test case in user context fixed --- tests/test_user_context.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_user_context.py b/tests/test_user_context.py index abc18a87..700d4a92 100644 --- a/tests/test_user_context.py +++ b/tests/test_user_context.py @@ -1040,7 +1040,9 @@ def test_decide_for_all(self): 'test_feature_in_experiment', 'test_feature_in_rollout', 'test_feature_in_group', - 'test_feature_in_experiment_and_rollout' + 'test_feature_in_experiment_and_rollout', + 'test_feature_in_exclusion_group', + 'test_feature_in_multiple_groups' ], options ) From 8fb6b73644f6746dfc9c1e7960351c5c0d016d59 Mon Sep 17 00:00:00 2001 From: ozayr-zaviar Date: Thu, 29 Apr 2021 20:44:39 +0500 Subject: [PATCH 10/16] config hardcoded feature experiment mapping fixed --- tests/base.py | 4 +- tests/test_optimizely_config.py | 186 ++++++++++++++++++++++++++++++++ 2 files changed, 188 insertions(+), 2 deletions(-) diff --git a/tests/base.py b/tests/base.py index 0efc8c1c..8cb2f2dc 100644 --- a/tests/base.py +++ b/tests/base.py @@ -327,7 +327,7 @@ def setUp(self, config_dict='config_dict'): }, { 'id': '42223', - 'key': 'group_exp_2', + 'key': 'group_2_exp_2', 'status': 'Running', "audienceConditions": [ "or", @@ -345,7 +345,7 @@ def setUp(self, config_dict='config_dict'): }, { 'id': '42224', - 'key': 'group_exp_2', + 'key': 'group_2_exp_3', 'status': 'Running', "audienceConditions": [ "or", diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index 695cdc91..3d69f582 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -196,6 +196,90 @@ def setUp(self): }, 'id': '32223', 'key': 'group_exp_2' + }, + 'group_2_exp_1': { + 'variations_map': { + 'var_1': { + 'variables_map': { + + }, + 'id': '38901', + 'key': 'var_1', + 'feature_enabled': None + }, + }, + 'id': '42222', + 'key': 'group_2_exp_1' + }, + 'group_2_exp_2': { + 'variations_map': { + 'var_1': { + 'variables_map': { + + }, + 'id': '38905', + 'key': 'var_1', + 'feature_enabled': None + }, + }, + 'id': '42223', + 'key': 'group_2_exp_2' + }, + 'group_2_exp_3': { + 'variations_map': { + 'var_1': { + 'variables_map': { + + }, + 'id': '38906', + 'key': 'var_1', + 'feature_enabled': None + }, + }, + 'id': '42224', + 'key': 'group_2_exp_3' + }, + 'test_experiment3': { + 'variations_map': { + 'control': { + 'variables_map': { + + }, + 'id': '222239', + 'key': 'control', + 'feature_enabled': None + }, + }, + 'id': '111134', + 'key': 'test_experiment3' + }, + 'test_experiment4': { + 'variations_map': { + 'control': { + 'variables_map': { + + }, + 'id': '222240', + 'key': 'control', + 'feature_enabled': None + }, + }, + 'id': '111135', + 'key': 'test_experiment4' + }, + 'test_experiment5': { + 'variations_map': { + 'control': { + 'variables_map': { + + }, + 'id': '222241', + 'key': 'control', + 'feature_enabled': None + }, + }, + 'id': '111136', + 'key': 'test_experiment5' } }, 'features_map': { @@ -453,6 +537,108 @@ def setUp(self): }, 'id': '91114', 'key': 'test_feature_in_experiment_and_rollout' + }, + 'test_feature_in_exclusion_group': { + 'variables_map': { + + }, + 'experiments_map': { + 'group_2_exp_1': { + 'variations_map': { + 'var_1': { + 'variables_map': { + + }, + 'id': '38901', + 'key': 'var_1', + 'feature_enabled': None + }, + }, + 'id': '42222', + 'key': 'group_2_exp_1' + }, + 'group_2_exp_2': { + 'variations_map': { + 'var_1': { + 'variables_map': { + + }, + 'id': '38905', + 'key': 'var_1', + 'feature_enabled': None + }, + }, + 'id': '42223', + 'key': 'group_2_exp_2' + }, + 'group_2_exp_3': { + 'variations_map': { + 'var_1': { + 'variables_map': { + + }, + 'id': '38906', + 'key': 'var_1', + 'feature_enabled': None + }, + }, + 'id': '42224', + 'key': 'group_2_exp_3' + } + }, + 'id': '91115', + 'key': 'test_feature_in_exclusion_group' + }, + 'test_feature_in_multiple_groups': { + 'variables_map': { + + }, + 'experiments_map': { + 'test_experiment3': { + 'variations_map': { + 'control': { + 'variables_map': { + + }, + 'id': '222239', + 'key': 'control', + 'feature_enabled': None + }, + }, + 'id': '111134', + 'key': 'test_experiment3' + }, + 'test_experiment4': { + 'variations_map': { + 'control': { + 'variables_map': { + + }, + 'id': '222240', + 'key': 'control', + 'feature_enabled': None + }, + }, + 'id': '111135', + 'key': 'test_experiment4' + }, + 'test_experiment5': { + 'variations_map': { + 'control': { + 'variables_map': { + + }, + 'id': '222241', + 'key': 'control', + 'feature_enabled': None + }, + }, + 'id': '111136', + 'key': 'test_experiment5' + } + }, + 'id': '91116', + 'key': 'test_feature_in_multiple_groups' } }, 'revision': '1', From 777e975e8b85afea3da3e3e84c40cbfc4139715f Mon Sep 17 00:00:00 2001 From: ozayr-zaviar Date: Fri, 30 Apr 2021 15:30:46 +0500 Subject: [PATCH 11/16] added testcases for group and experiments --- optimizely/project_config.py | 7 - tests/base.py | 2 +- tests/test_config.py | 2 +- tests/test_decision_service.py | 235 ++++++++++++++++++++++++++++++++ tests/test_optimizely_config.py | 4 +- tests/test_user_context.py | 2 +- 6 files changed, 240 insertions(+), 12 deletions(-) diff --git a/optimizely/project_config.py b/optimizely/project_config.py index 77b89e67..c0004495 100644 --- a/optimizely/project_config.py +++ b/optimizely/project_config.py @@ -128,13 +128,6 @@ def __init__(self, datafile, logger, error_handler): # Add this experiment in experiment-feature map. self.experiment_feature_map[exp_id] = [feature.id] - experiment_in_feature = self.experiment_id_map[exp_id] - # Check if any of the experiments are in a group and add the group id for faster bucketing later on - if experiment_in_feature.groupId: - feature.groupId = experiment_in_feature.groupId - # Experiments in feature can only belong to one mutex group - break - @staticmethod def _generate_key_map(entity_list, key, entity_class): """ Helper method to generate map from key to entity object for given list of dicts. diff --git a/tests/base.py b/tests/base.py index 8cb2f2dc..bf07b7b2 100644 --- a/tests/base.py +++ b/tests/base.py @@ -508,7 +508,7 @@ def setUp(self, config_dict='config_dict'): }, { 'id': '91116', - 'key': 'test_feature_in_multiple_groups', + 'key': 'test_feature_in_multiple_experiments', 'experimentIds': ['111134', '111135', '111136'], 'rolloutId': '211111', 'variables': [], diff --git a/tests/test_config.py b/tests/test_config.py index e8836471..4bf1f61c 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -501,7 +501,7 @@ def test_init__with_v4_datafile(self): '211111', {'number_of_projects': entities.Variable('131', 'number_of_projects', 'integer', '10')}, ), - 'test_feature_in_group': entities.FeatureFlag('91113', 'test_feature_in_group', ['32222'], '', {}, '19228'), + 'test_feature_in_group': entities.FeatureFlag('91113', 'test_feature_in_group', ['32222'], '', {}), } expected_rollout_id_map = { diff --git a/tests/test_decision_service.py b/tests/test_decision_service.py index 0697806f..31029a05 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -1421,3 +1421,238 @@ def test_get_variation_for_feature__returns_none_for_user_in_group_experiment_no mock_decision.assert_called_once_with( self.project_config, self.project_config.get_experiment_from_id("32222"), "test_user", None, False ) + + def test_get_variation_for_feature__returns_variation_for_feature_in_mutex_group_bucket_less_than_2500( + self, + ): + """ Test that if a user is in the mutex group and the user bucket value should be less than 2500.""" + + feature = self.project_config.get_feature_from_key("test_feature_in_exclusion_group") + expected_experiment = self.project_config.get_experiment_from_key("group_2_exp_1") + expected_variation = self.project_config.get_variation_from_id( + "group_2_exp_1", "38901" + ) + user_attr = {"experiment_attr": "group_experiment"} + with mock.patch( + 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=2400) as mock_generate_bucket_value,\ + mock.patch.object(self.project_config, 'logger') as mock_config_logging: + + variation_received, _ = self.decision_service.get_variation_for_feature( + self.project_config, feature, "test_user", user_attr + ) + + self.assertEqual( + decision_service.Decision( + expected_experiment, + expected_variation, + enums.DecisionSources.FEATURE_TEST, + ), + variation_received, + ) + + mock_config_logging.debug.assert_called_with('Assigned bucket 2400 to user with bucketing ID "test_user".') + mock_generate_bucket_value.assert_called_with('test_user42222') + + def test_get_variation_for_feature__returns_variation_for_feature_in_mutex_group_bucket_range_2500_5000( + self, + ): + """ Test that if a user is in the mutex group and the user bucket value should be equal to 2500 + or greater than 5000.""" + + feature = self.project_config.get_feature_from_key("test_feature_in_exclusion_group") + expected_experiment = self.project_config.get_experiment_from_key("group_2_exp_2") + expected_variation = self.project_config.get_variation_from_id( + "group_2_exp_2", "38905" + ) + user_attr = {"experiment_attr": "group_experiment"} + with mock.patch( + 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=4000) as mock_generate_bucket_value,\ + mock.patch.object(self.project_config, 'logger') as mock_config_logging: + + variation_received, _ = self.decision_service.get_variation_for_feature( + self.project_config, feature, "test_user", user_attr + ) + self.assertEqual( + decision_service.Decision( + expected_experiment, + expected_variation, + enums.DecisionSources.FEATURE_TEST, + ), + variation_received, + ) + mock_config_logging.debug.assert_called_with('Assigned bucket 4000 to user with bucketing ID "test_user".') + mock_generate_bucket_value.assert_called_with('test_user42223') + + def test_get_variation_for_feature__returns_variation_for_feature_in_mutex_group_bucket_range_5000_7500( + self, + ): + """ Test that if a user is in the mutex group and the user bucket value should be equal to 5000 + or greater than 7500.""" + + feature = self.project_config.get_feature_from_key("test_feature_in_exclusion_group") + expected_experiment = self.project_config.get_experiment_from_key("group_2_exp_3") + expected_variation = self.project_config.get_variation_from_id( + "group_2_exp_3", "38906" + ) + user_attr = {"experiment_attr": "group_experiment"} + + with mock.patch( + 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=6500) as mock_generate_bucket_value,\ + mock.patch.object(self.project_config, 'logger') as mock_config_logging: + + variation_received, _ = self.decision_service.get_variation_for_feature( + self.project_config, feature, "test_user", user_attr + ) + self.assertEqual( + decision_service.Decision( + expected_experiment, + expected_variation, + enums.DecisionSources.FEATURE_TEST, + ), + variation_received, + ) + mock_config_logging.debug.assert_called_with('Assigned bucket 6500 to user with bucketing ID "test_user".') + mock_generate_bucket_value.assert_called_with('test_user42224') + + def test_get_variation_for_feature__returns_variation_for_rollout_in_mutex_group_bucket_greater_than_7500( + self, + ): + """ Test that if a user is in the mutex group and the user bucket value should be greater than 7500.""" + + feature = self.project_config.get_feature_from_key("test_feature_in_exclusion_group") + user_attr = {"experiment_attr": "group_experiment"} + with mock.patch( + 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=8000) as mock_generate_bucket_value,\ + mock.patch.object(self.project_config, 'logger') as mock_config_logging: + + variation_received, _ = self.decision_service.get_variation_for_feature( + self.project_config, feature, "test_user", user_attr + ) + self.assertEqual( + decision_service.Decision( + None, + None, + enums.DecisionSources.ROLLOUT, + ), + variation_received, + ) + + mock_generate_bucket_value.assert_called_with('test_user211147') + mock_config_logging.debug.assert_called_with('Assigned bucket 8000 to user with bucketing ID "test_user".') + + def test_get_variation_for_feature__returns_variation_for_feature_in_experiment_bucket_less_than_2500( + self, + ): + """ Test that if a user is in the non-mutex group and the user bucket value should be less than 2500.""" + + feature = self.project_config.get_feature_from_key("test_feature_in_multiple_experiments") + expected_experiment = self.project_config.get_experiment_from_key("test_experiment3") + expected_variation = self.project_config.get_variation_from_id( + "test_experiment3", "222239" + ) + user_attr = {"experiment_attr": "group_experiment"} + + with mock.patch( + 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=2400) as mock_generate_bucket_value,\ + mock.patch.object(self.project_config, 'logger') as mock_config_logging: + + variation_received, _ = self.decision_service.get_variation_for_feature( + self.project_config, feature, "test_user", user_attr + ) + self.assertEqual( + decision_service.Decision( + expected_experiment, + expected_variation, + enums.DecisionSources.FEATURE_TEST, + ), + variation_received, + ) + mock_config_logging.debug.assert_called_with('Assigned bucket 2400 to user with bucketing ID "test_user".') + mock_generate_bucket_value.assert_called_with('test_user111134') + + def test_get_variation_for_feature__returns_variation_for_feature_in_experiment_bucket_range_2500_5000( + self, + ): + """ Test that if a user is in the non-mutex group and the user bucket value should be equal to 2500 + or greater than 5000.""" + + feature = self.project_config.get_feature_from_key("test_feature_in_multiple_experiments") + expected_experiment = self.project_config.get_experiment_from_key("test_experiment4") + expected_variation = self.project_config.get_variation_from_id( + "test_experiment4", "222240" + ) + user_attr = {"experiment_attr": "group_experiment"} + + with mock.patch( + 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=4000) as mock_generate_bucket_value,\ + mock.patch.object(self.project_config, 'logger') as mock_config_logging: + + variation_received, _ = self.decision_service.get_variation_for_feature( + self.project_config, feature, "test_user", user_attr + ) + self.assertEqual( + decision_service.Decision( + expected_experiment, + expected_variation, + enums.DecisionSources.FEATURE_TEST, + ), + variation_received, + ) + mock_config_logging.debug.assert_called_with('Assigned bucket 4000 to user with bucketing ID "test_user".') + mock_generate_bucket_value.assert_called_with('test_user111135') + + def test_get_variation_for_feature__returns_variation_for_feature_in_experiment_bucket_range_5000_7500( + self, + ): + """ Test that if a user is in the non-mutex group and the user bucket value should be equal to 5000 + or greater than 7500.""" + + feature = self.project_config.get_feature_from_key("test_feature_in_multiple_experiments") + expected_experiment = self.project_config.get_experiment_from_key("test_experiment5") + expected_variation = self.project_config.get_variation_from_id( + "test_experiment5", "222241" + ) + user_attr = {"experiment_attr": "group_experiment"} + + with mock.patch( + 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=6500) as mock_generate_bucket_value,\ + mock.patch.object(self.project_config, 'logger') as mock_config_logging: + + variation_received, _ = self.decision_service.get_variation_for_feature( + self.project_config, feature, "test_user", user_attr + ) + self.assertEqual( + decision_service.Decision( + expected_experiment, + expected_variation, + enums.DecisionSources.FEATURE_TEST, + ), + variation_received, + ) + mock_config_logging.debug.assert_called_with('Assigned bucket 6500 to user with bucketing ID "test_user".') + mock_generate_bucket_value.assert_called_with('test_user111136') + + def test_get_variation_for_feature__returns_variation_for_rollout_in_experiment_bucket_greater_than_7500( + self, + ): + """ Test that if a user is in the non-mutex group and the user bucket value should be greater than 7500.""" + + feature = self.project_config.get_feature_from_key("test_feature_in_multiple_experiments") + user_attr = {"experiment_attr": "group_experiment"} + with mock.patch( + 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=8000) as mock_generate_bucket_value, \ + mock.patch.object(self.project_config, 'logger') as mock_config_logging: + variation_received, _ = self.decision_service.get_variation_for_feature( + self.project_config, feature, "test_user", user_attr + ) + self.assertEqual( + decision_service.Decision( + None, + None, + enums.DecisionSources.ROLLOUT, + ), + variation_received, + ) + + mock_generate_bucket_value.assert_called_with('test_user211147') + mock_config_logging.debug.assert_called_with('Assigned bucket 8000 to user with bucketing ID "test_user".') diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index 3d69f582..94e1fb00 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -589,7 +589,7 @@ def setUp(self): 'id': '91115', 'key': 'test_feature_in_exclusion_group' }, - 'test_feature_in_multiple_groups': { + 'test_feature_in_multiple_experiments': { 'variables_map': { }, @@ -638,7 +638,7 @@ def setUp(self): } }, 'id': '91116', - 'key': 'test_feature_in_multiple_groups' + 'key': 'test_feature_in_multiple_experiments' } }, 'revision': '1', diff --git a/tests/test_user_context.py b/tests/test_user_context.py index 700d4a92..7c979028 100644 --- a/tests/test_user_context.py +++ b/tests/test_user_context.py @@ -1042,7 +1042,7 @@ def test_decide_for_all(self): 'test_feature_in_group', 'test_feature_in_experiment_and_rollout', 'test_feature_in_exclusion_group', - 'test_feature_in_multiple_groups' + 'test_feature_in_multiple_experiments' ], options ) From c2d6752ad5aaac01b2a03b1886be7d8b31fd137b Mon Sep 17 00:00:00 2001 From: ozayr-zaviar Date: Fri, 30 Apr 2021 15:39:16 +0500 Subject: [PATCH 12/16] indentation fixed --- tests/test_decision_service.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/test_decision_service.py b/tests/test_decision_service.py index 31029a05..9d250fa2 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -1434,7 +1434,7 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_mutex_group ) user_attr = {"experiment_attr": "group_experiment"} with mock.patch( - 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=2400) as mock_generate_bucket_value,\ + 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=2400) as mock_generate_bucket_value,\ mock.patch.object(self.project_config, 'logger') as mock_config_logging: variation_received, _ = self.decision_service.get_variation_for_feature( @@ -1466,7 +1466,7 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_mutex_group ) user_attr = {"experiment_attr": "group_experiment"} with mock.patch( - 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=4000) as mock_generate_bucket_value,\ + 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=4000) as mock_generate_bucket_value,\ mock.patch.object(self.project_config, 'logger') as mock_config_logging: variation_received, _ = self.decision_service.get_variation_for_feature( @@ -1497,7 +1497,7 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_mutex_group user_attr = {"experiment_attr": "group_experiment"} with mock.patch( - 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=6500) as mock_generate_bucket_value,\ + 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=6500) as mock_generate_bucket_value,\ mock.patch.object(self.project_config, 'logger') as mock_config_logging: variation_received, _ = self.decision_service.get_variation_for_feature( @@ -1522,7 +1522,7 @@ def test_get_variation_for_feature__returns_variation_for_rollout_in_mutex_group feature = self.project_config.get_feature_from_key("test_feature_in_exclusion_group") user_attr = {"experiment_attr": "group_experiment"} with mock.patch( - 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=8000) as mock_generate_bucket_value,\ + 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=8000) as mock_generate_bucket_value,\ mock.patch.object(self.project_config, 'logger') as mock_config_logging: variation_received, _ = self.decision_service.get_variation_for_feature( @@ -1553,7 +1553,7 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_experiment_ user_attr = {"experiment_attr": "group_experiment"} with mock.patch( - 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=2400) as mock_generate_bucket_value,\ + 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=2400) as mock_generate_bucket_value,\ mock.patch.object(self.project_config, 'logger') as mock_config_logging: variation_received, _ = self.decision_service.get_variation_for_feature( @@ -1584,7 +1584,7 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_experiment_ user_attr = {"experiment_attr": "group_experiment"} with mock.patch( - 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=4000) as mock_generate_bucket_value,\ + 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=4000) as mock_generate_bucket_value,\ mock.patch.object(self.project_config, 'logger') as mock_config_logging: variation_received, _ = self.decision_service.get_variation_for_feature( @@ -1615,7 +1615,7 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_experiment_ user_attr = {"experiment_attr": "group_experiment"} with mock.patch( - 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=6500) as mock_generate_bucket_value,\ + 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=6500) as mock_generate_bucket_value,\ mock.patch.object(self.project_config, 'logger') as mock_config_logging: variation_received, _ = self.decision_service.get_variation_for_feature( @@ -1640,7 +1640,7 @@ def test_get_variation_for_feature__returns_variation_for_rollout_in_experiment_ feature = self.project_config.get_feature_from_key("test_feature_in_multiple_experiments") user_attr = {"experiment_attr": "group_experiment"} with mock.patch( - 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=8000) as mock_generate_bucket_value, \ + 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=8000) as mock_generate_bucket_value, \ mock.patch.object(self.project_config, 'logger') as mock_config_logging: variation_received, _ = self.decision_service.get_variation_for_feature( self.project_config, feature, "test_user", user_attr From 790ee4093440caf03a5aea32545b873284cfd8b6 Mon Sep 17 00:00:00 2001 From: ozayr-zaviar Date: Fri, 30 Apr 2021 20:03:59 +0500 Subject: [PATCH 13/16] comments addressed --- tests/test_decision_service.py | 33 +-------------------------------- 1 file changed, 1 insertion(+), 32 deletions(-) diff --git a/tests/test_decision_service.py b/tests/test_decision_service.py index 9d250fa2..c597b94a 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -1277,6 +1277,7 @@ def test_get_variation_for_feature__returns_variation_if_user_not_in_experiment_ side_effect=[[False, []], [True, []]], ) as mock_audience_check, self.mock_decision_logger as mock_decision_service_logging, mock.patch( "optimizely.bucketer.Bucketer.bucket", return_value=[expected_variation, []]): + decision, _ = self.decision_service.get_variation_for_feature( self.project_config, feature, "test_user" ) @@ -1342,24 +1343,6 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_group(self) False ) - def test_get_variation_for_feature__returns_none_for_user_not_in_group(self): - """ Test that get_variation_for_feature returns None for - user not in group and the feature is not part of a rollout. """ - - feature = self.project_config.get_feature_from_key("test_feature_in_group") - - with mock.patch( - "optimizely.decision_service.DecisionService.get_variation", - return_value=[None, []], - ): - variation_received, _ = self.decision_service.get_variation_for_feature( - self.project_config, feature, "test_user" - ) - self.assertEqual( - decision_service.Decision(None, None, enums.DecisionSources.ROLLOUT), - variation_received, - ) - def test_get_variation_for_feature__returns_none_for_user_not_in_experiment(self): """ Test that get_variation_for_feature returns None for user not in the associated experiment. """ @@ -1385,20 +1368,6 @@ def test_get_variation_for_feature__returns_none_for_user_not_in_experiment(self False ) - def test_get_variation_for_feature__returns_none_for_invalid_group_id(self): - """ Test that get_variation_for_feature returns None for unknown group ID. """ - - feature = self.project_config.get_feature_from_key("test_feature_in_group") - feature.groupId = "aabbccdd" - - variation_received, _ = self.decision_service.get_variation_for_feature( - self.project_config, feature, "test_user" - ) - self.assertEqual( - decision_service.Decision(None, None, enums.DecisionSources.ROLLOUT), - variation_received, - ) - def test_get_variation_for_feature__returns_none_for_user_in_group_experiment_not_associated_with_feature( self, ): From bad76e7c3317a4c0cf0548200bc7578e1957d3bb Mon Sep 17 00:00:00 2001 From: ozayr-zaviar Date: Tue, 4 May 2021 00:12:33 +0500 Subject: [PATCH 14/16] empty traffic allocations removed before first non empty allocation --- tests/base.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/base.py b/tests/base.py index bf07b7b2..ba4cebd8 100644 --- a/tests/base.py +++ b/tests/base.py @@ -225,7 +225,6 @@ def setUp(self, config_dict='config_dict'): 'id': '111135', 'forcedVariations': {}, 'trafficAllocation': [ - {'entityId': '', 'endOfRange': 2500}, {'entityId': '222240', 'endOfRange': 5000}, {'entityId': '', 'endOfRange': 7500}, {'entityId': '', 'endOfRange': 10000} @@ -246,8 +245,6 @@ def setUp(self, config_dict='config_dict'): 'id': '111136', 'forcedVariations': {}, 'trafficAllocation': [ - {'entityId': '', 'endOfRange': 2500}, - {'entityId': '', 'endOfRange': 5000}, {'entityId': '222241', 'endOfRange': 7500}, {'entityId': '', 'endOfRange': 10000} ], From 804fdb609dcfd6348ac2a87b03a07814a96528ea Mon Sep 17 00:00:00 2001 From: ozayr-zaviar Date: Tue, 4 May 2021 17:25:13 +0500 Subject: [PATCH 15/16] comments addressed --- tests/base.py | 18 +++++++-- tests/test_decision_service.py | 69 ++++++++++++++++++++++++++++++++-- 2 files changed, 80 insertions(+), 7 deletions(-) diff --git a/tests/base.py b/tests/base.py index ba4cebd8..83506c8f 100644 --- a/tests/base.py +++ b/tests/base.py @@ -200,7 +200,11 @@ def setUp(self, config_dict='config_dict'): 'key': 'test_experiment3', 'status': 'Running', 'layerId': '6', - 'audienceIds': [], + "audienceConditions": [ + "or", + "11160" + ], + 'audienceIds': ['11160'], 'id': '111134', 'forcedVariations': {}, 'trafficAllocation': [ @@ -221,7 +225,11 @@ def setUp(self, config_dict='config_dict'): 'key': 'test_experiment4', 'status': 'Running', 'layerId': '7', - 'audienceIds': [], + "audienceConditions": [ + "or", + "11160" + ], + 'audienceIds': ['11160'], 'id': '111135', 'forcedVariations': {}, 'trafficAllocation': [ @@ -241,7 +249,11 @@ def setUp(self, config_dict='config_dict'): 'key': 'test_experiment5', 'status': 'Running', 'layerId': '8', - 'audienceIds': [], + "audienceConditions": [ + "or", + "11160" + ], + 'audienceIds': ['11160'], 'id': '111136', 'forcedVariations': {}, 'trafficAllocation': [ diff --git a/tests/test_decision_service.py b/tests/test_decision_service.py index c597b94a..2f417a73 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -1426,7 +1426,7 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_mutex_group self, ): """ Test that if a user is in the mutex group and the user bucket value should be equal to 2500 - or greater than 5000.""" + or less than 5000.""" feature = self.project_config.get_feature_from_key("test_feature_in_exclusion_group") expected_experiment = self.project_config.get_experiment_from_key("group_2_exp_2") @@ -1456,7 +1456,7 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_mutex_group self, ): """ Test that if a user is in the mutex group and the user bucket value should be equal to 5000 - or greater than 7500.""" + or less than 7500.""" feature = self.project_config.get_feature_from_key("test_feature_in_exclusion_group") expected_experiment = self.project_config.get_experiment_from_key("group_2_exp_3") @@ -1543,7 +1543,7 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_experiment_ self, ): """ Test that if a user is in the non-mutex group and the user bucket value should be equal to 2500 - or greater than 5000.""" + or less than 5000.""" feature = self.project_config.get_feature_from_key("test_feature_in_multiple_experiments") expected_experiment = self.project_config.get_experiment_from_key("test_experiment4") @@ -1574,7 +1574,7 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_experiment_ self, ): """ Test that if a user is in the non-mutex group and the user bucket value should be equal to 5000 - or greater than 7500.""" + or less than 7500.""" feature = self.project_config.get_feature_from_key("test_feature_in_multiple_experiments") expected_experiment = self.project_config.get_experiment_from_key("test_experiment5") @@ -1625,3 +1625,64 @@ def test_get_variation_for_feature__returns_variation_for_rollout_in_experiment_ mock_generate_bucket_value.assert_called_with('test_user211147') mock_config_logging.debug.assert_called_with('Assigned bucket 8000 to user with bucketing ID "test_user".') + + def test_get_variation_for_feature__returns_variation_for_rollout_in_mutex_group_audience_mismatch( + self, + ): + """ Test that if a user is in the mutex group and the user bucket value should be less than 2500 and + missing target by audience.""" + + feature = self.project_config.get_feature_from_key("test_feature_in_exclusion_group") + expected_experiment = self.project_config.get_experiment_from_id("211147") + expected_variation = self.project_config.get_variation_from_id( + "211147", "211149" + ) + user_attr = {"experiment_attr": "group_experiment_invalid"} + with mock.patch( + 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=2400) as mock_generate_bucket_value, \ + mock.patch.object(self.project_config, 'logger') as mock_config_logging: + variation_received, _ = self.decision_service.get_variation_for_feature( + self.project_config, feature, "test_user", user_attr + ) + + self.assertEqual( + decision_service.Decision( + expected_experiment, + expected_variation, + enums.DecisionSources.ROLLOUT, + ), + variation_received, + ) + + mock_config_logging.debug.assert_called_with('Assigned bucket 2400 to user with bucketing ID "test_user".') + mock_generate_bucket_value.assert_called_with('test_user211147') + + def test_get_variation_for_feature_returns_rollout_in_experiment_bucket_range_2500_5000_audience_mismatch( + self, + ): + """ Test that if a user is in the non-mutex group and the user bucket value should be equal to 2500 + or less than 5000 missing target by audience.""" + + feature = self.project_config.get_feature_from_key("test_feature_in_multiple_experiments") + expected_experiment = self.project_config.get_experiment_from_id("211147") + expected_variation = self.project_config.get_variation_from_id( + "211147", "211149" + ) + user_attr = {"experiment_attr": "group_experiment_invalid"} + + with mock.patch( + 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=4000) as mock_generate_bucket_value, \ + mock.patch.object(self.project_config, 'logger') as mock_config_logging: + variation_received, _ = self.decision_service.get_variation_for_feature( + self.project_config, feature, "test_user", user_attr + ) + self.assertEqual( + decision_service.Decision( + expected_experiment, + expected_variation, + enums.DecisionSources.ROLLOUT, + ), + variation_received, + ) + mock_config_logging.debug.assert_called_with('Assigned bucket 4000 to user with bucketing ID "test_user".') + mock_generate_bucket_value.assert_called_with('test_user211147') \ No newline at end of file From f0d9f60da5c3ea04c5981db3a159ef336cec51f2 Mon Sep 17 00:00:00 2001 From: ozayr-zaviar Date: Tue, 4 May 2021 18:10:36 +0500 Subject: [PATCH 16/16] newline added --- tests/test_decision_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_decision_service.py b/tests/test_decision_service.py index 2f417a73..97fefce7 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -1685,4 +1685,4 @@ def test_get_variation_for_feature_returns_rollout_in_experiment_bucket_range_25 variation_received, ) mock_config_logging.debug.assert_called_with('Assigned bucket 4000 to user with bucketing ID "test_user".') - mock_generate_bucket_value.assert_called_with('test_user211147') \ No newline at end of file + mock_generate_bucket_value.assert_called_with('test_user211147')