Updated tests. Updated log format.

rtv/__main__.py

@@ -74,7 +74,10 @@ def main():
         #     if args[0] != "header:":
         #         _http_logger.info(' '.join(args))
         # client.print = print_to_file
-        logging.basicConfig(level=logging.DEBUG, filename=config['log'])
+        logging.basicConfig(
+            level=logging.DEBUG,
+            filename=config['log'],
+            format='%(asctime)s:%(levelname)s:%(filename)s:%(lineno)d:%(message)s')
     else:
         # Add an empty handler so the logger doesn't complain
         logging.root.addHandler(logging.NullHandler())

rtv/terminal.py

@@ -385,19 +385,26 @@ class Terminal(object):
     @contextmanager
     def open_editor(self, data=''):
         """
-        Open a temporary file using the system's default editor.
+        Open a file for editing using the system's default editor.
 
-        The data string will be written to the file before opening. This
+        After the file has been altered, the text will be read back and lines
-        function will block until the editor has closed. At that point the file
+        starting with '#' will be stripped. If an error occurs inside of the
-        will be read and and lines starting with '#' will be stripped. If no
+        context manager, the file will be preserved. Otherwise, the file will
-        errors occur, the file will be deleted when the context manager closes.
+        be deleted when the context manager closes.
+
+        Params:
+            data (str): If provided, text will be written to the file before
+                opening it with the editor.
+
+        Returns:
+            text (str): The text that the user entered into the editor.
         """
 
         filename = 'rtv_{:%Y%m%d_%H%M%S}.txt'.format(datetime.now())
         filepath = os.path.join(tempfile.gettempdir(), filename)
 
         with codecs.open(filepath, 'w', 'utf-8') as fp:
-            fp.write(self.clean(data))
+            fp.write(data)
         _logger.info('File created: {}'.format(filepath))
 
         editor = os.getenv('RTV_EDITOR') or os.getenv('EDITOR') or 'nano'
@@ -426,8 +433,8 @@ class Terminal(object):
             # If no errors occurred, try to remove the file
             try:
                 os.remove(filepath)
-            except OSError as e:
+            except OSError:
-                _logger.exception(e)
+                _logger.warning('Could not delete: {}'.format(filepath))
             else:
                 _logger.info('File deleted: {}'.format(filepath))
 

tests/conftest.py

@@ -27,7 +27,9 @@ except ImportError:
 patch = partial(mock.patch, autospec=True)
 
 # Turn on logging, but disable vcr from spamming
-logging.basicConfig(level=logging.DEBUG)
+logging.basicConfig(
+    level=logging.DEBUG,
+    format='%(asctime)s:%(levelname)s:%(filename)s:%(lineno)d:%(message)s')
 for name in ['vcr.matchers', 'vcr.stubs']:
     logging.getLogger(name).disabled = True
 

tests/test_submission.py

@@ -178,7 +178,7 @@ def test_submission_comment(submission_page, terminal, refresh_token):
     with mock.patch('praw.objects.Submission.add_comment') as add_comment, \
             mock.patch.object(terminal, 'open_editor') as open_editor,     \
             mock.patch('time.sleep'):
-        open_editor.return_value = 'comment text'
+        open_editor.return_value.__enter__.return_value = 'comment text'
 
         submission_page.controller.trigger('c')
         assert open_editor.called
@@ -233,7 +233,7 @@ def test_submission_edit(submission_page, terminal, refresh_token):
     with mock.patch('praw.objects.Submission.edit') as edit,           \
             mock.patch.object(terminal, 'open_editor') as open_editor, \
             mock.patch('time.sleep'):
-        open_editor.return_value = 'submission text'
+        open_editor.return_value.__enter__.return_value = 'submission text'
 
         submission_page.controller.trigger('e')
         assert open_editor.called
@@ -249,7 +249,7 @@ def test_submission_edit(submission_page, terminal, refresh_token):
     with mock.patch('praw.objects.Comment.edit') as edit,              \
             mock.patch.object(terminal, 'open_editor') as open_editor, \
             mock.patch('time.sleep'):
-        open_editor.return_value = 'comment text'
+        open_editor.return_value.__enter__.return_value = 'comment text'
 
         submission_page.controller.trigger('e')
         assert open_editor.called

tests/test_subreddit.py

@@ -148,7 +148,7 @@ def test_subreddit_post(subreddit_page, terminal, reddit, refresh_token):
     # Post a submission with a title but with no body
     subreddit_page.refresh_content(name='python')
     with mock.patch.object(terminal, 'open_editor'):
-        terminal.open_editor.return_value = 'title'
+        terminal.open_editor.return_value.__enter__.return_value = 'title'
         subreddit_page.controller.trigger('c')
         text = 'Canceled'.encode('utf-8')
         terminal.stdscr.subwin.addstr.assert_called_with(1, 1, text)
@@ -160,7 +160,7 @@ def test_subreddit_post(subreddit_page, terminal, reddit, refresh_token):
             mock.patch.object(reddit, 'submit'),      \
             mock.patch('rtv.page.Page.loop') as loop, \
             mock.patch('time.sleep'):
-        terminal.open_editor.return_value = 'test\ncontent'
+        terminal.open_editor.return_value.__enter__.return_value = 'test\ncont'
         reddit.submit.return_value = submission
         subreddit_page.controller.trigger('c')
         assert reddit.submit.called

tests/test_terminal.py

@@ -10,6 +10,7 @@ import pytest
 
 from rtv.docs import HELP, COMMENT_EDIT_FILE
 from rtv.objects import Color
+from rtv.exceptions import TemporaryFileError
 
 try:
     from unittest import mock
@@ -269,7 +270,10 @@ def test_prompt_y_or_n(terminal, stdscr):
     assert curses.flash.called
 
 
-def test_open_editor(terminal):
+@pytest.mark.parametrize('ascii', [True, False])
+def test_open_editor(terminal, ascii):
+
+    terminal.ascii = ascii
 
     comment = COMMENT_EDIT_FILE.format(content='#| This is a comment! ❤')
     data = {'filename': None}
@@ -284,11 +288,57 @@ def test_open_editor(terminal):
     with mock.patch('subprocess.Popen', autospec=True) as Popen:
         Popen.side_effect = side_effect
 
-        reply_text = terminal.open_editor(comment)
+        with terminal.open_editor(comment) as reply_text:
-        assert reply_text == 'This is an amended comment! ❤'
+            assert reply_text == 'This is an amended comment! ❤'
+            assert os.path.isfile(data['filename'])
+            assert curses.endwin.called
+            assert curses.doupdate.called
         assert not os.path.isfile(data['filename'])
-        assert curses.endwin.called
+
-        assert curses.doupdate.called
+
+def test_open_editor_error(terminal):
+
+    with mock.patch('subprocess.Popen', autospec=True) as Popen, \
+            mock.patch.object(terminal, 'show_notification'):
+
+        # Invalid editor
+        Popen.side_effect = OSError
+        with terminal.open_editor('hello') as text:
+            assert text == 'hello'
+        assert 'Could not open' in terminal.show_notification.call_args[0][0]
+
+        data = {'filename': None}
+
+        def side_effect(args):
+            data['filename'] = args[1]
+            return mock.Mock()
+
+        # Temporary File Errors don't delete the file
+        Popen.side_effect = side_effect
+        with terminal.open_editor('test'):
+            assert os.path.isfile(data['filename'])
+            raise TemporaryFileError()
+        assert os.path.isfile(data['filename'])
+        os.remove(data['filename'])
+
+        # Other Exceptions don't delete the file *and* are propagated
+        Popen.side_effect = side_effect
+        with pytest.raises(ValueError):
+            with terminal.open_editor('test'):
+                assert os.path.isfile(data['filename'])
+                raise ValueError()
+        assert os.path.isfile(data['filename'])
+        os.remove(data['filename'])
+
+        # Gracefully handle the case when we can't remove the file
+        with mock.patch.object(os, 'remove'):
+            os.remove.side_effect = OSError
+            with terminal.open_editor():
+                pass
+            assert os.remove.called
+
+        assert os.path.isfile(data['filename'])
+        os.remove(data['filename'])
 
 
 def test_open_browser(terminal):