Added a bunch of unit tests

Also improved error handling in some exceptional cases.

3 files changed, 204 insertions, 37 deletions

diff --git a/cmd2.py b/cmd2.py
index 87a28cd2..97de8b0a 100755
--- a/cmd2.py
+++ b/cmd2.py
@@ -150,7 +150,9 @@ class OptionParser(optparse.OptionParser):
 
         We override exit so it doesn't automatically exit the application.
         """
-        self.values._exit = True
+        if self.values is not None:
+            self.values._exit = True
+
         if msg:
             print(msg)
 
@@ -159,10 +161,9 @@ class OptionParser(optparse.OptionParser):
 
         We override it so that before the standard optparse help, it prints the do_* method docstring, if available.
         """
-        try:
+        if self._func.__doc__:
             print(self._func.__doc__)
-        except AttributeError:
-            pass
+
         optparse.OptionParser.print_help(self, *args, **kwargs)
 
     def error(self, msg):
@@ -195,9 +196,13 @@ def remaining_args(opts_plus_args, arg_list):
 
 def _which(editor):
     try:
-        return subprocess.Popen(['which', editor], stdout=subprocess.PIPE, stderr=subprocess.STDOUT).communicate()[0]
-    except OSError:
-        return None
+        if six.PY3:
+            editor_path = subprocess.check_output(['which', editor], stderr=subprocess.STDOUT, encoding='utf-8').strip()
+        else:
+            editor_path = subprocess.check_output(['which', editor], stderr=subprocess.STDOUT).strip()
+    except subprocess.CalledProcessError:
+        editor_path = None
+    return editor_path
 
 
 def strip_quotes(arg):
@@ -297,7 +302,7 @@ def options(option_list, arg_desc="arg"):
             result = func(instance, arg, opts)
             return result
 
-        new_func.__doc__ = '%s\n%s' % (func.__doc__, option_parser.format_help())
+        new_func.__doc__ = '%s%s' % (func.__doc__ + '\n' if func.__doc__ else '', option_parser.format_help())
         return new_func
 
     return option_setup
@@ -746,7 +751,7 @@ class Cmd(cmd.Cmd):
 
             # NOTE: We couldn't get a real pipe working via subprocess for Python 3.x prior to 3.5.
             # So to allow compatibility with Python 2.7 and 3.3+ we are redirecting output to a temporary file.
-            # And once command is complete we are using the "cat" shell command to pipe to whatever.
+            # And once command is complete we are the temp file as stdin for the shell command to pipe to.
             # TODO: Once support for Python 3.x prior to 3.5 is no longer necessary, replace with a real subprocess pipe
 
             # Redirect stdout to a temporary file
@@ -789,8 +794,8 @@ class Cmd(cmd.Cmd):
                     # Pipe the contents of tempfile to the specified shell command
                     with open(self._temp_filename) as fd:
                         pipe_proc = subprocess.Popen(shlex.split(statement.parsed.pipeTo), stdin=fd,
-                                                     stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-                        output, err = pipe_proc.communicate()
+                                                     stdout=subprocess.PIPE)
+                        output, _ = pipe_proc.communicate()
 
                         if six.PY3:
                             self.stdout.write(output.decode())
@@ -970,12 +975,8 @@ class Cmd(cmd.Cmd):
             funcname = self._func_named(arg)
             if funcname:
                 fn = getattr(self, funcname)
-                try:
-                    # Use Optparse help for @options commands
-                    fn.optionParser.print_help(file=self.stdout)
-                except AttributeError:
-                    # No special behavior needed, delegate to cmd base class do_help()
-                    cmd.Cmd.do_help(self, funcname[3:])
+                # No special behavior needed, delegate to cmd base class do_help()
+                cmd.Cmd.do_help(self, funcname[3:])
         else:
             # Show a menu of what commands help can be gotten for
             self._help_menu()
@@ -1140,7 +1141,7 @@ class Cmd(cmd.Cmd):
         """Execute a command as if at the OS prompt.
 
     Usage:  shell <command> [arguments]"""
-        proc = subprocess.Popen(command, stdout=self.stdout, stderr=sys.stderr, shell=True)
+        proc = subprocess.Popen(command, stdout=self.stdout, shell=True)
         proc.communicate()
 
     def path_complete(self, text, line, begidx, endidx, dir_exe_only=False, dir_only=False):
@@ -1573,7 +1574,7 @@ Edited files are run on close if the ``autorun_on_edit`` settable parameter is T
         try:
             args = self.saveparser.parseString(arg)
         except pyparsing.ParseException:
-            self.perror('Could not understand save target %s' % arg)
+            self.perror('Could not understand save target %s' % arg, traceback_war=False)
             raise SyntaxError(self.do_save.__doc__)
 
         # If a filename was supplied then use that, otherwise use a temp file
@@ -1588,21 +1589,20 @@ Edited files are run on close if the ``autorun_on_edit`` settable parameter is T
         elif args.idx:
             saveme = self.history[int(args.idx) - 1]
         else:
-            saveme = ''
             # Wrap in try to deal with case of empty history
             try:
                 # Since this save command has already been added to history, need to go one more back for previous
                 saveme = self.history[-2]
             except IndexError:
-                pass
+                self.perror('History is empty, nothing to save.', traceback_war=False)
+                return
         try:
             f = open(os.path.expanduser(fname), 'w')
             f.write(saveme)
             f.close()
             self.pfeedback('Saved to {}'.format(fname))
-        except Exception:
-            self.perror('Error saving {}'.format(fname))
-            raise
+        except Exception as e:
+            self.perror('Saving {!r} - {}'.format(fname, e.strerror), traceback_war=False)
 
     def do__relative_load(self, file_path):
         """Runs commands in script file that is encoded as either ASCII or UTF-8 text.
diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py
index 9dd2697b..544b758e 100644
--- a/tests/test_cmd2.py
+++ b/tests/test_cmd2.py
@@ -90,8 +90,7 @@ now: True
     out = run_cmd(base_app, 'show quiet')
     assert out == ['quiet: True']
 
-
-def test_base_set_not_supported(base_app, capsys):
+def test_set_not_supported(base_app, capsys):
     run_cmd(base_app, 'set qqq True')
     out, err = capsys.readouterr()
     expected = normalize("""
@@ -100,6 +99,17 @@ To enable full traceback, run the following command:  'set debug true'
 """)
     assert normalize(str(err)) == expected
 
+def test_set_abbreviated(base_app):
+    out = run_cmd(base_app, 'set quie True')
+    expected = normalize("""
+quiet - was: False
+now: True
+""")
+    assert out == expected
+
+    out = run_cmd(base_app, 'show quiet')
+    assert out == ['quiet: True']
+
 
 def test_base_shell(base_app, monkeypatch):
     m = mock.Mock()
@@ -206,6 +216,16 @@ shortcuts
 """)
     assert out == expected
 
+def test_history_script_format(base_app):
+    run_cmd(base_app, 'help')
+    run_cmd(base_app, 'shortcuts')
+    out = run_cmd(base_app, 'history -s')
+    expected = normalize("""
+help
+shortcuts
+""")
+    assert out == expected
+
 def test_history_with_string_argument(base_app):
     run_cmd(base_app, 'help')
     run_cmd(base_app, 'shortcuts')
@@ -319,8 +339,7 @@ load {}
 """.format(filename))
     assert out == expected
 
-
-def test_base_load_with_empty_args(base_app, capsys):
+def test_load_with_empty_args(base_app, capsys):
     # The way the load command works, we can't directly capture its stdout or stderr
     run_cmd(base_app, 'load')
     out, err = capsys.readouterr()
@@ -330,7 +349,7 @@ def test_base_load_with_empty_args(base_app, capsys):
     assert normalize(str(err)) == expected
 
 
-def test_base_load_with_nonexistent_file(base_app, capsys):
+def test_load_with_nonexistent_file(base_app, capsys):
     # The way the load command works, we can't directly capture its stdout or stderr
     run_cmd(base_app, 'load does_not_exist.txt')
     out, err = capsys.readouterr()
@@ -340,7 +359,7 @@ def test_base_load_with_nonexistent_file(base_app, capsys):
     assert "does not exist or is not a file" in str(err)
 
 
-def test_base_load_with_empty_file(base_app, capsys, request):
+def test_load_with_empty_file(base_app, capsys, request):
     test_dir = os.path.dirname(request.module.__file__)
     filename = os.path.join(test_dir, 'scripts', 'empty.txt')
 
@@ -353,7 +372,7 @@ def test_base_load_with_empty_file(base_app, capsys, request):
     assert "is empty" in str(err)
 
 
-def test_base_load_with_binary_file(base_app, capsys, request):
+def test_load_with_binary_file(base_app, capsys, request):
     test_dir = os.path.dirname(request.module.__file__)
     filename = os.path.join(test_dir, 'scripts', 'binary.bin')
 
@@ -366,7 +385,7 @@ def test_base_load_with_binary_file(base_app, capsys, request):
     assert "is not an ASCII or UTF-8 encoded text file" in str(err)
 
 
-def test_base_load_with_utf8_file(base_app, capsys, request):
+def test_load_with_utf8_file(base_app, capsys, request):
     test_dir = os.path.dirname(request.module.__file__)
     filename = os.path.join(test_dir, 'scripts', 'utf8.txt')
 
@@ -396,6 +415,12 @@ _relative_load {}
 """.format(filename))
     assert out == expected
 
+def test_relative_load_requires_an_argument(base_app, capsys):
+    run_cmd(base_app, '_relative_load')
+    out, err = capsys.readouterr()
+    assert out == ''
+    assert err.startswith('ERROR: _relative_load command requires a file path:\n')
+
 
 def test_base_save(base_app):
     # TODO: Use a temporary directory for the file
@@ -436,6 +461,46 @@ save * deleteme.txt
     # Delete file that was created
     os.remove(filename)
 
+def test_save_parse_error(base_app, capsys):
+    invalid_file = '~!@'
+    run_cmd(base_app, 'save {}'.format(invalid_file))
+    out, err = capsys.readouterr()
+    assert out == ''
+    assert err.startswith('ERROR: Could not understand save target {}\n'.format(invalid_file))
+
+def test_save_tempfile(base_app):
+    # Just run help to make sure there is something in the history
+    run_cmd(base_app, 'help')
+    out = run_cmd(base_app, 'save *')
+    output = out[0]
+    assert output.startswith('Saved to ')
+
+    # Delete the tempfile which was created
+    temp_file = output.split('Saved to ')[1].strip()
+    os.remove(temp_file)
+
+def test_save_invalid_history_index(base_app, capsys):
+    run_cmd(base_app, 'save 5')
+    out, err = capsys.readouterr()
+    assert out == ''
+    assert err.startswith("EXCEPTION of type 'IndexError' occurred with message: 'list index out of range'\n")
+
+def test_save_empty_history_and_index(base_app, capsys):
+    run_cmd(base_app, 'save')
+    out, err = capsys.readouterr()
+    assert out == ''
+    assert err.startswith("ERROR: History is empty, nothing to save.\n")
+
+def test_save_invalid_path(base_app, capsys):
+    # Just run help to make sure there is something in the history
+    run_cmd(base_app, 'help')
+
+    invalid_path = '/no_such_path/foobar.txt'
+    run_cmd(base_app, 'save {}'.format(invalid_path))
+    out, err = capsys.readouterr()
+    assert out == ''
+    assert err.startswith("ERROR: Saving '{}' - ".format(invalid_path))
+
 
 def test_output_redirection(base_app):
     # TODO: Use a temporary directory/file for this file
@@ -587,7 +652,6 @@ To enable full traceback, run the following command:  'set debug true'
 
     return expected_text
 
-
 def test_edit_no_editor(base_app, capsys):
     # Purposely set the editor to None
     base_app.editor = None
@@ -599,7 +663,6 @@ def test_edit_no_editor(base_app, capsys):
     expected = _expected_no_editor_error()
     assert normalize(str(err)) == expected
 
-
 def test_edit_file(base_app, request):
     # Set a fake editor just to make sure we have one.  We aren't really going to call it due to the mock
     base_app.editor = 'fooedit'
@@ -616,7 +679,6 @@ def test_edit_file(base_app, request):
     # We think we have an editor, so should expect a system call
     m.assert_called_once_with('{} {}'.format(base_app.editor, filename))
 
-
 def test_edit_number(base_app):
     # Set a fake editor just to make sure we have one.  We aren't really going to call it due to the mock
     base_app.editor = 'fooedit'
@@ -633,7 +695,6 @@ def test_edit_number(base_app):
     # We have an editor, so should expect a system call
     m.assert_called_once()
 
-
 def test_edit_blank(base_app):
     # Set a fake editor just to make sure we have one.  We aren't really going to call it due to the mock
     base_app.editor = 'fooedit'
@@ -650,6 +711,12 @@ def test_edit_blank(base_app):
     # We have an editor, so should expect a system call
     m.assert_called_once()
 
+def test_edit_empty_history(base_app, capsys):
+    run_cmd(base_app, 'edit')
+    out, err = capsys.readouterr()
+    assert out == ''
+    assert err == 'ERROR: edit must be called with argument if history is empty\n'
+
 
 def test_base_py_interactive(base_app):
     # Mock out the InteractiveConsole.interact() call so we don't actually wait for a user's response on stdin
@@ -999,3 +1066,91 @@ arg 2: 'bar'
     run_cmd(noarglist_app, 'pyscript {} foo bar'.format(python_script))
     out, err = capsys.readouterr()
     assert out == expected
+
+
+class OptionApp(cmd2.Cmd):
+    @cmd2.options([cmd2.make_option('-s', '--shout', action="store_true", help="N00B EMULATION MODE")])
+    def do_greet(self, arg, opts=None):
+        arg = ''.join(arg)
+        if opts.shout:
+            arg = arg.upper()
+        self.stdout.write(arg + '\n')
+
+def test_option_help_with_no_docstring(capsys):
+    app = OptionApp()
+    app.onecmd_plus_hooks('greet -h')
+    out, err = capsys.readouterr()
+    assert err == ''
+    assert out == """Usage: greet [options] arg
+
+Options:
+  -h, --help   show this help message and exit
+  -s, --shout  N00B EMULATION MODE
+"""
+
+@pytest.mark.skipif(sys.platform.startswith('win'),
+                    reason="cmd2._which function only used on Mac and Linux")
+def test_which_editor_good():
+    editor = 'vi'
+    path = cmd2._which(editor)
+    # Assert that the vi editor was found because it should exist on all Mac and Linux systems
+    assert path
+
+@pytest.mark.skipif(sys.platform.startswith('win'),
+                    reason="cmd2._which function only used on Mac and Linux")
+def test_which_editor_bad():
+    editor = 'notepad.exe'
+    path = cmd2._which(editor)
+    # Assert that the editor wasn't found because no notepad.exe on non-Windows systems ;-)
+    assert path is None
+
+
+class MultilineApp(cmd2.Cmd):
+    def __init__(self, *args, **kwargs):
+        self.multilineCommands = ['orate']
+
+        # Need to use this older form of invoking super class constructor to support Python 2.x and Python 3.x
+        cmd2.Cmd.__init__(self, *args, **kwargs)
+
+    @cmd2.options([cmd2.make_option('-s', '--shout', action="store_true", help="N00B EMULATION MODE")])
+    def do_orate(self, arg, opts=None):
+        arg = ''.join(arg)
+        if opts.shout:
+            arg = arg.upper()
+        self.stdout.write(arg + '\n')
+
+@pytest.fixture
+def multiline_app():
+    app = MultilineApp()
+    app.stdout = StdOut()
+    return app
+
+def test_multiline_complete_empty_statement_raises_exception(multiline_app):
+    with pytest.raises(cmd2.EmptyStatement):
+        multiline_app._complete_statement('')
+
+def test_multiline_complete_statement_without_terminator(multiline_app):
+    # Mock out the input call so we don't actually wait for a user's response on stdin when it looks for more input
+    m = mock.MagicMock(name='input', return_value='\n')
+    sm.input = m
+
+    command = 'orate'
+    args = 'hello world'
+    line = '{} {}'.format(command, args)
+    statement = multiline_app._complete_statement(line)
+    assert statement == args
+    assert statement.parsed.command == command
+
+
+def test_clipboard_failure(capsys):
+    # Force cmd2 clipboard to be disabled
+    cmd2.can_clip = False
+    app = cmd2.Cmd()
+
+    # Redirect command output to the clipboard when a clipboard isn't present
+    app.onecmd_plus_hooks('help > ')
+
+    # Make sure we got the error output
+    out, err = capsys.readouterr()
+    assert out == ''
+    assert 'Cannot redirect to paste buffer; install ``xclip`` and re-run to enable' in err
diff --git a/tests/test_parsing.py b/tests/test_parsing.py
index dda29911..1a32a734 100644
--- a/tests/test_parsing.py
+++ b/tests/test_parsing.py
@@ -20,7 +20,6 @@ def hist():
     h = cmd2.History([HistoryItem('first'), HistoryItem('second'), HistoryItem('third'), HistoryItem('fourth')])
     return h
 
-
 @pytest.fixture
 def parser():
     c = cmd2.Cmd()
@@ -38,6 +37,11 @@ def input_parser():
     c = cmd2.Cmd()
     return c.parser_manager.input_source_parser
 
+@pytest.fixture
+def option_parser():
+    op = cmd2.OptionParser()
+    return op
+
 
 def test_remaining_args():
     assert cmd2.remaining_args('-f bar   bar   cow', ['bar', 'cow']) == 'bar   cow'
@@ -265,3 +269,11 @@ def test_parse_input_redirect_from_unicode_filename(input_parser):
     line = '< café'
     results = input_parser.parseString(line)
     assert results.inputFrom == line
+
+
+def test_option_parser_exit_with_msg(option_parser, capsys):
+    msg = 'foo bar'
+    option_parser.exit(msg=msg)
+    out, err = capsys.readouterr()
+    assert out == msg + '\n'
+    assert err == ''