comments addressed

optimizely · May 18, 2021 · 5e13d7b · 5e13d7b
1 parent ab4ea05
commit 5e13d7b
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 59 deletions.
diff --git a/optimizely/optimizely_factory.py b/optimizely/optimizely_factory.py
@@ -29,44 +29,22 @@ class OptimizelyFactory(object):
     blocking_timeout = None
 
     @staticmethod
-    def set_batch_size(batch_size, logger=optimizely_logger.NoOpLogger().logger):
+    def set_batch_size(batch_size):
         """ Convenience method for setting the maximum number of events contained within a batch.
         Args:
           batch_size: Sets size of event_queue.
-          logger: Optional logger interface provides a log method to log messages.
          """
 
-        if not isinstance(batch_size, int) or isinstance(batch_size, bool):
-            logger.error('Batch size is invalid, setting to default batch size # {0}'.format(
-                BatchEventProcessor._DEFAULT_BATCH_SIZE))
-            return
-
-        if not batch_size > 0:
-            logger.error('Batch size is negative, setting to default batch size # {0}'.format(
-                BatchEventProcessor._DEFAULT_BATCH_SIZE))
-            return
-
         OptimizelyFactory.max_event_batch_size = batch_size
         return OptimizelyFactory.max_event_batch_size
 
     @staticmethod
-    def set_flush_interval(flush_interval, logger=optimizely_logger.NoOpLogger().logger):
+    def set_flush_interval(flush_interval):
         """ Convenience method for setting the maximum time interval in milliseconds between event dispatches.
         Args:
           flush_interval: Time interval between event dispatches.
-          logger: Optional logging interface provides a log method to log messages.
          """
 
-        if not isinstance(flush_interval, int) or isinstance(flush_interval, bool):
-            logger.error('Flush interval is invalid, setting to default flush interval # {0}'.format(
-                BatchEventProcessor._DEFAULT_FLUSH_INTERVAL))
-            return
-
-        if not flush_interval > 0:
-            logger.error('Flush interval is negative, setting to default flush interval # {0}'.format(
-                BatchEventProcessor._DEFAULT_FLUSH_INTERVAL))
-            return
-
         OptimizelyFactory.max_event_flush_interval = flush_interval
         return OptimizelyFactory.max_event_flush_interval
 

diff --git a/tests/test_optimizely_factory.py b/tests/test_optimizely_factory.py
@@ -127,38 +127,36 @@ def test_custom_instance__should_assign_passed_components_to_both_the_instance_a
         self.assertEqual(self.notification_center,
                          optimizely_instance.notification_center)
 
-    def test_max_event_batch_size__should_log_error_message_and_return_none_when_invalid(self, _):
-        self.assertEqual(OptimizelyFactory.set_batch_size([], self.mock_client_logger), None)
-        self.assertEqual(OptimizelyFactory.set_batch_size('test', self.mock_client_logger), None)
-        self.assertEqual(OptimizelyFactory.set_batch_size(5.5, self.mock_client_logger), None)
-        self.assertEqual(OptimizelyFactory.set_batch_size(None, self.mock_client_logger), None)
-        self.assertEqual(OptimizelyFactory.set_batch_size(False, self.mock_client_logger), None)
-        self.assertEqual(OptimizelyFactory.set_batch_size(True, self.mock_client_logger), None)
-        self.mock_client_logger.error.assert_called_with('Batch size is invalid, setting to default batch size # 10')
-
-        self.assertEqual(OptimizelyFactory.set_batch_size(0, self.mock_client_logger), None)
-        self.assertEqual(OptimizelyFactory.set_batch_size(-8, self.mock_client_logger), None)
-        self.mock_client_logger.error.assert_called_with('Batch size is negative, setting to default batch size # 10')
-
-    def test_max_event_batch_size__should_not_log_error_message_when_valid_batch_size(self, _):
-        self.assertEqual(OptimizelyFactory.set_batch_size(5, self.mock_client_logger), 5)
-        self.mock_client_logger.assert_not_called()
-
-    def test_max_event_flush_interval__should_log_error_message_and_return_none_when_invalid(self, _):
-        self.assertEqual(OptimizelyFactory.set_flush_interval([], self.mock_client_logger), None)
-        self.assertEqual(OptimizelyFactory.set_flush_interval('test', self.mock_client_logger), None)
-        self.assertEqual(OptimizelyFactory.set_flush_interval(5.5, self.mock_client_logger), None)
-        self.assertEqual(OptimizelyFactory.set_flush_interval(None, self.mock_client_logger), None)
-        self.assertEqual(OptimizelyFactory.set_flush_interval(False, self.mock_client_logger), None)
-        self.assertEqual(OptimizelyFactory.set_flush_interval(True, self.mock_client_logger), None)
-        self.mock_client_logger.error.assert_called_with(
-            'Flush interval is invalid, setting to default flush interval # 30')
-
-        self.assertEqual(OptimizelyFactory.set_flush_interval(0, self.mock_client_logger), None)
-        self.assertEqual(OptimizelyFactory.set_flush_interval(-8, self.mock_client_logger), None)
-        self.mock_client_logger.error.assert_called_with(
-            'Flush interval is negative, setting to default flush interval # 30')
-
-    def test_max_event_flush_interval__should_not_log_error_message_when_valid_flush_interval(self, _):
-        self.assertEqual(OptimizelyFactory.set_flush_interval(50, self.mock_client_logger), 50)
-        self.mock_client_logger.assert_not_called()
+    def test_set_batch_size_and_set_flush_interval___should_set_values_valid_or_invalid(self, _):
+
+        # pass valid value so no default value is set
+        OptimizelyFactory.set_flush_interval(5)
+        OptimizelyFactory.set_batch_size(100)
+        optimizely_instance = OptimizelyFactory.custom_instance('sdk_key')
+        self.assertEqual(optimizely_instance.event_processor.flush_interval.seconds, 5)
+        self.assertEqual(optimizely_instance.event_processor.batch_size, 100)
+
+        # pass invalid value so set default value
+        OptimizelyFactory.set_flush_interval('test')
+        OptimizelyFactory.set_batch_size('test')
+        optimizely_instance = OptimizelyFactory.custom_instance('sdk_key')
+        self.assertEqual(optimizely_instance.event_processor.flush_interval.seconds, 30)
+        self.assertEqual(optimizely_instance.event_processor.batch_size, 10)
+
+        OptimizelyFactory.set_flush_interval(20.5)
+        OptimizelyFactory.set_batch_size(85.5)
+        optimizely_instance = OptimizelyFactory.custom_instance('sdk_key')
+        self.assertEqual(optimizely_instance.event_processor.flush_interval.seconds, 20)
+        self.assertEqual(optimizely_instance.event_processor.batch_size, 10)
+
+        OptimizelyFactory.set_flush_interval(None)
+        OptimizelyFactory.set_batch_size(None)
+        optimizely_instance = OptimizelyFactory.custom_instance('sdk_key')
+        self.assertEqual(optimizely_instance.event_processor.flush_interval.seconds, 30)
+        self.assertEqual(optimizely_instance.event_processor.batch_size, 10)
+
+        OptimizelyFactory.set_flush_interval(True)
+        OptimizelyFactory.set_batch_size(True)
+        optimizely_instance = OptimizelyFactory.custom_instance('sdk_key')
+        self.assertEqual(optimizely_instance.event_processor.flush_interval.seconds, 30)
+        self.assertEqual(optimizely_instance.event_processor.batch_size, 10)