Merge pull request #744 from python-cmd2/make_history_directory

Create directory for the persistent history file if it does not already exist

Author: kmvanbrunt

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

cmd2/cmd2.py

@@ -3603,23 +3603,33 @@ def _initialize_history(self, hist_file):
 
         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)
         except (AttributeError, EOFError, FileNotFoundError, ImportError, IndexError, KeyError, pickle.UnpicklingError):
             # 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 @@ def _persist_history(self):
             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:

tests/test_history.py

@@ -638,14 +638,44 @@ 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
 
     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