Deprecate the debug= parameter (#63)

Author: Anthchirp

HISTORY.rst

@@ -8,6 +8,11 @@ History
   list as the first argument) is now deprecated. As a number of arguments
   will be removed in a future version the use of unnamed arguments will
   cause future confusion. `Use explicit keyword arguments instead (#62). <https://github.com/DiamondLightSource/python-procrunner/pull/62>`_
+* The run() function debug argument has been deprecated. This is only
+  only used to debug the NonBlockingStream* classes. Those are due to be
+  replaced in a future release, so the argument will no longer serve a
+  purpose. Debugging information remains available via standard logging
+  mechanisms.
 
 2.1.0 (2020-09-05)
 ------------------

procrunner/__init__.py

@@ -210,7 +210,6 @@ def __init__(self, stream, data, debug=False, notify=None):
         self._buffer = data
         self._buffer_len = len(data)
         self._buffer_pos = 0
-        self._debug = debug
         self._max_block_len = 4096
         self._stream = stream
         self._terminated = False
@@ -433,7 +432,7 @@ def wrapper(*args, **kwargs):
 def run(
     command,
     timeout=None,
-    debug=False,
+    debug=None,
     stdin=None,
     print_stdout=True,
     print_stderr=True,
@@ -453,7 +452,7 @@ def run(
 
     :param array command: Command line to be run, specified as array.
     :param timeout: Terminate program execution after this many seconds.
-    :param boolean debug: Enable further debug messages.
+    :param boolean debug: Enable further debug messages. (deprecated)
     :param stdin: Optional bytestring that is passed to command stdin.
     :param boolean print_stdout: Pass stdout through to sys.stdout.
     :param boolean print_stderr: Pass stderr through to sys.stderr.
@@ -487,6 +486,10 @@ def run(
     else:
         assert sys.platform != "win32", "stdin argument not supported on Windows"
         stdin_pipe = subprocess.PIPE
+    if debug is not None:
+        warnings.warn(
+            "Use of the debug parameter is deprecated", DeprecationWarning, stacklevel=3
+        )
 
     start_time = timeit.default_timer()
     if timeout is not None:
@@ -495,7 +498,7 @@ def run(
             warnings.warn(
                 "Using procrunner with timeout and without raise_timeout_exception set is deprecated",
                 DeprecationWarning,
-                stacklevel=2,
+                stacklevel=3,
             )
 
     if environment is not None:

tests/test_procrunner.py

@@ -42,7 +42,7 @@ def test_run_command_aborts_after_timeout(
     task = ["___"]
 
     with pytest.raises(RuntimeError):
-        procrunner.run(task, timeout=-1, debug=False, raise_timeout_exception=True)
+        procrunner.run(task, timeout=-1, raise_timeout_exception=True)
 
     assert mock_subprocess.Popen.called
     assert mock_process.terminate.called
@@ -84,7 +84,6 @@ def streamreader_processing(*args, **kwargs):
     actual = procrunner.run(
         command,
         timeout=0.5,
-        debug=False,
         callback_stdout=mock.sentinel.callback_stdout,
         callback_stderr=mock.sentinel.callback_stderr,
         working_directory=mock.sentinel.cwd,
@@ -99,14 +98,14 @@ def streamreader_processing(*args, **kwargs):
             mock.call(
                 stream_stdout,
                 output=mock.ANY,
-                debug=mock.ANY,
+                debug=None,
                 notify=mock.ANY,
                 callback=mock.sentinel.callback_stdout,
             ),
             mock.call(
                 stream_stderr,
                 output=mock.ANY,
-                debug=mock.ANY,
+                debug=None,
                 notify=mock.ANY,
                 callback=mock.sentinel.callback_stderr,
             ),
@@ -128,12 +127,21 @@ def streamreader_processing(*args, **kwargs):
 def test_default_process_environment_is_parent_environment(mock_subprocess):
     mock_subprocess.Popen.side_effect = NotImplementedError()  # cut calls short
     with pytest.raises(NotImplementedError):
-        procrunner.run(
-            [mock.Mock()], timeout=-1, debug=False, raise_timeout_exception=True
-        )
+        procrunner.run([mock.Mock()], timeout=-1, raise_timeout_exception=True)
     assert mock_subprocess.Popen.call_args[1]["env"] == os.environ
 
 
+@mock.patch("procrunner.subprocess")
+def test_using_debug_parameter_raises_warning(mock_subprocess):
+    mock_subprocess.Popen.side_effect = NotImplementedError()  # cut calls short
+    with pytest.warns(DeprecationWarning, match="debug"):
+        with pytest.raises(NotImplementedError):
+            procrunner.run([mock.Mock()], debug=True)
+    with pytest.warns(DeprecationWarning, match="debug"):
+        with pytest.raises(NotImplementedError):
+            procrunner.run([mock.Mock()], debug=False)
+
+
 @mock.patch("procrunner.subprocess")
 def test_pass_custom_environment_to_process(mock_subprocess):
     mock_subprocess.Popen.side_effect = NotImplementedError()  # cut calls short
@@ -143,7 +151,6 @@ def test_pass_custom_environment_to_process(mock_subprocess):
         procrunner.run(
             [mock.Mock()],
             timeout=-1,
-            debug=False,
             environment=copy.copy(mock_env),
             raise_timeout_exception=True,
         )
@@ -160,7 +167,6 @@ def test_pass_custom_environment_to_process_and_add_another_value(mock_subproces
         procrunner.run(
             [mock.Mock()],
             timeout=-1,
-            debug=False,
             environment=copy.copy(mock_env1),
             environment_override=copy.copy(mock_env2),
             raise_timeout_exception=True,
@@ -178,7 +184,6 @@ def test_use_default_process_environment_and_add_another_value(mock_subprocess):
         procrunner.run(
             [mock.Mock()],
             timeout=-1,
-            debug=False,
             environment_override=copy.copy(mock_env2),
             raise_timeout_exception=True,
         )
@@ -207,7 +212,6 @@ def test_use_default_process_environment_and_override_a_value(mock_subprocess):
         procrunner.run(
             [mock.Mock()],
             timeout=-1,
-            debug=False,
             environment_override={
                 random_environment_variable: "X" + random_environment_value
             },