Improve code coverage (#114)

Author: oakbani

optimizely/project_config.py

@@ -575,9 +575,6 @@ def get_forced_variation(self, experiment_key, user_id):
       return None
 
     variation = self.get_variation_from_id(experiment_key, variation_id)
-    if not variation:
-      # The invalid variation ID will be logged inside this call.
-      return None
 
     self.logger.log(enums.LogLevels.DEBUG,
                     'Variation "%s" is mapped to experiment "%s" and user "%s" in the forced variation map'

tests/test_bucketing.py

@@ -1,4 +1,4 @@
-# Copyright 2016-2017, Optimizely
+# Copyright 2016-2018, Optimizely
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
 # You may obtain a copy of the License at
@@ -79,6 +79,18 @@ def test_bucket__invalid_experiment(self):
                                            'test_user',
                                            'test_user'))
 
+  def test_bucket__invalid_group(self):
+    """ Test that bucket returns None for unknown group. """
+
+    project_config = self.project_config
+    experiment = project_config.get_experiment_from_key('group_exp_1')
+    # Set invalid group ID for the experiment
+    experiment.groupId = 'aabbcc'
+
+    self.assertIsNone(self.bucketer.bucket(experiment,
+                                           'test_user',
+                                           'test_user'))
+
   def test_bucket__experiment_in_group(self):
     """ Test that for provided bucket values correct variation ID is returned. """
 

tests/test_config.py

@@ -887,6 +887,17 @@ def test_get_rollout_from_id__valid_rollout_id(self):
     }])
     self.assertEqual(expected_rollout, project_config.get_rollout_from_id('211111'))
 
+  def test_get_rollout_from_id__invalid_rollout_id(self):
+    """ Test that None is returned for an unknown Rollout ID """
+
+    opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features),
+                                    logger=logger.NoOpLogger())
+    project_config = opt_obj.config
+    with mock.patch('optimizely.logger.NoOpLogger.log') as mock_logging:
+      self.assertIsNone(project_config.get_rollout_from_id('aabbccdd'))
+
+    mock_logging.assert_called_once_with(enums.LogLevels.ERROR, 'Rollout with ID "aabbccdd" is not in datafile.')
+
   def test_get_variable_value_for_variation__returns_valid_value(self):
     """ Test that the right value is returned. """
     opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
@@ -971,6 +982,29 @@ def test_get_forced_variation__invalid_experiment_key(self):
     self.assertIsNone(self.project_config.get_forced_variation(None, 'test_user'))
     self.assertIsNone(self.project_config.get_forced_variation('', 'test_user'))
 
+  def test_get_forced_variation_with_none_set_for_user(self):
+    """ Test get_forced_variation when none set for user ID in forced variation map. """
+    self.project_config.forced_variation_map = {}
+    self.project_config.forced_variation_map['test_user'] = {}
+
+    with mock.patch('optimizely.logger.NoOpLogger.log') as mock_logging:
+      self.assertIsNone(self.project_config.get_forced_variation('test_experiment', 'test_user'))
+    mock_logging.assert_called_once_with(
+      enums.LogLevels.DEBUG,
+      'No experiment "test_experiment" mapped to user "test_user" in the forced variation map.')
+
+  def test_get_forced_variation_missing_variation_mapped_to_experiment(self):
+    """ Test get_forced_variation when no variation found against given experiment for the user. """
+    self.project_config.forced_variation_map = {}
+    self.project_config.forced_variation_map['test_user'] = {}
+    self.project_config.forced_variation_map['test_user']['test_experiment'] = None
+
+    with mock.patch('optimizely.logger.NoOpLogger.log') as mock_logging:
+      self.assertIsNone(self.project_config.get_forced_variation('test_experiment', 'test_user'))
+    mock_logging.assert_called_once_with(
+      enums.LogLevels.DEBUG,
+      'No variation mapped to experiment "test_experiment" in the forced variation map.')
+
   # set_forced_variation tests
   def test_set_forced_variation__invalid_user_id(self):
     """ Test invalid user IDs set fail to set a forced variation """
@@ -1017,6 +1051,28 @@ def test_set_forced_variation__multiple_sets(self):
     self.assertEqual(self.project_config.get_forced_variation('test_experiment', 'test_user_1').key, 'control')
     self.assertEqual(self.project_config.get_forced_variation('group_exp_1', 'test_user_1').key, 'group_exp_1_control')
 
+  def test_set_forced_variation_when_called_to_remove_forced_variation(self):
+    """ Test set_forced_variation when no variation is given. """
+    # Test case where both user and experiment are present in the forced variation map
+    self.project_config.forced_variation_map = {}
+    self.project_config.set_forced_variation('test_experiment', 'test_user', 'variation')
+
+    with mock.patch('optimizely.logger.NoOpLogger.log') as mock_logging:
+      self.assertTrue(self.project_config.set_forced_variation('test_experiment', 'test_user', None))
+    mock_logging.assert_called_once_with(
+      enums.LogLevels.DEBUG,
+      'Variation mapped to experiment "test_experiment" has been removed for user "test_user".')
+
+    # Test case where user is present in the forced variation map, but the given experiment isn't
+    self.project_config.forced_variation_map = {}
+    self.project_config.set_forced_variation('test_experiment', 'test_user', 'variation')
+
+    with mock.patch('optimizely.logger.NoOpLogger.log') as mock_logging:
+      self.assertTrue(self.project_config.set_forced_variation('group_exp_1', 'test_user', None))
+    mock_logging.assert_called_once_with(
+      enums.LogLevels.DEBUG,
+      'Nothing to remove. Variation mapped to experiment "group_exp_1" for user "test_user" does not exist.')
+
 
 class ConfigLoggingTest(base.BaseTest):
   def setUp(self):

tests/test_decision_service.py

@@ -1,4 +1,4 @@
-# Copyright 2017, Optimizely
+# Copyright 2017-2018, Optimizely
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
 # You may obtain a copy of the License at
@@ -437,11 +437,14 @@ def test_get_variation_for_rollout__skips_to_everyone_else_rule(self, mock_loggi
     for the experiment, then it skips to the Everyone Else rule. """
 
     rollout = self.project_config.get_rollout_from_id('211111')
+    everyone_else_exp = self.project_config.get_experiment_from_id('211147')
+    variation_to_mock = self.project_config.get_variation_from_id('211147', '211149')
 
     with mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=True) as mock_audience_check,\
-      mock.patch('optimizely.bucketer.Bucketer.bucket', return_value=None):
-      self.assertEqual(decision_service.Decision(None, None, decision_service.DECISION_SOURCE_ROLLOUT),
-                       self.decision_service.get_variation_for_rollout(rollout, 'test_user'))
+      mock.patch('optimizely.bucketer.Bucketer.bucket', side_effect=[None, variation_to_mock]):
+        self.assertEqual(
+          decision_service.Decision(everyone_else_exp, variation_to_mock, decision_service.DECISION_SOURCE_ROLLOUT),
+          self.decision_service.get_variation_for_rollout(rollout, 'test_user'))
 
     # Check that after first experiment, it skips to the last experiment to check
     self.assertEqual(
@@ -454,8 +457,9 @@ def test_get_variation_for_rollout__skips_to_everyone_else_rule(self, mock_loggi
     self.assertEqual(
       [mock.call(enums.LogLevels.DEBUG, 'User "test_user" meets conditions for targeting rule 1.'),
        mock.call(enums.LogLevels.DEBUG, 'User "test_user" is not in the traffic group for the targeting else. '
-                                        'Checking "Everyone Else" rule now.')
-       ], mock_logging.call_args_list)
+                                        'Checking "Everyone Else" rule now.'),
+       mock.call(enums.LogLevels.DEBUG, 'User "test_user" meets conditions for targeting rule "Everyone Else".')],
+       mock_logging.call_args_list)
 
   def test_get_variation_for_rollout__returns_none_for_user_not_in_rollout(self, mock_logging):
     """ Test that get_variation_for_rollout returns None for the user not in the associated rollout. """
@@ -594,6 +598,20 @@ def test_get_variation_for_feature__returns_none_for_user_not_in_experiment(self
       self.project_config.get_experiment_from_key('test_experiment'), 'test_user', None
     )
 
+  def test_get_variation_for_feature__returns_none_for_invalid_group_id(self, mock_logging):
+    """ 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'
+
+    self.assertEqual(decision_service.Decision(None,
+                                               None,
+                                               decision_service.DECISION_SOURCE_EXPERIMENT),
+                       self.decision_service.get_variation_for_feature(feature, 'test_user')
+                    )
+    mock_logging.assert_called_with(enums.LogLevels.ERROR,
+                                     enums.Errors.INVALID_GROUP_ID_ERROR.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. """

tests/test_optimizely.py

@@ -1096,6 +1096,19 @@ def test_track__experiment_not_running(self):
     mock_is_experiment_running.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'))
     self.assertEqual(0, mock_dispatch_event.call_count)
 
+  def test_track_invalid_event_key(self):
+    """ Test that track does not call dispatch_event when event does not exist. """
+
+    with mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch_event,\
+         mock.patch('optimizely.logger.NoOpLogger.log') as mock_logging:
+      self.optimizely.track('aabbcc_event', 'test_user')
+
+    self.assertEqual(0, mock_dispatch_event.call_count)
+    mock_logging.assert_called_with(
+      enums.LogLevels.INFO,
+      'Not tracking user "test_user" for event "aabbcc_event".'
+    )
+
   def test_track__whitelisted_user_overrides_audience_check(self):
     """ Test that track does not check for user in audience when user is in whitelist. """
 
@@ -1132,6 +1145,16 @@ def test_get_variation__invalid_object(self):
 
     mock_logging.assert_called_once_with(enums.LogLevels.ERROR, 'Datafile has invalid format. Failing "get_variation".')
 
+  def test_get_variation_invalid_experiment_key(self):
+    """ Test that get_variation retuns None when invalid experiment key is given. """
+    with mock.patch('optimizely.logger.NoOpLogger.log') as mock_logging:
+      self.optimizely.get_variation('aabbccdd', 'test_user', None)
+
+    mock_logging.assert_called_with(
+      enums.LogLevels.INFO,
+      'Experiment key "aabbccdd" is invalid. Not activating user "test_user".'
+    )
+
   def test_is_feature_enabled__returns_false_for_none_feature_key(self):
     """ Test that is_feature_enabled returns false if the provided feature key is None. """
 
@@ -1706,6 +1729,26 @@ def test_get_feature_variable__returns_none_if_type_mismatch(self):
       'Use correct API to retrieve value. Returning None.'
     )
 
+  def test_get_feature_variable__returns_none_if_unable_to_cast(self):
+    """ Test that get_feature_variable_* returns None if unable_to_cast_value """
+
+    opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
+    mock_experiment = opt_obj.config.get_experiment_from_key('test_experiment')
+    mock_variation = opt_obj.config.get_variation_from_id('test_experiment', '111129')
+    with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
+                    return_value=decision_service.Decision(mock_experiment,
+                                                           mock_variation,
+                                                           decision_service.DECISION_SOURCE_EXPERIMENT)), \
+         mock.patch('optimizely.project_config.ProjectConfig.get_typecast_value',
+                    side_effect=ValueError()),\
+         mock.patch('optimizely.logger.NoOpLogger.log') as mock_logger:
+      self.assertEqual(None, opt_obj.get_feature_variable_integer('test_feature_in_experiment', 'count', 'test_user'))
+
+    mock_logger.assert_called_with(
+      enums.LogLevels.ERROR,
+      'Unable to cast value. Returning None.'
+    )
+
 
 class OptimizelyWithExceptionTest(base.BaseTest):
   def setUp(self):