Revert "Moving around some code (#70)"

aliabbasrizvi · aliabbasrizvi · commit dde426a3c896 · 2017-12-18T17:12:00.000-08:00
This reverts commit 02eca8a.
diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py
@@ -291,7 +291,6 @@ def is_feature_enabled(self, feature_key, user_id, attributes=None):
     Returns:
       True if the feature is enabled for the user. False otherwise.
     """
-
     if not self.is_valid:
       self.logger.log(enums.LogLevels.ERROR, enums.Errors.INVALID_DATAFILE.format('is_feature_enabled'))
       return False
@@ -317,13 +316,13 @@ def get_enabled_features(self, user_id, attributes=None):
 
     Returns:
       A list of the keys of the features that are enabled for the user.
-    """
 
-    enabled_features = []
+    """
     if not self.is_valid:
       self.logger.log(enums.LogLevels.ERROR, enums.Errors.INVALID_DATAFILE.format('get_enabled_features'))
-      return enabled_features
+      return False
 
+    enabled_features = []
     for feature in self.config.feature_key_map.values():
       if self.is_feature_enabled(feature.key, user_id, attributes):
         enabled_features.append(feature.key)
diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py
@@ -701,7 +701,7 @@ def test_activate__bucketer_returns_none(self):
   def test_activate__invalid_object(self):
     """ Test that activate logs error if Optimizely object is not created correctly. """
 
-    opt_obj = optimizely.Optimizely('invalid_datafile')
+    opt_obj = optimizely.Optimizely('invalid_file')
 
     with mock.patch('optimizely.logger.SimpleLogger.log') as mock_logging:
       self.assertIsNone(opt_obj.activate('test_experiment', 'test_user'))
@@ -1156,7 +1156,7 @@ def test_track__whitelisted_user_overrides_audience_check(self):
   def test_track__invalid_object(self):
     """ Test that track logs error if Optimizely object is not created correctly. """
 
-    opt_obj = optimizely.Optimizely('invalid_datafile')
+    opt_obj = optimizely.Optimizely('invalid_file')
 
     with mock.patch('optimizely.logger.SimpleLogger.log') as mock_logging:
       opt_obj.track('test_event', 'test_user')
@@ -1166,7 +1166,7 @@ def test_track__invalid_object(self):
   def test_get_variation__invalid_object(self):
     """ Test that get_variation logs error if Optimizely object is not created correctly. """
 
-    opt_obj = optimizely.Optimizely('invalid_datafile')
+    opt_obj = optimizely.Optimizely('invalid_file')
 
     with mock.patch('optimizely.logger.SimpleLogger.log') as mock_logging:
       self.assertIsNone(opt_obj.get_variation('test_experiment', 'test_user'))
@@ -1175,42 +1175,34 @@ def test_get_variation__invalid_object(self):
 
   def test_is_feature_enabled__returns_false_for_invalid_feature(self):
     """ Test that the feature is not enabled for the user if the provided feature key is invalid. """
+    optimizely_instance = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
+    project_config = optimizely_instance.config
 
-    opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
-
-    with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature') as mock_decision:
-      self.assertFalse(opt_obj.is_feature_enabled('invalid_feature', 'user1'))
+    with mock.patch(
+      'optimizely.decision_service.DecisionService.get_variation_for_feature'
+      ) as mock_decision:
+      self.assertFalse(optimizely_instance.is_feature_enabled('invalid_feature', 'user1'))
 
     self.assertFalse(mock_decision.called)
 
   def test_is_feature_enabled__returns_true_if_user_is_bucketed_into_a_variation(self):
     """ Test that the feature is not enabled for the user if the provided feature key is invalid. """
-
-    opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
-    project_config = opt_obj.config
+    optimizely_instance = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
+    project_config = optimizely_instance.config
     feature = project_config.get_feature_from_key('test_feature_1')
 
-    with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
-                    return_value=project_config.get_variation_from_id('test_experiment', '111129')) as mock_decision:
-      self.assertTrue(opt_obj.is_feature_enabled('test_feature_1', 'user1'))
+    with mock.patch(
+      'optimizely.decision_service.DecisionService.get_variation_for_feature',
+      return_value=project_config.get_variation_from_id('test_experiment', '111129')
+      ) as mock_decision:
+      self.assertTrue(optimizely_instance.is_feature_enabled('test_feature_1', 'user1'))
 
     mock_decision.assert_called_once_with(feature, 'user1', None)
 
-  def test_is_feature_enabled__invalid_object(self):
-    """ Test that is_feature_enabled returns False if Optimizely object is not valid. """
-
-    opt_obj = optimizely.Optimizely('invalid_file')
-
-    with mock.patch('optimizely.logger.SimpleLogger.log') as mock_logging:
-      self.assertFalse(opt_obj.is_feature_enabled('test_feature_1', 'user_1'))
-
-    mock_logging.assert_called_once_with(enums.LogLevels.ERROR,
-                                         'Datafile has invalid format. Failing "is_feature_enabled".')
-
   def test_get_enabled_features(self):
     """ Test that get_enabled_features only returns features that are enabled for the specified user. """
-
-    opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
+    optimizely_instance = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
+    project_config = optimizely_instance.config
 
     def side_effect(*args, **kwargs):
       feature_key = args[0]
@@ -1219,9 +1211,10 @@ def side_effect(*args, **kwargs):
 
       return False
 
-    with mock.patch('optimizely.optimizely.Optimizely.is_feature_enabled',
-                    side_effect=side_effect) as mock_is_feature_enabled:
-      received_features = opt_obj.get_enabled_features('user_1')
+    with mock.patch(
+      'optimizely.optimizely.Optimizely.is_feature_enabled',
+      side_effect=side_effect) as mock_is_feature_enabled:
+      received_features = optimizely_instance.get_enabled_features('user_1')
 
     expected_enabled_features = ['test_feature_1', 'test_feature_2']
     self.assertEqual(sorted(expected_enabled_features), sorted(received_features))
@@ -1230,17 +1223,6 @@ def side_effect(*args, **kwargs):
     mock_is_feature_enabled.assert_any_call('test_feature_in_group', 'user_1', None)
     mock_is_feature_enabled.assert_any_call('test_feature_in_experiment_and_rollout', 'user_1', None)
 
-  def test_get_enabled_features__invalid_object(self):
-    """ Test that get_enabled_features returns empty list if Optimizely object is not valid. """
-
-    opt_obj = optimizely.Optimizely('invalid_file')
-
-    with mock.patch('optimizely.logger.SimpleLogger.log') as mock_logging:
-      self.assertEqual([], opt_obj.get_enabled_features('user_1'))
-
-    mock_logging.assert_called_once_with(enums.LogLevels.ERROR,
-                                         'Datafile has invalid format. Failing "get_enabled_features".')
-
 
 class OptimizelyWithExceptionTest(base.BaseTest):
   def setUp(self):
