Fixed broken string conversion in Python-exported OST logging.

modules/base/pymod/export_logger.cc

@@ -54,10 +54,10 @@ typedef boost::shared_ptr<WrappedLogSink> WrappedLogSinkPtr;
 
 void push_verb(int n) 
 {
- if(n<0){
-   Logger::Instance().PushVerbosityLevel(0);
-  }else{
-   Logger::Instance().PushVerbosityLevel(n);
+  if (n<0) {
+    Logger::Instance().PushVerbosityLevel(0);
+  } else {
+    Logger::Instance().PushVerbosityLevel(n);
   }
 }
 
@@ -90,17 +90,18 @@ LogSinkPtr get_log_sink()
 String args_to_string(tuple args, dict kwargs)
 {
   std::stringstream ss;
-  bool empty=true;
-  for (size_t i=0, l=len(args); i<l; ++i) {
+  bool empty = true;
+  for (size_t i = 0, l = len(args); i < l; ++i) {
     if (!empty) {
       ss << " ";
     }
-    empty=false;
+    empty = false;
     String string_val;
-    try {
-      string_val=extract<String>(args[i]);
-    } catch (...) {
-      string_val=extract<String>(args[i].attr("__str__")());
+    extract<String> tst(args[i]);
+    if (tst.check()) {
+      string_val = tst();
+    } else {
+      string_val = extract<String>(str(args[i])); // use boost python str
     }
     ss << string_val;
   }

modules/base/tests/test_log.py

 import unittest
 import ost
 
+# helper log sink to campture messages
+class _CapturingLogSink(ost.LogSink):
+  def __init__(self):
+    ost.LogSink.__init__(self)
+  def LogMessage(self, message, severity):
+    self.message = message
+    self.severity = severity
+
 # Altough the logging system might appear to be too simple to be worth writing a 
 # specific test case for, it actually isn't. The python export is very fragile 
 # and seemingly trivial changes can break the code in unexpected ways. So let's 
 # check for some invariants
 class TestLog(unittest.TestCase):
+
   def testGetLogSink(self):
-    logsink=ost.GetCurrentLogSink()
+    logsink = ost.GetCurrentLogSink()
     self.assertTrue(hasattr(logsink, 'LogMessage'))
     # Check if the return type of logsink is sane
     ost.PushLogSink(ost.GetCurrentLogSink())
+
   def testPushPopLogSink(self):
     class MyLogSink(ost.LogSink):
        def __init__(self):
          ost.LogSink.__init__(self)
-    ls=MyLogSink()
+    ls = MyLogSink()
     ost.PushLogSink(ls)
     self.assertEqual(ls, ost.GetCurrentLogSink())
     ost.PopLogSink()
     self.assertNotEqual(ls, ost.GetCurrentLogSink())
 
   def testLogMessage(self):
-    class CapturingLogSink(ost.LogSink):
-      def __init__(self):
-        ost.LogSink.__init__(self)
-      def LogMessage(self, message, severity):
-        self.message=message
-        self.severity=severity
-        ost.PushLogSink(ls)
-    ls=CapturingLogSink()
+    ls = _CapturingLogSink()
     ost.PushVerbosityLevel(1)
     ost.PushLogSink(ls)
     ost.LogError('error message')
@@ -39,6 +42,20 @@ class TestLog(unittest.TestCase):
     self.assertEqual(ls.message, '1 2 3\n')
     self.assertEqual(ls.severity, 1)
     ost.PopLogSink()
+
+  def testLogMultipleMessages(self):
+    # observation: converting non-strings in logging can break following calls
+    ls = _CapturingLogSink()
+    ost.PushVerbosityLevel(1)
+    ost.PushLogSink(ls)
+    ost.LogWarning('foo')
+    self.assertEqual(ls.message, 'foo\n')
+    ost.LogWarning(1)
+    self.assertEqual(ls.message, '1\n')
+    ost.LogWarning('bar')
+    self.assertEqual(ls.message, 'bar\n')
+    ost.PopLogSink()
+
 if __name__ == "__main__":
   from ost import testutils
   testutils.RunTests()
\ No newline at end of file