Merge pull request #1135 from dcbaker/submit/external-command-tests

Submit/external command tests

2 files changed, 68 insertions, 17 deletions

diff --git a/alot/commands/globals.py b/alot/commands/globals.py
index f8f58749..787ff450 100644
--- a/alot/commands/globals.py
+++ b/alot/commands/globals.py
@@ -247,7 +247,7 @@ class ExternalCommand(Command):
         # set standard input for subcommand
         stdin = None
         if self.stdin is not None:
-            # wrap strings in StrinIO so that they behaves like a file
+            # wrap strings in StringIO so that they behave like files
             if isinstance(self.stdin, unicode):
                 stdin = StringIO(self.stdin)
             else:
@@ -255,7 +255,7 @@ class ExternalCommand(Command):
 
         def afterwards(data):
             if data == 'success':
-                if callable(self.on_success):
+                if self.on_success is not None:
                     self.on_success()
             else:
                 ui.notify(data, priority='error')
@@ -267,24 +267,17 @@ class ExternalCommand(Command):
 
         def thread_code(*_):
             try:
-                if stdin is None:
-                    proc = subprocess.Popen(self.cmdlist, shell=self.shell,
-                                            stderr=subprocess.PIPE)
-                    ret = proc.wait()
-                    err = proc.stderr.read()
-                else:
-                    proc = subprocess.Popen(self.cmdlist, shell=self.shell,
-                                            stdin=subprocess.PIPE,
-                                            stderr=subprocess.PIPE)
-                    _, err = proc.communicate(stdin.read())
-                    ret = proc.wait()
-                if ret == 0:
-                    return 'success'
-                else:
-                    return err.strip()
+                proc = subprocess.Popen(self.cmdlist, shell=self.shell,
+                                        stdin=subprocess.PIPE,
+                                        stderr=subprocess.PIPE)
             except OSError as e:
                 return str(e)
 
+            _, err = proc.communicate(stdin.read() if stdin else None)
+            if proc.returncode == 0:
+                return 'success'
+            return err.strip()
+
         if self.in_thread:
             d = threads.deferToThread(thread_code)
             d.addCallback(afterwards)
diff --git a/tests/commands/global_test.py b/tests/commands/global_test.py
index 860a6ccb..edb903dd 100644
--- a/tests/commands/global_test.py
+++ b/tests/commands/global_test.py
@@ -17,6 +17,7 @@
 """Tests for global commands."""
 
 from __future__ import absolute_import
+import os
 
 from twisted.trial import unittest
 from twisted.internet.defer import inlineCallbacks
@@ -116,3 +117,60 @@ class TestComposeCommand(unittest.TestCase):
 
         self.assertFalse(envelope.sign)
         self.assertIs(envelope.sign_key, None)
+
+
+class TestExternalCommand(unittest.TestCase):
+
+    class Success(Exception):
+        pass
+
+    def on_success(self):
+        raise self.Success
+
+    def test_no_spawn_no_stdin_success(self):
+        cmd = g_commands.ExternalCommand(
+            u'true',
+            refocus=False, on_success=self.on_success)
+        with self.assertRaises(self.Success):
+            cmd.apply(mock.Mock())
+
+    def test_no_spawn_stdin_success(self):
+        cmd = g_commands.ExternalCommand(
+            u"awk '{ exit $0 }'",
+            stdin=u'0', refocus=False, on_success=self.on_success)
+        with self.assertRaises(self.Success):
+            cmd.apply(mock.Mock())
+
+    def test_no_spawn_failure(self):
+        ui = mock.Mock()
+        cmd = g_commands.ExternalCommand(
+            u'false',
+            refocus=False, on_success=self.on_success)
+        cmd.apply(ui)
+        ui.notify.assert_called_once_with('', priority='error')
+
+    @mock.patch('alot.commands.globals.settings.get', mock.Mock(return_value=''))
+    @mock.patch.dict(os.environ, {'DISPLAY': ':0'})
+    def test_spawn_no_stdin_success(self):
+        ui = mock.Mock()
+        cmd = g_commands.ExternalCommand(u'true', refocus=False, spawn=True)
+        cmd.apply(ui)
+        ui.notify.assert_not_called()
+
+    @mock.patch('alot.commands.globals.settings.get', mock.Mock(return_value=''))
+    @mock.patch.dict(os.environ, {'DISPLAY': ':0'})
+    def test_spawn_stdin_success(self):
+        ui = mock.Mock()
+        cmd = g_commands.ExternalCommand(
+            u"awk '{ exit $0 }'",
+            stdin=u'0', refocus=False, spawn=True)
+        cmd.apply(ui)
+        ui.notify.assert_not_called()
+
+    @mock.patch('alot.commands.globals.settings.get', mock.Mock(return_value=''))
+    @mock.patch.dict(os.environ, {'DISPLAY': ':0'})
+    def test_spawn_failure(self):
+        ui = mock.Mock()
+        cmd = g_commands.ExternalCommand(u'false', refocus=False, spawn=True)
+        cmd.apply(ui)
+        ui.notify.assert_called_once_with('', priority='error')