Merge pull request #663 from python-cmd2/pyscript_capture

Pyscript now saves command output during the same period that redirection does

Author: kmvanbrunt

CHANGELOG.md

@@ -1,3 +1,8 @@
+## 0.9.13 (TBD, 2019)
+* Enhancements
+    * `pyscript` limits a command's stdout capture to the same period that redirection does.
+      Therefore output from a command's postparsing and finalization hooks isn't saved in the StdSim object.
+    
 ## 0.9.12 (April 22, 2019)
 * Bug Fixes
     * Fixed a bug in how redirection and piping worked inside ``py`` or ``pyscript`` commands

cmd2/cmd2.py

@@ -1675,10 +1675,13 @@ def parseline(self, line: str) -> Tuple[str, str, str]:
         statement = self.statement_parser.parse_command_only(line)
         return statement.command, statement.args, statement.command_and_args
 
-    def onecmd_plus_hooks(self, line: str) -> bool:
+    def onecmd_plus_hooks(self, line: str, pyscript_bridge_call: bool = False) -> bool:
         """Top-level function called by cmdloop() to handle parsing a line and running the command and all of its hooks.
 
         :param line: line of text read from input
+        :param pyscript_bridge_call: This should only ever be set to True by PyscriptBridge to signify the beginning
+                                     of an app() call in a pyscript. It is used to enable/disable the storage of the
+                                     command's stdout.
         :return: True if cmdloop() should exit, False otherwise
         """
         import datetime
@@ -1718,6 +1721,10 @@ def onecmd_plus_hooks(self, line: str) -> bool:
             try:
                 # Get sigint protection while we set up redirection
                 with self.sigint_protection:
+                    if pyscript_bridge_call:
+                        # Start saving command's stdout at this point
+                        self.stdout.pause_storage = False
+
                     redir_error, saved_state = self._redirect_output(statement)
                     self.cur_pipe_proc_reader = saved_state.pipe_proc_reader
 
@@ -1763,6 +1770,10 @@ def onecmd_plus_hooks(self, line: str) -> bool:
                     if not already_redirecting:
                         self.redirecting = False
 
+                    if pyscript_bridge_call:
+                        # Stop saving command's stdout before command finalization hooks run
+                        self.stdout.pause_storage = True
+
         except EmptyStatement:
             # don't do anything, but do allow command finalization hooks to run
             pass
@@ -3022,11 +3033,10 @@ def _reset_py_display() -> None:
     @with_argparser(py_parser, preserve_quotes=True)
     def do_py(self, args: argparse.Namespace) -> bool:
         """Invoke Python command or shell"""
-        from .pyscript_bridge import PyscriptBridge, CommandResult
+        from .pyscript_bridge import PyscriptBridge
         if self._in_py:
             err = "Recursively entering interactive Python consoles is not allowed."
             self.perror(err, traceback_war=False)
-            self._last_result = CommandResult('', err)
             return False
 
         try:

cmd2/pyscript_bridge.py

@@ -62,11 +62,9 @@ def __bool__(self) -> bool:
 
 
 class PyscriptBridge(object):
-    """Preserves the legacy 'cmd' interface for pyscript while also providing a new python API wrapper for
-    application commands."""
+    """Provides a Python API wrapper for application commands."""
     def __init__(self, cmd2_app):
         self._cmd2_app = cmd2_app
-        self._last_result = None
         self.cmd_echo = False
 
     def __dir__(self):
@@ -89,6 +87,9 @@ def __call__(self, command: str, echo: Optional[bool] = None) -> CommandResult:
         # This will be used to capture _cmd2_app.stdout and sys.stdout
         copy_cmd_stdout = StdSim(self._cmd2_app.stdout, echo)
 
+        # Pause the storing of stdout until onecmd_plus_hooks enables it
+        copy_cmd_stdout.pause_storage = True
+
         # This will be used to capture sys.stderr
         copy_stderr = StdSim(sys.stderr, echo)
 
@@ -98,7 +99,7 @@ def __call__(self, command: str, echo: Optional[bool] = None) -> CommandResult:
             self._cmd2_app.stdout = copy_cmd_stdout
             with redirect_stdout(copy_cmd_stdout):
                 with redirect_stderr(copy_stderr):
-                    self._cmd2_app.onecmd_plus_hooks(command)
+                    self._cmd2_app.onecmd_plus_hooks(command, pyscript_bridge_call=True)
         finally:
             self._cmd2_app.stdout = copy_cmd_stdout.inner_stream
 

tests/pyscript/stdout_capture.py

@@ -0,0 +1,26 @@
+# flake8: noqa F821
+# This script demonstrates when output of a command finalization hook is captured by a pyscript app() call
+import sys
+
+# The unit test framework passes in the string being printed by the command finalization hook
+hook_output = sys.argv[1]
+
+# Run a help command which results in 1 call to onecmd_plus_hooks
+res = app('help')
+
+# hook_output will not be captured because there are no nested calls to onecmd_plus_hooks
+if hook_output not in res.stdout:
+    print("PASSED")
+else:
+    print("FAILED")
+
+# Run the last command in the history
+res = app('history -r -1')
+
+# All output of the history command will be captured. This includes all output of the commands
+# started in do_history() using onecmd_plus_hooks(), including any output in those commands' hooks.
+# Therefore we expect the hook_output to show up this time.
+if hook_output in res.stdout:
+    print("PASSED")
+else:
+    print("FAILED")

tests/test_cmd2.py

@@ -268,9 +268,8 @@ def test_recursive_pyscript_not_allowed(base_app, request):
     python_script = os.path.join(test_dir, 'scripts', 'recursive.py')
     expected = 'Recursively entering interactive Python consoles is not allowed.'
 
-    run_cmd(base_app, "pyscript {}".format(python_script))
-    err = base_app._last_result.stderr
-    assert err == expected
+    out, err = run_cmd(base_app, "pyscript {}".format(python_script))
+    assert err[0] == expected
 
 def test_pyscript_with_nonexist_file(base_app):
     python_script = 'does_not_exist.py'

tests/test_plugin.py

@@ -205,13 +205,13 @@ def cmdfinalization_hook(self, data: plugin.CommandFinalizationData) -> plugin.C
         return data
 
     def cmdfinalization_hook_stop(self, data: cmd2.plugin.CommandFinalizationData) -> cmd2.plugin.CommandFinalizationData:
-        """A postparsing hook which requests application exit"""
+        """A command finalization hook which requests application exit"""
         self.called_cmdfinalization += 1
         data.stop = True
         return data
 
     def cmdfinalization_hook_exception(self, data: cmd2.plugin.CommandFinalizationData) -> cmd2.plugin.CommandFinalizationData:
-        """A postparsing hook which raises an exception"""
+        """A command finalization hook which raises an exception"""
         self.called_cmdfinalization += 1
         raise ValueError
 

tests/test_pyscript.py

@@ -4,9 +4,16 @@
 Unit/functional testing for pytest in cmd2
 """
 import os
+from cmd2 import plugin
 
 from .conftest import run_cmd
 
+HOOK_OUTPUT = "TEST_OUTPUT"
+
+def cmdfinalization_hook(data: plugin.CommandFinalizationData) -> plugin.CommandFinalizationData:
+    """A cmdfinalization_hook hook which requests application exit"""
+    print(HOOK_OUTPUT)
+    return data
 
 def test_pyscript_help(base_app, request):
     test_dir = os.path.dirname(request.module.__file__)
@@ -23,3 +30,13 @@ def test_pyscript_dir(base_app, request):
     out, err = run_cmd(base_app, 'pyscript {}'.format(python_script))
     assert out
     assert out[0] == "['cmd_echo']"
+
+
+def test_pyscript_stdout_capture(base_app, request):
+    base_app.register_cmdfinalization_hook(cmdfinalization_hook)
+    test_dir = os.path.dirname(request.module.__file__)
+    python_script = os.path.join(test_dir, 'pyscript', 'stdout_capture.py')
+    out, err = run_cmd(base_app, 'pyscript {} {}'.format(python_script, HOOK_OUTPUT))
+
+    assert out[0] == "PASSED"
+    assert out[1] == "PASSED"