Cleanup some history tests

2 files changed, 41 insertions, 25 deletions

diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py
index 14c4227a..18a9d5d9 100644
--- a/cmd2/cmd2.py
+++ b/cmd2/cmd2.py
@@ -403,6 +403,10 @@ class Cmd(cmd.Cmd):
         self.hidden_commands = ['eof', 'eos', '_relative_load']
 
         # Commands to exclude from the history command
+        # initialize history
+        self.persistent_history_file = persistent_history_file
+        self.persistent_history_length = persistent_history_length
+        self._initialize_history()
         self.exclude_from_history = '''history edit eof eos'''.split()
 
         # Command aliases and macros
@@ -463,9 +467,6 @@ class Cmd(cmd.Cmd):
         # If this string is non-empty, then this warning message will print if a broken pipe error occurs while printing
         self.broken_pipe_warning = ''
 
-        # initialize history
-        self._initialize_history(persistent_history_file, persistent_history_length)
-
         # If a startup script is provided, then add it in the queue to load
         if startup_script is not None:
             startup_script = os.path.expanduser(startup_script)
@@ -3447,8 +3448,8 @@ class Cmd(cmd.Cmd):
             for hi in history:
                 self.poutput(hi.pr(script=args.script, expanded=args.expanded, verbose=args.verbose))
 
-    def _initialize_history(self, histfile, maxlen):
-        """Initialize history with optional persistence and maximum length
+    def _initialize_history(self):
+        """Initialize history using history related attributes
 
         This function can determine whether history is saved in the prior text-based
         format (one line of input is stored as one line in the file), or the new-as-
@@ -3461,38 +3462,57 @@ class Cmd(cmd.Cmd):
         """
         self.history = History()
         # with no persistent history, nothing else in this method is relevant
-        if not histfile:
+        if not self.persistent_history_file:
             return
 
-        histfile = os.path.expanduser(histfile)
+        self.persistent_history_file = os.path.expanduser(self.persistent_history_file)
 
         # first we try and unpickle the history file
         history = History()
         try:
-            with open(histfile, 'rb') as fobj:
+            with open(self.persistent_history_file, 'rb') as fobj:
                 history = pickle.load(fobj)
         except (FileNotFoundError, KeyError, EOFError):
             pass
+        except IsADirectoryError:
+            msg = "persistent history file '{}' is a directory"
+            self.perror(msg.format(self.persistent_history_file))
         except OSError as ex:
             msg = "can not read persistent history file '{}': {}"
-            self.perror(msg.format(histfile, ex), traceback_war=False)
+            self.perror(msg.format(self.persistent_history_file, ex), traceback_war=False)
+
+        self.history = history
 
         # trim history to length and ensure it's writable
-        history.truncate(maxlen)
-        try:
-            # open with append so it doesn't truncate the file
-            with open(histfile, 'ab') as fobj:
-                self.persistent_history_file = histfile
-        except OSError as ex:
-            msg = "can not write persistent history file '{}': {}"
-            self.perror(msg.format(histfile, ex), traceback_war=False)
+        # history.truncate(maxlen)
+        # try:
+        #     # open with append so it doesn't truncate the file
+        #     with open(histfile, 'ab') as fobj:
+        #         self.persistent_history_file = histfile
+        # except OSError as ex:
+        #     msg = "can not write persistent history file '{}': {}"
+        #     self.perror(msg.format(histfile, ex), traceback_war=False)
 
         # register a function to write history at save
         # if the history file is in plain text format from 0.9.12 or lower
         # this will fail, and the history in the plain text file will be lost
 
         #import atexit
-        #atexit.register(readline.write_history_file, self.persistent_history_file)
+        #atexit.register(self._persist_history_on_exit)
+
+    def _persist_history_on_exit(self):
+        """write history out to the history file"""
+        if not self.persistent_history_file:
+            return
+
+        self.history.truncate(self.persistent_history_length)
+        try:
+            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 '{}': {}"
+            self.perror(msg.format(self.persistent_history_file, ex), traceback_war=False)
 
     def _generate_transcript(self, history: List[Union[HistoryItem, str]], transcript_file: str) -> None:
         """Generate a transcript file from a given history of commands."""
diff --git a/tests/test_history.py b/tests/test_history.py
index 554281c4..2a2d90d2 100644
--- a/tests/test_history.py
+++ b/tests/test_history.py
@@ -398,8 +398,6 @@ def hist_file():
         pass
 
 def test_existing_history_file(hist_file, capsys):
-    import atexit
-    import readline
 
     # Create the history file before making cmd2 app
     with open(hist_file, 'w'):
@@ -416,11 +414,9 @@ def test_existing_history_file(hist_file, capsys):
     ## TODO atexit.unregister(readline.write_history_file)
 
     # Remove created history file
-    os.remove(hist_file)
+    #os.remove(hist_file)
 
 def test_new_history_file(hist_file, capsys):
-    import atexit
-    import readline
 
     # Remove any existing history file
     try:
@@ -439,7 +435,7 @@ def test_new_history_file(hist_file, capsys):
     ### TODO atexit.unregister(readline.write_history_file)
 
     # Remove created history file
-    os.remove(hist_file)
+    #os.remove(hist_file)
 
 def test_bad_history_file_path(capsys, request):
     # Use a directory path as the history file
@@ -449,7 +445,7 @@ def test_bad_history_file_path(capsys, request):
     cmd2.Cmd(persistent_history_file=test_dir)
     _, err = capsys.readouterr()
 
-    assert 'can not write' in err
+    assert 'is a directory' in err
 
 def test_history_file_conversion_no_truncate_on_init(hist_file, capsys):
     # test the code that converts a plain text history file to a pickle binary