Merge pull request #158 from joguSD/memory-history

Don't append 'aws' twice when invoking the .edit command

Author: joguSD

awsshell/app.py

@@ -80,6 +80,11 @@ def _get_editor_command(self):
         else:
             return compat.default_editor()
 
+    def _generate_edit_history(self, application):
+        history = list(application.history)
+        commands = [h for h in history if not h.startswith(('.', '!'))]
+        return '\n'.join(commands)
+
     def run(self, command, application):
         """Open application's history buffer in an editor.
 
@@ -91,10 +96,8 @@ def run(self, command, application):
         :param application: The application object.
 
         """
-        all_commands = '\n'.join(
-            ['aws ' + h for h in list(application.history)
-             if not h.startswith(('.', '!'))])
         with temporary_file('w') as f:
+            all_commands = self._generate_edit_history(application)
             f.write(all_commands)
             f.flush()
             editor = self._get_editor_command()
@@ -222,7 +225,7 @@ class AWSShell(object):
     """
 
     def __init__(self, completer, model_completer, docs,
-                 input=None, output=None):
+                 input=None, output=None, popen_cls=None):
         self.completer = completer
         self.model_completer = model_completer
         self.history = InMemoryHistory()
@@ -238,6 +241,10 @@ def __init__(self, completer, model_completer, docs,
         self._input = input
         self._output = output
 
+        if popen_cls is None:
+            popen_cls = subprocess.Popen
+        self._popen_cls = popen_cls
+
         # These attrs come from the config file.
         self.config_obj = None
         self.config_section = None
@@ -309,7 +316,7 @@ def run(self):
                         initial_document=Document(self.current_docs,
                                                   cursor_position=0))
                     self.cli.request_redraw()
-                    p = subprocess.Popen(full_cmd, shell=True, env=self._env)
+                    p = self._popen_cls(full_cmd, shell=True, env=self._env)
                     p.communicate()
 
     def stop_input_and_refresh_cli(self):

tests/unit/test_app.py

@@ -26,17 +26,35 @@ def run(self, command, context):
     assert call_args == [(['.foo', 'a', 'b', 'c'], context)]
 
 
+class PopenLogger(object):
+    def __call__(self, cmd):
+        self.cmd = cmd
+        filename = cmd[1]
+        with open(filename, 'r') as f:
+            self.contents = f.read()
+        return mock.Mock()
+
+
 def test_edit_handler():
     env = {'EDITOR': 'my-editor'}
     popen_cls = mock.Mock()
-    context = mock.Mock()
-    context.history = []
-    handler = app.EditHandler(popen_cls, env)
-    handler.run(['.edit'], context)
+    application = mock.Mock()
+    application.history = [
+        '!ls',
+        '.edit',
+        'aws ec2 describe-instances',
+        'aws ec2 allocate-hosts',
+    ]
+    popen = PopenLogger()
+    handler = app.EditHandler(popen, env)
+    handler.run(['.edit'], application)
     # Ensure our editor was called with some arbitrary temp filename.
-    command_run = popen_cls.call_args[0][0]
+    command_run = popen.cmd
     assert len(command_run) == 2
     assert command_run[0] == 'my-editor'
+    # Ensure the contents of the temp file are correct
+    expected_contents = 'aws ec2 describe-instances\naws ec2 allocate-hosts'
+    assert popen.contents == expected_contents
 
 
 def test_error_msg_printed_on_error_handler(errstream):
@@ -132,6 +150,25 @@ def test_error_displayed_when_chdir_fails(errstream):
     assert 'FAILED' in errstream.getvalue()
 
 
+def test_history_stored_correctly():
+    mock_prompter = mock.Mock()
+    mock_prompter.buffers = {'clidocs': mock.Mock()}
+    # Simulate the user entering various commands
+    quit_document = mock.Mock()
+    quit_document.text = '.quit'
+    command_document = mock.Mock()
+    command_document.text = 'ec2 describe-instances'
+    mock_prompter.run.side_effect = [command_document, quit_document]
+    shell = app.AWSShell(mock.Mock(), mock.Mock(), mock.Mock(),
+            popen_cls=mock.Mock())
+    shell.create_cli_interface = mock.Mock(return_value=mock_prompter)
+    shell.run()
+
+    # two calls should have been made, history should have added aws
+    assert mock_prompter.run.call_count == 2
+    assert list(shell.history) == ['aws ec2 describe-instances']
+
+
 def test_exit_dot_command_exits_shell():
     mock_prompter = mock.Mock()
     # Simulate the user entering '.quit'