Rev 9: Catch failures when processing a sub-command, in http://bzr.arbash-meinel.com/branches/bzr/other/moin_graphviz

John Arbash Meinel john at arbash-meinel.com
Thu Apr 5 21:25:00 BST 2007


At http://bzr.arbash-meinel.com/branches/bzr/other/moin_graphviz

------------------------------------------------------------
revno: 9
revision-id: john at arbash-meinel.com-20070405202442-hvthng8rpojdtuam
parent: john at arbash-meinel.com-20070405194758-95ioshbgq02k5s3v
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: moin_graphviz
timestamp: Thu 2007-04-05 15:24:42 -0500
message:
  Catch failures when processing a sub-command,
  and make sure they are properly escaped in the output.
modified:
  graphviz.py                    graphviz.py-20070402222439-3qloodrjtlsuaz1o-1
  test_graphviz.py               test_graphviz.py-20070402223544-kez8ooyxqh1wx5w0-1
-------------- next part --------------
=== modified file 'graphviz.py'
--- a/graphviz.py	2007-04-05 19:47:58 +0000
+++ b/graphviz.py	2007-04-05 20:24:42 +0000
@@ -32,6 +32,16 @@
 class DotError(RuntimeError):
     """Raised when we fail to run Dot"""
 
+    def __init__(self, command, returncode, stderr):
+        self.command = command
+        self.str_command = ' '.join(command)
+        self.returncode = returncode
+        self.stderr = stderr
+
+        RuntimeError.__init__(self,
+            '"%s" failed with return code %s: %s'
+            % (self.str_command, self.returncode, self.stderr))
+
 
 class DotNotFound(Exception):
     """Raised when the specified dot command cannot be found."""
@@ -56,9 +66,7 @@
         fd = self._spawn_dot()
         (myout, myerr) = fd.communicate(self.raw)
         if fd.returncode != 0:
-            raise DotError('%s failed with return code %s'
-                            % (' '.join(self._dot_command()),
-                               fd.returncode))
+            raise DotError(self._dot_command(), fd.returncode, myerr)
         return myout
 
     def _dot_command(self):
@@ -66,7 +74,8 @@
 
     def _spawn_dot(self):
         try:
-            return Popen(self._dot_command(), stdin=PIPE, stdout=PIPE)
+            return Popen(self._dot_command(),
+                         stdin=PIPE, stdout=PIPE, stderr=PIPE)
         except OSError, e:
             if e.errno in (errno.ENOENT,):
                 raise DotNotFound()
@@ -89,6 +98,23 @@
                         % (xml_escape(self._dot_command()[0]),)
                         ))
             return
+        except DotError, e:
+            template = ('<b>Processing failed\n'
+                        '<br />\n'
+                        'input:<pre>%s</pre>\n'
+                        'command:<pre>%s</pre>\n'
+                        'returncode: %s\n'
+                        'error:<pre>%s<pre>\n'
+                        '</b>'
+                        )
+            txt = template % (xml_escape(self.raw),
+                              xml_escape(e.str_command),
+                              e.returncode,
+                              xml_escape(e.stderr),
+                             )
+            self.request.write(formatter.rawHTML(txt))
+            return
+
         img_str = ('<img src="data:image/png;base64,%s"'
                    ' alt="Graphviz Image" />') % (png_base64,)
         # TODO: jam 20070402 Are we supposed to be calling wikiutil.escape?

=== modified file 'test_graphviz.py'
--- a/test_graphviz.py	2007-04-05 19:47:58 +0000
+++ b/test_graphviz.py	2007-04-05 20:24:42 +0000
@@ -79,6 +79,31 @@
             request = self.get_request()
         return graphviz.Parser(raw, request)
 
+    def failUnlessRaises(self, excClass, callableObj, *args, **kwargs):
+        """Fail unless an exception of class excClass is thrown
+           by callableObj when invoked with arguments args and keyword
+           arguments kwargs. If a different type of exception is
+           thrown, it will not be caught, and the test case will be
+           deemed to have suffered an error, exactly as for an
+           unexpected exception.
+
+           :return: The raised exception.
+        """
+        # Override TestCase.failUnlessRaises to return the exception for
+        # inspection
+        try:
+            callableObj(*args, **kwargs)
+        except excClass, e:
+            return e
+        else:
+            excName = getattr(excClass, '__name__', None)
+            if excName is None:
+                excName = str(excClass)
+            raise self.failureException("%s not raised" % excName)
+
+    assertRaises = failUnlessRaises
+
+
 
 class TestParser(TestCaseWithParser):
 
@@ -135,14 +160,28 @@
         sub = p._spawn_dot()
         out, err = sub.communicate()
         self.assertEqual('', out)
-        self.assertEqual(None, err)
+        self.assertEqual('', err)
 
         sub = p._spawn_dot()
         out, err = sub.communicate('some text\n')
         self.assertEqual('some text\n', out)
-        self.assertEqual(None, err)
-
-    def test__spawn_bad_command(self):
+        self.assertEqual('', err)
+
+    def test__spawn_dot_stderr(self):
+        """Spawn dot should grab stderr as well."""
+        p = ParserWithCommand('', self.get_request())
+        p._command = [sys.executable, '-c',
+                        'import sys;'
+                        'sys.stderr.write(sys.stdin.read());'
+                        'sys.stderr.flush();'
+                     ]
+
+        sub = p._spawn_dot()
+        out, err = sub.communicate('text')
+        self.assertEqual('', out)
+        self.assertEqual('text', err)
+
+    def test__spawn_dot_bad_command(self):
         """Check the error when 'dot' doesn't exist."""
         orig_val = graphviz.Parser._dot
         try:
@@ -161,12 +200,27 @@
 
     def test__create_png_handles_failure(self):
         p = ParserWithCommand('raw_text', self.get_request())
+        # Pass through, but fail when done
         p._command = [sys.executable, '-c',
                         'import sys;'
                         'sys.stdout.write(sys.stdin.read());'
+                        'sys.exit(1);'
+                     ]
+        e = self.assertRaises(graphviz.DotError, p._create_png)
+        self.assertEqual('"%s" failed with return code 1: '
+                         % (' '.join(p._command),),
+                         str(e))
+
+    def test__create_png_grabs_errors(self):
+        p = ParserWithCommand('raw_text', self.get_request())
+        p._command = [sys.executable, '-c',
+                        'import sys;'
+                        'sys.stderr.write(sys.stdin.read());'
                         'sys.exit(1)']
-        # Pass through, but fail when done
-        self.assertRaises(graphviz.DotError, p._create_png)
+        e = self.assertRaises(graphviz.DotError, p._create_png)
+        self.assertEqual('"%s" failed with return code 1: raw_text'
+                         % (' '.join(p._command),),
+                         str(e))
 
     def test__init__ignores_extra_args(self):
         request = self.get_request()
@@ -235,9 +289,32 @@
         self.assertEqual(expected_text, request._output.getvalue())
         self.assertEqual([('rawHTML', expected_text)], formatter._actions)
 
+    def test_format_escapes_failed_command(self):
+        request = self.get_request()
+        p = ParserWithCommand('input\n<text>\n', request)
+        p._command = [sys.executable, '-c',
+                        'import sys;'
+                        'sys.stderr.write("I can\'t do that <george>\\n");'
+                        'sys.exit(2);'
+                     ]
+        formatter = self.get_formatter()
+        p.format(formatter)
+
+        expected_text = ('<b>Processing failed\n'
+                         '<br />\n'
+                         'input:<pre>input\n&lt;text&gt;\n</pre>\n'
+                         'command:<pre>%s</pre>\n'
+                         'returncode: 2\n'
+                         'error:<pre>I can&apos;t do that &lt;george&gt;\n'
+                         '<pre>\n'
+                         '</b>'
+                         ) % (graphviz.xml_escape(' '.join(p._command)),)
+        self.assertEqual(expected_text, request._output.getvalue())
+        self.assertEqual([('rawHTML', expected_text)], formatter._actions)
+
 
 class TestXmlEscape(unittest.TestCase):
-    """Test the basic abilities of graphiz.escape"""
+    """Test the basic abilities of graphviz.xml_escape"""
 
     def test_simple(self):
         self.assertEqual('foo', graphviz.xml_escape('foo'))
@@ -265,3 +342,19 @@
         """Most chars shouldn't be escaped."""
         txt = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789'
         self.assertEqual(txt, graphviz.xml_escape(txt))
+
+
+class TestDotError(unittest.TestCase):
+    """Test the formatting for DotError."""
+
+    def test_formatting(self):
+        e = graphviz.DotError(['command', 'foo'], 2, 'error')
+        self.assertEqual('"command foo" failed with return code 2: error',
+                         str(e))
+
+    def test_args(self):
+        e = graphviz.DotError(['command', 'foo'], 2, 'error')
+        self.assertEqual(['command', 'foo'], e.command)
+        self.assertEqual('command foo', e.str_command)
+        self.assertEqual(2, e.returncode)
+        self.assertEqual('error', e.stderr)



More information about the bazaar-commits mailing list