Don't call logging.basicConfig if logging is already configured (#775).

Will not call logging.basicConfig if there is a root logger,
tornado logger or tornado.application logger.

Updated patch to reflect comments about tests and method privacy.

diff --git a/tornado/ioloop.py b/tornado/ioloop.py
index 0477ade048543e202164f5351d76ce5dbccc9b8e..7463882f94e1f7025ac3932df7ad684168e3b472 100644 (file)
--- a/tornado/ioloop.py
+++ b/tornado/ioloop.py
@@ -549,16 +549,23 @@ class PollIOLoop(IOLoop):
             signal.signal(signal.SIGALRM,
                           action if action is not None else signal.SIG_DFL)
 
-    def start(self):
-        if not logging.getLogger().handlers:
-            # The IOLoop catches and logs exceptions, so it's
-            # important that log output be visible.  However, python's
-            # default behavior for non-root loggers (prior to python
-            # 3.2) is to print an unhelpful "no handlers could be
-            # found" message rather than the actual log entry, so we
-            # must explicitly configure logging if we've made it this
-            # far without anything.
+    def _setup_logging(self):
+        """The IOLoop catches and logs exceptions, so it's
+        important that log output be visible.  However, python's
+        default behavior for non-root loggers (prior to python
+        3.2) is to print an unhelpful "no handlers could be
+        found" message rather than the actual log entry, so we
+        must explicitly configure logging if we've made it this
+        far without anything.
+
+        """
+        if not any([logging.getLogger().handlers,
+                    logging.getLogger('tornado').handlers,
+                    logging.getLogger('tornado.application').handlers]):
             logging.basicConfig()
+
+    def start(self):
+        self._setup_logging()
         if self._stopped:
             self._stopped = False
             return

diff --git a/tornado/test/ioloop_test.py b/tornado/test/ioloop_test.py
index d5a3b3b6519863138888f70ba6307225b6ce4fe4..daa1beab5df0b5e0f2fc0878fe03c79c3b2ab67c 100644 (file)
--- a/tornado/test/ioloop_test.py
+++ b/tornado/test/ioloop_test.py
@@ -5,6 +5,7 @@ from __future__ import absolute_import, division, print_function, with_statement
 import contextlib
 import datetime
 import functools
+import logging
 import socket
 import sys
 import threading
@@ -21,6 +22,14 @@ try:
 except ImportError:
     futures = None
 
+try:
+    from unittest import mock  # python 3.3+
+except ImportError:
+    try:
+        import mock  # third-party mock package
+    except ImportError:
+        mock = None
+
 
 class TestIOLoop(AsyncTestCase):
     @skipOnTravis
@@ -329,5 +338,44 @@ class TestIOLoopRunSync(unittest.TestCase):
             yield gen.Task(self.io_loop.add_timeout, self.io_loop.time() + 1)
         self.assertRaises(TimeoutError, self.io_loop.run_sync, f, timeout=0.01)
 
+
+class TestIOLoopLogging(AsyncTestCase):
+
+    @unittest.skipIf(mock is None, 'mock package not present')
+    def test_basic_config_invoked(self):
+        """If no loggers have been defined, logging.basicConfig should be called
+        test for the presence of a root logger, start IOloop and then test
+        to see if one has been added
+
+        """
+        with mock.patch('logging.basicConfig') as basicConfig:
+            self.io_loop.add_timeout(datetime.timedelta(microseconds=1),
+                                     self.stop)
+            self.wait()
+            basicConfig.assert_called_once()
+
+    @unittest.skipIf(mock is None, 'mock package not present')
+    def test_basic_config_not_invoked(self):
+        """The setup_logging method will check for getLogger().handlers,
+        getLogger('tornado').handlers and
+        getLogger('tornado.application').handlers. If all are empty, it will
+        invoke basicConfig. Test to make sure all three are called, all three
+        are checked and basicConfig is not invoked because there is a handler
+        returned (for each).
+
+        """
+        with mock.patch('logging.getLogger') as getLogger:
+            with mock.patch('logging.basicConfig') as basicConfig:
+                getLogger.handlers = [True]
+                self.io_loop.add_timeout(datetime.timedelta(microseconds=1),
+                                         self.stop)
+                self.wait()
+                basicConfig.assert_not_called()
+                getLogger.assert_has_calls([mock.call(),
+                                            mock.call('tornado'),
+                                            mock.call('tornado.application')])
+                self.assertEqual(getLogger.call_count, 3)
+
+
 if __name__ == "__main__":
     unittest.main()