Skip to content

Commit ca1b248

Browse files
optimizely/user_profile.py -> Updated type for experiment_bucket_map.
tests/test_decision_service.py -> Fixed tests
1 parent 309223c commit ca1b248

File tree

2 files changed

+52
-31
lines changed

2 files changed

+52
-31
lines changed

optimizely/user_profile.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ class UserProfile:
4646
def __init__(
4747
self,
4848
user_id: str,
49-
experiment_bucket_map: Optional[dict[str, dict[str, Optional[str]]]] = None,
49+
experiment_bucket_map: Optional[dict[str, Decision]] = None,
5050
**kwargs: Any
5151
):
5252
self.user_id = user_id
@@ -131,7 +131,7 @@ def load_user_profile(self, reasons: Optional[list[str]]=[], error_handler: Opti
131131
reasons.append(message)
132132
except Exception as exception:
133133
message = reasons.append(str(exception))
134-
self.logger.error(message)
134+
self.logger.exception(f'Unable to retrieve user profile for user "{self.user_id}"as lookup failed.')
135135
# Todo: add error handler
136136
# error_handler.handle_error()
137137

tests/test_decision_service.py

Lines changed: 50 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -788,7 +788,7 @@ def test_get_variation__user_does_not_meet_audience_conditions(self):
788788

789789
def test_get_variation__user_profile_in_invalid_format(self):
790790
""" Test that get_variation handles invalid user profile gracefully. """
791-
791+
792792
user = optimizely_user_context.OptimizelyUserContext(optimizely_client=None,
793793
logger=None,
794794
user_id="test_user",
@@ -801,7 +801,7 @@ def test_get_variation__user_profile_in_invalid_format(self):
801801
"optimizely.decision_service.DecisionService.get_whitelisted_variation",
802802
return_value=[None, []],
803803
) as mock_get_whitelisted_variation, mock.patch(
804-
"optimizely.decision_service.DecisionService.get_stored_variation"
804+
"optimizely.decision_service.DecisionService.get_stored_variation", return_value=None,
805805
) as mock_get_stored_variation, mock.patch(
806806
"optimizely.helpers.audience.does_user_meet_audience_conditions", return_value=[True, []]
807807
) as mock_audience_check, mock.patch(
@@ -814,20 +814,33 @@ def test_get_variation__user_profile_in_invalid_format(self):
814814
"optimizely.user_profile.UserProfileService.save"
815815
) as mock_save:
816816
variation, _ = self.decision_service.get_variation(
817-
self.project_config, experiment, user, user_profile_tracker
817+
self.project_config,
818+
experiment,
819+
user,
820+
user_profile_tracker, # Pass the tracker
821+
[], # Empty reasons list
822+
None # No options
818823
)
819824
self.assertEqual(
820825
entities.Variation("111129", "variation"),
821826
variation,
822827
)
823-
828+
824829
# Assert that user is bucketed and new decision is stored
825830
mock_get_whitelisted_variation.assert_called_once_with(
826831
self.project_config, experiment, "test_user"
827832
)
828833
mock_lookup.assert_called_once_with("test_user")
834+
829835
# Stored decision is not consulted as user profile is invalid
830-
self.assertEqual(0, mock_get_stored_variation.call_count)
836+
mock_get_stored_variation.assert_called_once_with(
837+
self.project_config,
838+
experiment,
839+
user_profile_tracker.get_user_profile() # Get the actual UserProfile object
840+
)
841+
842+
# self.assertEqual(0, mock_get_stored_variation.call_count)
843+
831844
mock_audience_check.assert_called_once_with(
832845
self.project_config,
833846
experiment.get_audience_conditions_or_ids(),
@@ -842,12 +855,13 @@ def test_get_variation__user_profile_in_invalid_format(self):
842855
mock_bucket.assert_called_once_with(
843856
self.project_config, experiment, "test_user", "test_user"
844857
)
845-
mock_save.assert_called_once_with(
846-
{
847-
"user_id": "test_user",
848-
"experiment_bucket_map": {"111127": {"variation_id": "111129"}},
858+
mock_save.assert_called_once_with({
859+
"user_id": "test_user",
860+
"experiment_bucket_map": {
861+
"111127": mock.ANY
849862
}
850-
)
863+
})
864+
851865

852866
def test_get_variation__user_profile_lookup_fails(self):
853867
""" Test that get_variation acts gracefully when lookup fails. """
@@ -864,7 +878,8 @@ def test_get_variation__user_profile_lookup_fails(self):
864878
"optimizely.decision_service.DecisionService.get_whitelisted_variation",
865879
return_value=[None, []],
866880
) as mock_get_whitelisted_variation, mock.patch(
867-
"optimizely.decision_service.DecisionService.get_stored_variation"
881+
"optimizely.decision_service.DecisionService.get_stored_variation",
882+
return_value=None
868883
) as mock_get_stored_variation, mock.patch(
869884
"optimizely.helpers.audience.does_user_meet_audience_conditions", return_value=[True, []]
870885
) as mock_audience_check, mock.patch(
@@ -877,7 +892,7 @@ def test_get_variation__user_profile_lookup_fails(self):
877892
"optimizely.user_profile.UserProfileService.save"
878893
) as mock_save:
879894
variation, _ = self.decision_service.get_variation(
880-
self.project_config, experiment, user, user_profile_tracker
895+
self.project_config, experiment, user, user_profile_tracker, [], None
881896
)
882897
self.assertEqual(
883898
entities.Variation("111129", "variation"),
@@ -889,8 +904,7 @@ def test_get_variation__user_profile_lookup_fails(self):
889904
self.project_config, experiment, "test_user"
890905
)
891906
mock_lookup.assert_called_once_with("test_user")
892-
# Stored decision is not consulted as lookup failed
893-
self.assertEqual(0, mock_get_stored_variation.call_count)
907+
894908
mock_audience_check.assert_called_once_with(
895909
self.project_config,
896910
experiment.get_audience_conditions_or_ids(),
@@ -899,18 +913,20 @@ def test_get_variation__user_profile_lookup_fails(self):
899913
user,
900914
mock_decision_service_logging
901915
)
902-
mock_decision_service_logging.exception.assert_called_once_with(
903-
'Unable to retrieve user profile for user "test_user" as lookup failed.'
904-
)
916+
905917
mock_bucket.assert_called_once_with(
906918
self.project_config, experiment, "test_user", "test_user"
907919
)
908-
mock_save.assert_called_once_with(
909-
{
910-
"user_id": "test_user",
911-
"experiment_bucket_map": {"111127": {"variation_id": "111129"}},
920+
mock_save.assert_called_once_with({
921+
"user_id": "test_user",
922+
"experiment_bucket_map": {
923+
"111127": decision_service.Decision(
924+
experiment=None,
925+
variation=mock.ANY, # Don't care about the specific object instance
926+
source=None
927+
)
912928
}
913-
)
929+
})
914930

915931
def test_get_variation__user_profile_save_fails(self):
916932
""" Test that get_variation acts gracefully when save fails. """
@@ -928,7 +944,8 @@ def test_get_variation__user_profile_save_fails(self):
928944
"optimizely.decision_service.DecisionService.get_whitelisted_variation",
929945
return_value=[None, []],
930946
) as mock_get_whitelisted_variation, mock.patch(
931-
"optimizely.decision_service.DecisionService.get_stored_variation"
947+
"optimizely.decision_service.DecisionService.get_stored_variation",
948+
return_value=None
932949
) as mock_get_stored_variation, mock.patch(
933950
"optimizely.helpers.audience.does_user_meet_audience_conditions", return_value=[True, []]
934951
) as mock_audience_check, mock.patch(
@@ -953,7 +970,7 @@ def test_get_variation__user_profile_save_fails(self):
953970
self.project_config, experiment, "test_user"
954971
)
955972
mock_lookup.assert_called_once_with("test_user")
956-
self.assertEqual(0, mock_get_stored_variation.call_count)
973+
957974
mock_audience_check.assert_called_once_with(
958975
self.project_config,
959976
experiment.get_audience_conditions_or_ids(),
@@ -969,12 +986,16 @@ def test_get_variation__user_profile_save_fails(self):
969986
mock_bucket.assert_called_once_with(
970987
self.project_config, experiment, "test_user", "test_user"
971988
)
972-
mock_save.assert_called_once_with(
973-
{
974-
"user_id": "test_user",
975-
"experiment_bucket_map": {"111127": {"variation_id": "111129"}},
989+
mock_save.assert_called_once_with({
990+
"user_id": "test_user",
991+
"experiment_bucket_map": {
992+
"111127": decision_service.Decision(
993+
experiment=None,
994+
variation=mock.ANY,
995+
source=None
996+
)
976997
}
977-
)
998+
})
978999

9791000
def test_get_variation__ignore_user_profile_when_specified(self):
9801001
""" Test that we ignore the user profile service if specified. """

0 commit comments

Comments
 (0)