Skip to content

Commit c1cd97a

Browse files
FarhanAnjum-optiFarhanAnjum-opti
committed
update: enhance CMAB decision handling and add related tests
1 parent a5e4993 commit c1cd97a

File tree

3 files changed

+274
-2
lines changed

3 files changed

+274
-2
lines changed

‎optimizely/decision_service.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,8 @@ def get_variation(
417417
options)
418418
decide_reasons += decision_variation_value.get('reasons', [])
419419
cmab_decision = decision_variation_value.get('result')
420-
if not cmab_decision:
420+
if not cmab_decision or decision_variation_value['error']:
421+
self.logger.error(Errors.CMAB_FETCH_FAILED.format(decide_reasons[0]))
421422
return None, decide_reasons, cmab_uuid
422423
variation_id = cmab_decision['variation_id']
423424
cmab_uuid = cmab_decision['cmab_uuid']

‎tests/test_decision_service.py‎

Lines changed: 270 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -750,6 +750,276 @@ def test_get_variation__ignore_user_profile_when_specified(self):
750750
self.assertEqual(0, mock_lookup.call_count)
751751
self.assertEqual(0, mock_save.call_count)
752752

753+
def test_get_variation_cmab_experiment_user_in_traffic_allocation(self):
754+
"""Test get_variation with CMAB experiment where user is in traffic allocation."""
755+
756+
# Create a user context
757+
user = optimizely_user_context.OptimizelyUserContext(
758+
optimizely_client=None,
759+
logger=None,
760+
user_id="test_user",
761+
user_attributes={}
762+
)
763+
764+
# Create a CMAB experiment
765+
cmab_experiment = entities.Experiment(
766+
'111150',
767+
'cmab_experiment',
768+
'Running',
769+
'111150',
770+
[], # No audience IDs
771+
{},
772+
[
773+
entities.Variation('111151', 'variation_1'),
774+
entities.Variation('111152', 'variation_2')
775+
],
776+
[
777+
{'entityId': '111151', 'endOfRange': 5000},
778+
{'entityId': '111152', 'endOfRange': 10000}
779+
],
780+
cmab={'trafficAllocation': 5000}
781+
)
782+
783+
cmab_decision = {
784+
'variation_id': '111151',
785+
'cmab_uuid': 'test-cmab-uuid-123'
786+
}
787+
788+
with mock.patch('optimizely.helpers.experiment.is_experiment_running', return_value=True), \
789+
mock.patch('optimizely.helpers.audience.does_user_meet_audience_conditions', return_value=[True, []]), \
790+
mock.patch('optimizely.bucketer.Bucketer.bucket_to_entity_id', return_value=['$', []]), \
791+
mock.patch('optimizely.decision_service.DecisionService._get_decision_for_cmab_experiment',
792+
return_value={'error': False, 'result': cmab_decision, 'reasons': []}), \
793+
mock.patch.object(self.project_config, 'get_variation_from_id',
794+
return_value=entities.Variation('111151', 'variation_1')), \
795+
mock.patch.object(self.decision_service,
796+
'logger') as mock_logger:
797+
798+
# Call get_variation with the CMAB experiment
799+
variation, reasons, cmab_uuid = self.decision_service.get_variation(
800+
self.project_config,
801+
cmab_experiment,
802+
user,
803+
None
804+
)
805+
806+
# Verify the variation and cmab_uuid
807+
self.assertEqual(entities.Variation('111151', 'variation_1'), variation)
808+
self.assertEqual('test-cmab-uuid-123', cmab_uuid)
809+
810+
# Verify logger was called
811+
mock_logger.info.assert_any_call('User "test_user" is in variation\
812+
"variation_1" of experiment cmab_experiment.')
813+
814+
def test_get_variation_cmab_experiment_user_not_in_traffic_allocation(self):
815+
"""Test get_variation with CMAB experiment where user is not in traffic allocation."""
816+
817+
# Create a user context
818+
user = optimizely_user_context.OptimizelyUserContext(
819+
optimizely_client=None,
820+
logger=None,
821+
user_id="test_user",
822+
user_attributes={}
823+
)
824+
825+
# Create a CMAB experiment
826+
cmab_experiment = entities.Experiment(
827+
'111150',
828+
'cmab_experiment',
829+
'Running',
830+
'111150',
831+
[], # No audience IDs
832+
{},
833+
[entities.Variation('111151', 'variation_1')],
834+
[{'entityId': '111151', 'endOfRange': 10000}],
835+
cmab={'trafficAllocation': 5000}
836+
)
837+
838+
with mock.patch('optimizely.helpers.experiment.is_experiment_running', return_value=True), \
839+
mock.patch('optimizely.helpers.audience.does_user_meet_audience_conditions', return_value=[True, []]), \
840+
mock.patch('optimizely.bucketer.Bucketer.bucket_to_entity_id', return_value=['not_in_allocation', []]), \
841+
mock.patch('optimizely.decision_service.DecisionService._get_decision_for_cmab_experiment'
842+
) as mock_cmab_decision, \
843+
mock.patch.object(self.decision_service,
844+
'logger') as mock_logger:
845+
846+
# Call get_variation with the CMAB experiment
847+
variation, reasons, cmab_uuid = self.decision_service.get_variation(
848+
self.project_config,
849+
cmab_experiment,
850+
user,
851+
None
852+
)
853+
854+
# Verify we get no variation and CMAB service wasn't called
855+
self.assertIsNone(variation)
856+
self.assertIsNone(cmab_uuid)
857+
mock_cmab_decision.assert_not_called()
858+
859+
# Verify logger was called
860+
mock_logger.info.assert_any_call('User "test_user" not in CMAB\
861+
experiment "cmab_experiment" due to traffic allocation.')
862+
863+
def test_get_variation_cmab_experiment_service_error(self):
864+
"""Test get_variation with CMAB experiment when the CMAB service returns an error."""
865+
866+
# Create a user context
867+
user = optimizely_user_context.OptimizelyUserContext(
868+
optimizely_client=None,
869+
logger=None,
870+
user_id="test_user",
871+
user_attributes={}
872+
)
873+
874+
# Create a CMAB experiment
875+
cmab_experiment = entities.Experiment(
876+
'111150',
877+
'cmab_experiment',
878+
'Running',
879+
'111150',
880+
[], # No audience IDs
881+
{},
882+
[entities.Variation('111151', 'variation_1')],
883+
[{'entityId': '111151', 'endOfRange': 10000}],
884+
cmab={'trafficAllocation': 5000}
885+
)
886+
887+
with mock.patch('optimizely.helpers.experiment.is_experiment_running', return_value=True), \
888+
mock.patch('optimizely.helpers.audience.does_user_meet_audience_conditions', return_value=[True, []]), \
889+
mock.patch('optimizely.bucketer.Bucketer.bucket_to_entity_id', return_value=['$', []]), \
890+
mock.patch('optimizely.decision_service.DecisionService._get_decision_for_cmab_experiment',
891+
return_value={'error': True, 'result': None, 'reasons': ['CMAB service error']}), \
892+
mock.patch.object(self.decision_service,
893+
'logger') as mock_logger:
894+
895+
# Call get_variation with the CMAB experiment
896+
variation, reasons, cmab_uuid = self.decision_service.get_variation(
897+
self.project_config,
898+
cmab_experiment,
899+
user,
900+
None
901+
)
902+
903+
# Verify we get no variation due to CMAB service error
904+
self.assertIsNone(variation)
905+
self.assertIsNone(cmab_uuid)
906+
self.assertIn('CMAB service error', reasons)
907+
908+
# Verify logger was called
909+
mock_logger.error.assert_any_call('CMAB decision fetch failed with status: CMAB service error')
910+
911+
def test_get_variation_cmab_experiment_forced_variation(self):
912+
"""Test get_variation with CMAB experiment when user has a forced variation."""
913+
914+
# Create a user context
915+
user = optimizely_user_context.OptimizelyUserContext(
916+
optimizely_client=None,
917+
logger=None,
918+
user_id="test_user",
919+
user_attributes={}
920+
)
921+
922+
# Create a CMAB experiment
923+
cmab_experiment = entities.Experiment(
924+
'111150',
925+
'cmab_experiment',
926+
'Running',
927+
'111150',
928+
[], # No audience IDs
929+
{},
930+
[
931+
entities.Variation('111151', 'variation_1'),
932+
entities.Variation('111152', 'variation_2')
933+
],
934+
[
935+
{'entityId': '111151', 'endOfRange': 5000},
936+
{'entityId': '111152', 'endOfRange': 10000}
937+
],
938+
cmab={'trafficAllocation': 5000}
939+
)
940+
941+
forced_variation = entities.Variation('111152', 'variation_2')
942+
943+
with mock.patch('optimizely.decision_service.DecisionService.get_forced_variation',
944+
return_value=[forced_variation, ['User is forced into variation']]), \
945+
mock.patch('optimizely.bucketer.Bucketer.bucket_to_entity_id') as mock_bucket, \
946+
mock.patch('optimizely.decision_service.DecisionService._get_decision_for_cmab_experiment'
947+
) as mock_cmab_decision:
948+
949+
# Call get_variation with the CMAB experiment
950+
variation, reasons, cmab_uuid = self.decision_service.get_variation(
951+
self.project_config,
952+
cmab_experiment,
953+
user,
954+
None
955+
)
956+
957+
# Verify we get the forced variation
958+
self.assertEqual(forced_variation, variation)
959+
self.assertIsNone(cmab_uuid)
960+
self.assertIn('User is forced into variation', reasons)
961+
962+
# Verify CMAB-specific methods weren't called
963+
mock_bucket.assert_not_called()
964+
mock_cmab_decision.assert_not_called()
965+
966+
def test_get_variation_cmab_experiment_with_whitelisted_variation(self):
967+
"""Test get_variation with CMAB experiment when user has a whitelisted variation."""
968+
969+
# Create a user context
970+
user = optimizely_user_context.OptimizelyUserContext(
971+
optimizely_client=None,
972+
logger=None,
973+
user_id="test_user",
974+
user_attributes={}
975+
)
976+
977+
# Create a CMAB experiment with forced variations
978+
cmab_experiment = entities.Experiment(
979+
'111150',
980+
'cmab_experiment',
981+
'Running',
982+
'111150',
983+
[], # No audience IDs
984+
{'test_user': 'variation_2'},
985+
[
986+
entities.Variation('111151', 'variation_1'),
987+
entities.Variation('111152', 'variation_2')
988+
],
989+
[
990+
{'entityId': '111151', 'endOfRange': 5000},
991+
{'entityId': '111152', 'endOfRange': 10000}
992+
],
993+
cmab={'trafficAllocation': 5000}
994+
)
995+
996+
whitelisted_variation = entities.Variation('111152', 'variation_2')
997+
998+
with mock.patch('optimizely.decision_service.DecisionService.get_forced_variation',
999+
return_value=[None, []]), \
1000+
mock.patch('optimizely.decision_service.DecisionService.get_whitelisted_variation',
1001+
return_value=[whitelisted_variation, ['User is whitelisted into variation']]), \
1002+
mock.patch('optimizely.bucketer.Bucketer.bucket_to_entity_id') as mock_bucket, \
1003+
mock.patch('optimizely.decision_service.DecisionService._get_decision_for_cmab_experiment'
1004+
) as mock_cmab_decision:
1005+
1006+
# Call get_variation with the CMAB experiment
1007+
variation, reasons, cmab_uuid = self.decision_service.get_variation(
1008+
self.project_config,
1009+
cmab_experiment,
1010+
user,
1011+
None
1012+
)
1013+
1014+
# Verify we get the whitelisted variation
1015+
self.assertEqual(whitelisted_variation, variation)
1016+
self.assertIsNone(cmab_uuid)
1017+
self.assertIn('User is whitelisted into variation', reasons)
1018+
1019+
# Verify CMAB-specific methods weren't called
1020+
mock_bucket.assert_not_called()
1021+
mock_cmab_decision.assert_not_called()
1022+
7531023

7541024
class FeatureFlagDecisionTests(base.BaseTest):
7551025
def setUp(self):

‎tests/test_optimizely.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -671,7 +671,8 @@ def on_activate(experiment, user_id, attributes, variation, event):
671671
with mock.patch(
672672
'optimizely.decision_service.DecisionService.get_variation_for_feature',
673673
return_value=(
674-
decision_service.Decision(mock_experiment, mock_variation, enums.DecisionSources.FEATURE_TEST, None), []),
674+
decision_service.Decision(mock_experiment, mock_variation,
675+
enums.DecisionSources.FEATURE_TEST, None), []),
675676
) as mock_decision, mock.patch('optimizely.event.event_processor.ForwardingEventProcessor.process'):
676677
self.assertTrue(opt_obj.is_feature_enabled('test_feature_in_experiment', 'test_user'))
677678

0 commit comments

Comments
 (0)