testtools-dev team mailing list archive

["Martin \[gz\]" <gzlist@xxxxxxxxxxxxxx>]

Martin [gz] has proposed merging lp:~gz/testtools/handle_malformed_syntaxerror_instances into lp:testtools.

Requested reviews:
  testtools developers (testtools-dev)


Some dodgy script runner code in bzr turned up a case where my handling of SyntaxError decoding needs to be more robust. The actual change is here is trivial, unfortunately it meant reindenting a bunch of the code hence the horrid diff, sorry. Basically, switches from looking at the second arg of the instance to tuple unpacking the whole exception inside a try/except similar to how the traceback code does.

This function still needs splitting up into smaller chunks, then I could add a more targetted unittest as well rather than using the big hammer that is the TestNonAsciiResults class.

See also the bzr mp for an example of the kind of code that was an issue:
<https://code.launchpad.net/~gz/bzr/script_test_missing_command_error/+merge/38875>

The code added isn't valid on Python 3, but does not run there. I believe but did not check that the syntax is still fine. Ended up filing three test suite issues, and by the third didn't feel like rolling back even further to verify the fix.
-- 
https://code.launchpad.net/~gz/testtools/handle_malformed_syntaxerror_instances/+merge/38894
Your team testtools developers is requested to review the proposed merge of lp:~gz/testtools/handle_malformed_syntaxerror_instances into lp:testtools.

=== modified file 'testtools/compat.py'
--- testtools/compat.py	2010-07-29 18:15:18 +0000
+++ testtools/compat.py	2010-10-19 22:16:08 +0000
@@ -209,38 +209,43 @@
         list = []
     if evalue is None:
         # Is a (deprecated) string exception
-        list.append(eclass.decode("ascii", "replace"))
-    elif isinstance(evalue, SyntaxError) and len(evalue.args) > 1:
+        list.append((eclass + "\n").decode("ascii", "replace"))
+        return list
+    if isinstance(evalue, SyntaxError):
         # Avoid duplicating the special formatting for SyntaxError here,
         # instead create a new instance with unicode filename and line
         # Potentially gives duff spacing, but that's a pre-existing issue
-        filename, lineno, offset, line = evalue.args[1]
-        if line:
+        try:
+            msg, (filename, lineno, offset, line) = evalue
+        except (TypeError, ValueError):
+            pass # Strange exception instance, fall through to generic code
+        else:
             # Errors during parsing give the line from buffer encoded as
             # latin-1 or utf-8 or the encoding of the file depending on the
             # coding and whether the patch for issue #1031213 is applied, so
             # give up on trying to decode it and just read the file again
-            bytestr = linecache.getline(filename, lineno)
-            if bytestr:
-                if lineno == 1 and bytestr.startswith("\xef\xbb\xbf"):
-                    bytestr = bytestr[3:]
-                line = bytestr.decode(_get_source_encoding(filename), "replace")
-                del linecache.cache[filename]
-            else:
-                line = line.decode("ascii", "replace")
-        if filename:
-            filename = filename.decode(fs_enc, "replace")
-        evalue = eclass(evalue.args[0], (filename, lineno, offset, line))
-        list.extend(traceback.format_exception_only(eclass, evalue))
+            if line:
+                bytestr = linecache.getline(filename, lineno)
+                if bytestr:
+                    if lineno == 1 and bytestr.startswith("\xef\xbb\xbf"):
+                        bytestr = bytestr[3:]
+                    line = bytestr.decode(
+                        _get_source_encoding(filename), "replace")
+                    del linecache.cache[filename]
+                else:
+                    line = line.decode("ascii", "replace")
+            if filename:
+                filename = filename.decode(fs_enc, "replace")
+            evalue = eclass(msg, (filename, lineno, offset, line))
+            list.extend(traceback.format_exception_only(eclass, evalue))
+            return list
+    sclass = eclass.__name__
+    svalue = _exception_to_text(evalue)
+    if svalue:
+        list.append("%s: %s\n" % (sclass, svalue))
+    elif svalue is None:
+        # GZ 2010-05-24: Not a great fallback message, but keep for the moment
+        list.append("%s: <unprintable %s object>\n" % (sclass, sclass))
     else:
-        sclass = eclass.__name__
-        svalue = _exception_to_text(evalue)
-        if svalue:
-            list.append("%s: %s\n" % (sclass, svalue))
-        elif svalue is None:
-            # GZ 2010-05-24: Not a great fallback message, but keep for the
-            #                the same for compatibility for the moment
-            list.append("%s: <unprintable %s object>\n" % (sclass, sclass))
-        else:
-            list.append("%s\n" % sclass)
+        list.append("%s\n" % sclass)
     return list

=== modified file 'testtools/tests/test_testresult.py'
--- testtools/tests/test_testresult.py	2010-10-17 16:34:55 +0000
+++ testtools/tests/test_testresult.py	2010-10-19 22:16:08 +0000
@@ -1082,6 +1082,11 @@
             'SyntaxError: '
             ), textoutput)
 
+    def test_syntax_error_malformed(self):
+        """Syntax errors with bogus parameters should break anythong"""
+        textoutput = self._test_external_case("raise SyntaxError(3, 2, 1)")
+        self.assertIn(self._as_output("\nSyntaxError: "), textoutput)
+
     def test_syntax_error_import_binary(self):
         """Importing a binary file shouldn't break SyntaxError formatting"""
         if sys.version_info < (2, 5):