Merge pull request #1372 from dmach/improve-print_msg

Improve print_msg() and migrate some arbitrary prints to it

osc/_private/common.py

@@ -1,16 +1,22 @@
 import sys
 
 
-def print_msg(msg, print_to="debug"):
+def print_msg(*args, print_to="debug"):
     from .. import conf
 
     if print_to is None:
         return
     elif print_to == "debug":
+        # print a debug message to stderr if config["debug"] is set
         if conf.config["debug"]:
-            print(f"DEBUG: {msg}", file=sys.stderr)
+            print("DEBUG:", *args, file=sys.stderr)
+    elif print_to == "verbose":
+        # print a verbose message to stdout if config["verbose"] or config["debug"] is set
+        if conf.config["verbose"] or conf.config["debug"]:
+            print(*args)
     elif print_to == "stdout":
-        print(msg)
+        # print the message to stdout
+        print(*args)
     else:
         raise ValueError(f"Invalid value of the 'print_to' option: {print_to}")
 

osc/babysitter.py

@@ -116,9 +116,8 @@ def run(prg, argv=None):
         except AttributeError:
             body = ''
 
-        if osc_conf.config["debug"]:
+        _private.print_msg(e.hdrs, print_to="debug")
-            print(e.hdrs, file=sys.stderr)
+        _private.print_msg(body, print_to="debug")
-            print(body, file=sys.stderr)
 
         if e.code in [400, 403, 404, 500]:
             if b'<summary>' in body:
@@ -162,8 +161,7 @@ def run(prg, argv=None):
         print(e.message, file=sys.stderr)
     except oscerr.OscIOError as e:
         print(e.msg, file=sys.stderr)
-        if osc_conf.config["debug"]:
+        _private.print_msg(e.e, print_to="debug")
-            print(e.e, file=sys.stderr)
     except (oscerr.WrongOptions, oscerr.WrongArgs) as e:
         print(e, file=sys.stderr)
         return 2

osc/commandline.py

@@ -4264,8 +4264,7 @@ Please submit there instead, or use --nodevelproject to force direct submission.
                 except:
                     print('Error while checkout package:\n', package, file=sys.stderr)
 
-            if conf.config['verbose']:
+            _private.print_msg('Note: You can use "osc delete" or "osc submitpac" when done.\n', print_to="verbose")
-                print('Note: You can use "osc delete" or "osc submitpac" when done.\n')
 
     @cmdln.alias('branchco')
     @cmdln.alias('bco')
@@ -4416,8 +4415,7 @@ Please submit there instead, or use --nodevelproject to force direct submission.
         if opts.checkout:
             checkout_package(apiurl, targetprj, package, server_service_files=False,
                              expand_link=True, prj_dir=Path(targetprj))
-            if conf.config['verbose']:
+            _private.print_msg('Note: You can use "osc delete" or "osc submitpac" when done.\n', print_to="verbose")
-                print('Note: You can use "osc delete" or "osc submitpac" when done.\n')
         else:
             apiopt = ''
             if conf.get_configParser().get('general', 'apiurl') != apiurl:

osc/connection.py

@@ -21,6 +21,7 @@ import urllib3.response
 import urllib3.util
 
 from . import __version__
+from . import _private
 from . import conf
 from . import oscerr
 from . import oscssl
@@ -686,9 +687,7 @@ class SignatureAuthHandler(AuthHandlerBase):
             return False
 
         if not self.ssh_keygen_path:
-            if conf.config["debug"]:
+            _private.print_msg("Skipping signature auth because ssh-keygen is not available", print_to="debug")
-                msg = "Skipping signature auth because ssh-keygen is not available"
-                print(msg, file=sys.stderr)
             return False
 
         if not self.sshkey_known():

osc/core.py

@@ -516,8 +516,7 @@ class Serviceinfo:
                     raise oscerr.PackageNotInstalled("obs-service-%s" % cmd[0])
                 cmd[0] = "/usr/lib/obs/service/" + cmd[0]
                 cmd = cmd + ["--outdir", temp_dir]
-                if conf.config['verbose'] or verbose or conf.config['debug']:
+                _private.print_msg("Run source service:", " ".join(cmd), print_to="verbose")
-                    print("Run source service:", ' '.join(cmd))
                 r = run_external(*cmd)
 
                 if r != 0:
@@ -3600,8 +3599,7 @@ def makeurl(baseurl: str, l, query=None):
     function. In case of a list not -- this is to be backwards compatible.
     """
     query = query or []
-    if conf.config['debug']:
+    _private.print_msg("makeurl:", baseurl, l, query, print_to="debug")
-        print('makeurl:', baseurl, l, query)
 
     if isinstance(query, list):
         query = '&'.join(query)
@@ -4774,8 +4772,7 @@ def get_review_list(
             xpath_base = xpath_join(xpath_base, 'action/source/@%(kind)s=\'%(val)s\'', op='or', inner=True)
         xpath = xpath_join(xpath, xpath_base % {'kind': kind, 'val': val}, op='and', nexpr_parentheses=True)
 
-    if conf.config['debug']:
+    _private.print_msg(f"[ {xpath} ]", print_to="debug")
-        print('[ %s ]' % xpath)
     res = search(apiurl, request=xpath)
     collection = res['request']
     requests = []
@@ -4916,8 +4913,7 @@ def get_exact_request_list(
     if req_type:
         xpath += " and action/@type=\'%s\'" % req_type
 
-    if conf.config['debug']:
+    _private.print_msg(f"[ {xpath} ]", print_to="debug")
-        print('[ %s ]' % xpath)
 
     res = search(apiurl, request=xpath)
     collection = res['request']
@@ -5589,10 +5585,10 @@ def checkout_package(
         prj_dir = Path(str(prj_dir).replace(':', sep))
 
     root_dots = Path('.')
+    oldproj = None
     if conf.config['checkout_rooted']:
         if prj_dir.stem == '/':
-            if conf.config['verbose']:
+            _private.print_msg(f"checkout_rooted ignored for {prj_dir}", print_to="verbose")
-                print("checkout_rooted ignored for %s" % prj_dir)
             # ?? should we complain if not is_project_dir(prj_dir) ??
         else:
             # if we are inside a project or package dir, ascend to parent
@@ -5619,9 +5615,7 @@ def checkout_package(
                 root_dots = root_dots / ("../" * n)
 
     if str(root_dots) != '.':
-        if conf.config['verbose']:
+        _private.print_msg(f"{prj_dir} is project dir of {oldproj}. Root found at {os.path.abspath(root_dots)}", print_to="verbose")
-            print("%s is project dir of %s. Root found at %s" %
-                  (prj_dir, oldproj, os.path.abspath(root_dots)))
         prj_dir = root_dots / prj_dir
 
     if not pathname:

tests/test_output.py

@@ -1,5 +1,10 @@
+import contextlib
+import importlib
+import io
 import unittest
 
+import osc.conf
+from osc._private import print_msg
 from osc.output import KeyValueTable
 
 
@@ -67,5 +72,74 @@ Key    : Value
         self.assertEqual(str(t), expected)
 
 
+class TestPrintMsg(unittest.TestCase):
+    def setUp(self):
+        # reset the global `config` in preparation for running the tests
+        importlib.reload(osc.conf)
+
+    def tearDown(self):
+        # reset the global `config` to avoid impacting tests from other classes
+        importlib.reload(osc.conf)
+
+    def test_debug(self):
+        osc.conf.config["debug"] = 0
+        stdout = io.StringIO()
+        stderr = io.StringIO()
+        with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr):
+            print_msg("foo", "bar", print_to="debug")
+        self.assertEqual("", stdout.getvalue())
+        self.assertEqual("", stderr.getvalue())
+
+        osc.conf.config["debug"] = 1
+        stdout = io.StringIO()
+        stderr = io.StringIO()
+        with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr):
+            print_msg("foo", "bar", print_to="debug")
+        self.assertEqual("", stdout.getvalue())
+        self.assertEqual("DEBUG: foo bar\n", stderr.getvalue())
+
+    def test_verbose(self):
+        osc.conf.config["verbose"] = 0
+        stdout = io.StringIO()
+        stderr = io.StringIO()
+        with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr):
+            print_msg("foo", "bar", print_to="verbose")
+        self.assertEqual("", stdout.getvalue())
+        self.assertEqual("", stderr.getvalue())
+
+        osc.conf.config["verbose"] = 1
+        stdout = io.StringIO()
+        stderr = io.StringIO()
+        with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr):
+            print_msg("foo", "bar", print_to="verbose")
+        self.assertEqual("foo bar\n", stdout.getvalue())
+        self.assertEqual("", stderr.getvalue())
+
+        osc.conf.config["verbose"] = 0
+        osc.conf.config["debug"] = 1
+        stdout = io.StringIO()
+        stderr = io.StringIO()
+        with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr):
+            print_msg("foo", "bar", print_to="verbose")
+        self.assertEqual("foo bar\n", stdout.getvalue())
+        self.assertEqual("", stderr.getvalue())
+
+    def test_none(self):
+        stdout = io.StringIO()
+        stderr = io.StringIO()
+        with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr):
+            print_msg("foo", "bar", print_to=None)
+        self.assertEqual("", stdout.getvalue())
+        self.assertEqual("", stderr.getvalue())
+
+    def test_stdout(self):
+        stdout = io.StringIO()
+        stderr = io.StringIO()
+        with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr):
+            print_msg("foo", "bar", print_to="stdout")
+        self.assertEqual("foo bar\n", stdout.getvalue())
+        self.assertEqual("", stderr.getvalue())
+
+
 if __name__ == "__main__":
     unittest.main()