This commit improves the logging system: instead of setting a boolean to enable logging, the user sets a logging level (per module). By default only critical errors are logged (ie nothing for now).

Also adds a test for the function run_through_shell

i3pystatus/__init__.py

@@ -12,7 +12,7 @@ h = logging.FileHandler(".i3pystatus-" + str(os.getpid()), delay=True)
 
 logger = logging.getLogger("i3pystatus")
 logger.addHandler(h)
-logger.setLevel(logging.DEBUG)
+logger.setLevel(logging.CRITICAL)
 
 
 __path__ = extend_path(__path__, __name__)
@@ -22,7 +22,6 @@ __all__ = [
     "Module", "IntervalModule",
     "SettingsBase",
     "formatp",
-    "logger",
 ]
 
 

i3pystatus/core/command.py

@@ -1,32 +1,29 @@
-from subprocess import CalledProcessError
+# from subprocess import CalledProcessError
 import subprocess
-from i3pystatus import logger
 
 
-def run_through_shell(command, enable_log, enable_shell=False):
+def run_through_shell(command, enable_shell=False):
     """
-    Retrieves output of shell command
-    Returns tuple boolean (success)/ string (error msg, output)
+    Retrieves output of command
+    Returns tuple success (boolean)/ stdout(string) / stderr (string)
+
+    Don't use this function with programs that outputs lots of data since the output is saved
+    in one variable
     """
-    result = False
+    returncode = None
     try:
         proc = subprocess.Popen(command, stderr=subprocess.PIPE, stdout=subprocess.PIPE, shell=enable_shell)
 
         out, stderr = proc.communicate()
-        if stderr and enable_log:
-            logger.error(stderr)
+        out = out.decode("UTF-8")
+        stderr = stderr.decode("UTF-8")
 
-        result = True
+        returncode = proc.returncode
 
-    except CalledProcessError as e:
+    except OSError as e:
+        out = e.strerror
+        stderr = e.strerror
+    except subprocess.CalledProcessError as e:
         out = e.output
-        # color = self.error_color
 
-    out = out.decode("UTF-8").replace("\n", " ")
-    try:
-        if out[-1] == " ":
-            out = out[:-1]
-    except:
-        out = ""
-
-    return out, result
+    return returncode, out, stderr

i3pystatus/core/settings.py

@@ -1,6 +1,7 @@
 from i3pystatus.core.util import KeyConstraintDict
 from i3pystatus.core.exceptions import ConfigKeyError, ConfigMissingError
 import inspect
+import logging
 
 
 class SettingsBase:
@@ -18,7 +19,7 @@ class SettingsBase:
     """
 
     settings = (
-        ("enable_log", "Set to true to log error to .i3pystatus-<pid> file"),
+        ("log_level", "Set to true to log error to .i3pystatus-<pid> file"),
     )
 
     """settings should be tuple containing two types of elements:
@@ -31,7 +32,8 @@ class SettingsBase:
     required = tuple()
     """required can list settings which are required"""
 
-    enable_log = False
+    log_level = logging.NOTSET
+    logger = None
 
     def __init__(self, *args, **kwargs):
         def get_argument_dict(args, kwargs):
@@ -71,6 +73,8 @@ class SettingsBase:
         self.__name__ = "{}.{}".format(
             self.__module__, self.__class__.__name__)
 
+        self.logger = logging.getLogger(self.__name__)
+        self.logger.setLevel(self.log_level)
         self.init()
 
     def init(self):

i3pystatus/mail/__init__.py

@@ -67,7 +67,9 @@ class Mail(IntervalModule):
 
     def on_leftclick(self):
         if self.email_client:
-            run_through_shell(self.email_client, self.enable_log)
+            retcode, _, stderr = run_through_shell(self.email_client)
+            if retcode != 0 and stderr:
+                self.logger.error(stderr)
 
     def on_rightclick(self):
         self.run()

i3pystatus/shell.py

@@ -1,5 +1,8 @@
 from i3pystatus import IntervalModule
 from i3pystatus.core.command import run_through_shell
+import logging
+
+# logger = logging.getLogger(__name__)
 
 
 class Shell(IntervalModule):
@@ -19,8 +22,17 @@ class Shell(IntervalModule):
     required = ("command",)
 
     def run(self):
-        out, success = run_through_shell(self.command, self.enable_log, enable_shell=True)
+        retvalue, out, stderr = run_through_shell(self.command, enable_shell=True)
+
+        if retvalue != 0:
+            self.logger.error(stderr if stderr else "Unknown error")
+
+        if out:
+            out.replace("\n", " ").strip()
+        elif stderr:
+            out = stderr
+
         self.output = {
-            "full_text": out,
-            "color": self.color if success else self.error_color
+            "full_text": out if out else "Command `%s` returned %d" % (self.command, retvalue),
+            "color": self.color if retvalue == 0 else self.error_color
         }

tests/test_core_modules.py

@@ -11,7 +11,8 @@ class IntervalModuleMetaTest(unittest.TestCase):
     def test_no_settings(self):
         class NoSettings(IntervalModule):
             pass
-        self.assertTrue('interval' in NoSettings.settings)
+        for element in ('interval', ):
+            self.assertIn(element, NoSettings.settings)
 
     def test_no_interval_setting(self):
         class NoIntervalSetting(IntervalModule):

tests/test_shell.py

@@ -0,0 +1,24 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+import unittest
+import logging
+
+from i3pystatus.shell import Shell
+from i3pystatus.core.command import run_through_shell
+
+
+class ShellModuleMetaTest(unittest.TestCase):
+
+    valid_output = "hello world"
+
+    def test_shell_correct_output(self):
+        # ShellTest test
+        # http://python.readthedocs.org/en/latest/library/unittest.html
+        retcode, out, err = run_through_shell("echo '%s'" % (self.valid_output), enable_shell=True)
+        self.assertTrue(retcode == 0)
+        self.assertEqual(out.strip(), self.valid_output)
+
+    def test_program_failure(self):
+        success, out, err = run_through_shell("thisshouldtriggeranerror")
+        self.assertFalse(success)