Merge pull request #663 from python-cmd2/pyscript_capture

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

7 files changed, 70 insertions, 12 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c72cfff0..c67fee2c 100644
--- a/CHANGELOG.md
+++ b/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
diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py
index ddc3945b..a7b60b1a 100644
--- a/cmd2/cmd2.py
+++ b/cmd2/cmd2.py
@@ -1675,10 +1675,13 @@ class Cmd(cmd.Cmd):
         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 @@ class Cmd(cmd.Cmd):
             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 @@ class Cmd(cmd.Cmd):
                     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 @@ class Cmd(cmd.Cmd):
     @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:
diff --git a/cmd2/pyscript_bridge.py b/cmd2/pyscript_bridge.py
index a4eaf308..1c720cf9 100644
--- a/cmd2/pyscript_bridge.py
+++ b/cmd2/pyscript_bridge.py
@@ -62,11 +62,9 @@ class CommandResult(namedtuple_with_defaults('CommandResult', ['stdout', 'stderr
 
 
 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 @@ class PyscriptBridge(object):
         # 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 @@ class PyscriptBridge(object):
             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
 
diff --git a/tests/pyscript/stdout_capture.py b/tests/pyscript/stdout_capture.py
new file mode 100644
index 00000000..4aa78d53
--- /dev/null
+++ b/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")
diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py
index d33477f2..300e3ed9 100644
--- a/tests/test_cmd2.py
+++ b/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'
diff --git a/tests/test_plugin.py b/tests/test_plugin.py
index 1f95017c..242b0d25 100644
--- a/tests/test_plugin.py
+++ b/tests/test_plugin.py
@@ -205,13 +205,13 @@ class Plugin:
         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
 
diff --git a/tests/test_pyscript.py b/tests/test_pyscript.py
index 6981980b..4866548b 100644
--- a/tests/test_pyscript.py
+++ b/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"