From: Gavin M. Roy <gavinmroy@gmail.com>
Date: Mon, 6 Jan 2014 21:47:54 +0000 (-0500)
Subject: Don't call logging.basicConfig if logging is already configured (#775).
X-Git-Tag: v3.2.0b2~8^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F971%2Fhead;p=thirdparty%2Ftornado.git

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 0477ade04..7463882f9 100644
--- 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 d5a3b3b65..daa1beab5 100644
--- 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()