diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | cmd2/cmd2.py | 20 | ||||
| -rw-r--r-- | tests/test_history.py | 34 |
3 files changed, 48 insertions, 7 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 37999bbf..57b321fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ## 0.9.16 (TBD, 2019) * Enhancements * Raise `TypeError` if trying to set choices/completions on argparse action that accepts no arguments + * Create directory for the persistent history file if it does not already exist ## 0.9.15 (July 24, 2019) * Bug Fixes diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index adadbdf8..7c529acc 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -3603,15 +3603,25 @@ class Cmd(cmd.Cmd): hist_file = os.path.abspath(os.path.expanduser(hist_file)) - # first we try and unpickle the history file - history = History() # on Windows, trying to open a directory throws a permission # error, not a `IsADirectoryError`. So we'll check it ourselves. if os.path.isdir(hist_file): - msg = "persistent history file '{}' is a directory" + msg = "Persistent history file '{}' is a directory" self.perror(msg.format(hist_file)) return + # Create the directory for the history file if it doesn't already exist + hist_file_dir = os.path.dirname(hist_file) + try: + os.makedirs(hist_file_dir, exist_ok=True) + except OSError as ex: + msg = "Error creating persistent history file directory '{}': {}".format(hist_file_dir, ex) + self.pexcept(msg) + return + + # first we try and unpickle the history file + history = History() + try: with open(hist_file, 'rb') as fobj: history = pickle.load(fobj) @@ -3619,7 +3629,7 @@ class Cmd(cmd.Cmd): # If any non-operating system error occurs when attempting to unpickle, just use an empty history pass except OSError as ex: - msg = "can not read persistent history file '{}': {}" + msg = "Can not read persistent history file '{}': {}" self.pexcept(msg.format(hist_file, ex)) return @@ -3655,7 +3665,7 @@ class Cmd(cmd.Cmd): with open(self.persistent_history_file, 'wb') as fobj: pickle.dump(self.history, fobj) except OSError as ex: - msg = "can not write persistent history file '{}': {}" + msg = "Can not write persistent history file '{}': {}" self.pexcept(msg.format(self.persistent_history_file, ex)) def _generate_transcript(self, history: List[Union[HistoryItem, str]], transcript_file: str) -> None: diff --git a/tests/test_history.py b/tests/test_history.py index 2da34cfd..9752ed07 100644 --- a/tests/test_history.py +++ b/tests/test_history.py @@ -638,6 +638,36 @@ def test_history_file_is_directory(capsys): _, err = capsys.readouterr() assert 'is a directory' in err +def test_history_can_create_directory(mocker): + # Mock out atexit.register so the persistent file doesn't written when this function + # exists because we will be deleting the directory it needs to go to. + mock_register = mocker.patch('atexit.register') + + # Create a temp path for us to use and let it get deleted + with tempfile.TemporaryDirectory() as test_dir: + pass + assert not os.path.isdir(test_dir) + + # Add some subdirectories for the complete history file directory + hist_file_dir = os.path.join(test_dir, 'subdir1', 'subdir2') + hist_file = os.path.join(hist_file_dir, 'hist_file') + + # Make sure cmd2 creates the history file directory + cmd2.Cmd(persistent_history_file=hist_file) + assert os.path.isdir(hist_file_dir) + + # Cleanup + os.rmdir(hist_file_dir) + +def test_history_cannot_create_directory(mocker, capsys): + mock_open = mocker.patch('os.makedirs') + mock_open.side_effect = OSError + + hist_file_path = os.path.join('fake_dir', 'file') + cmd2.Cmd(persistent_history_file=hist_file_path) + _, err = capsys.readouterr() + assert 'Error creating persistent history file directory' in err + def test_history_file_permission_error(mocker, capsys): mock_open = mocker.patch('builtins.open') mock_open.side_effect = PermissionError @@ -645,7 +675,7 @@ def test_history_file_permission_error(mocker, capsys): cmd2.Cmd(persistent_history_file='/tmp/doesntmatter') out, err = capsys.readouterr() assert not out - assert 'can not read' in err + assert 'Can not read' in err def test_history_file_conversion_no_truncate_on_init(hist_file, capsys): # make sure we don't truncate the plain text history file on init @@ -720,4 +750,4 @@ def test_persist_history_permission_error(hist_file, mocker, capsys): app._persist_history() out, err = capsys.readouterr() assert not out - assert 'can not write' in err + assert 'Can not write' in err |