Hacking check for str in exception breaks in py34

So change [1] skips this check in python34 tests.

Replace CheckForStrExc with CheckForStrUnicodeEx
and quit skipping unit test for this hacking check.

Partially-Implements: bp py3-compatibility

[1] Id3d196d6006e7b2e5e56343bd7ba207c5196b61d

Change-Id: I9fdd035a61715039b737bf65df5c5636e8a55f54

HACKING.rst

@@ -13,7 +13,7 @@ Manila Specific Commandments
 - [M313] Use assertTrue(...) rather than assertEqual(True, ...).
 - [M319] Validate that debug level logs are not translated.
 - [M323] Ensure that the _() function is explicitly imported to ensure proper translations.
-- [M325] str() cannot be used on an exception.  Remove use or use six.text_type()
+- [M325] str() and unicode() cannot be used on an exception. Remove or use six.text_type().
 - [M326] Translated messages cannot be concatenated.  String should be
   included in translated message.
 - [M328] LOG.critical messages require translations _LC()!

manila/hacking/checks.py

@@ -170,40 +170,51 @@ def check_explicit_underscore_import(logical_line, filename):
         yield(0, "M323: Found use of _() without explicit import of _ !")
 
 
-class CheckForStrExc(BaseASTChecker):
+class CheckForStrUnicodeExc(BaseASTChecker):
-    """Checks for the use of str() on an exception.
+    """Checks for the use of str() or unicode() on an exception.
 
-    This currently only handles the case where str() is used in
+    This currently only handles the case where str() or unicode()
-    the scope of an exception handler.  If the exception is passed
+    is used in the scope of an exception handler.  If the exception
-    into a function, returned from an assertRaises, or used on an
+    is passed into a function, returned from an assertRaises, or
-    exception created in the same scope, this does not catch it.
+    used on an exception created in the same scope, this does not
+    catch it.
     """
 
-    CHECK_DESC = ('M325 str() cannot be used on an exception.  '
+    CHECK_DESC = ('M325 str() and unicode() cannot be used on an '
-                  'Remove or use six.text_type()')
+                  'exception.  Remove or use six.text_type()')
 
     def __init__(self, tree, filename):
-        super(CheckForStrExc, self).__init__(tree, filename)
+        super(CheckForStrUnicodeExc, self).__init__(tree, filename)
         self.name = []
         self.already_checked = []
 
+    # Python 2
     def visit_TryExcept(self, node):
         for handler in node.handlers:
             if handler.name:
                 self.name.append(handler.name.id)
-                super(CheckForStrExc, self).generic_visit(node)
+                super(CheckForStrUnicodeExc, self).generic_visit(node)
                 self.name = self.name[:-1]
             else:
-                super(CheckForStrExc, self).generic_visit(node)
+                super(CheckForStrUnicodeExc, self).generic_visit(node)
+
+    # Python 3
+    def visit_ExceptHandler(self, node):
+        if node.name:
+            self.name.append(node.name)
+            super(CheckForStrUnicodeExc, self).generic_visit(node)
+            self.name = self.name[:-1]
+        else:
+            super(CheckForStrUnicodeExc, self).generic_visit(node)
 
     def visit_Call(self, node):
-        if self._check_call_names(node, ['str']):
+        if self._check_call_names(node, ['str', 'unicode']):
             if node not in self.already_checked:
                 self.already_checked.append(node)
                 if isinstance(node.args[0], ast.Name):
                     if node.args[0].id in self.name:
                         self.add_error(node.args[0])
-        super(CheckForStrExc, self).generic_visit(node)
+        super(CheckForStrUnicodeExc, self).generic_visit(node)
 
 
 class CheckForTransAdd(BaseASTChecker):
@@ -268,8 +279,8 @@ def validate_assertIsNone(logical_line):
 def factory(register):
     register(validate_log_translations)
     register(check_explicit_underscore_import)
+    register(CheckForStrUnicodeExc)
     register(no_translate_debug_logs)
-    register(CheckForStrExc)
     register(CheckForTransAdd)
     register(check_oslo_namespace_imports)
     register(dict_constructor_with_list_copy)

manila/tests/test_hacking.py

@@ -17,8 +17,6 @@ import textwrap
 
 import mock
 import pep8
-import six
-import testtools
 
 from manila.hacking import checks
 from manila import test
@@ -126,10 +124,12 @@ class HackingTestCase(test.TestCase):
                          self._run_check(code, checker, filename)]
         self.assertEqual(expected_errors or [], actual_errors)
 
-    @testtools.skipIf(six.PY3, "It is PY2-specific. Skip it for PY3.")
+    def _assert_has_no_errors(self, code, checker, filename=None):
-    def test_str_exception(self):
+        self._assert_has_errors(code, checker, filename=filename)
 
-        checker = checks.CheckForStrExc
+    def test_str_on_exception(self):
+
+        checker = checks.CheckForStrUnicodeExc
         code = """
                def f(a, b):
                    try:
@@ -141,6 +141,20 @@ class HackingTestCase(test.TestCase):
         errors = [(5, 16, 'M325')]
         self._assert_has_errors(code, checker, expected_errors=errors)
 
+    def test_no_str_unicode_on_exception(self):
+        checker = checks.CheckForStrUnicodeExc
+        code = """
+               def f(a, b):
+                   try:
+                       p = unicode(a) + str(b)
+                   except ValueError as e:
+                       p = e
+                   return p
+               """
+        self._assert_has_no_errors(code, checker)
+
+    def test_unicode_on_exception(self):
+        checker = checks.CheckForStrUnicodeExc
         code = """
                def f(a, b):
                    try:
@@ -149,9 +163,11 @@ class HackingTestCase(test.TestCase):
                        p = unicode(e)
                    return p
                """
-        errors = []
+        errors = [(5, 20, 'M325')]
         self._assert_has_errors(code, checker, expected_errors=errors)
 
+    def test_str_on_multiple_exceptions(self):
+        checker = checks.CheckForStrUnicodeExc
         code = """
                def f(a, b):
                    try:
@@ -161,12 +177,29 @@ class HackingTestCase(test.TestCase):
                            p  = unicode(a) + unicode(b)
                        except ValueError as ve:
                            p = str(e) + str(ve)
-                       p = unicode(e)
+                       p = e
                    return p
                """
         errors = [(8, 20, 'M325'), (8, 29, 'M325')]
         self._assert_has_errors(code, checker, expected_errors=errors)
 
+    def test_str_unicode_on_multiple_exceptions(self):
+        checker = checks.CheckForStrUnicodeExc
+        code = """
+               def f(a, b):
+                   try:
+                       p = str(a) + str(b)
+                   except ValueError as e:
+                       try:
+                           p  = unicode(a) + unicode(b)
+                       except ValueError as ve:
+                           p = str(e) + unicode(ve)
+                       p = str(e)
+                   return p
+               """
+        errors = [(8, 20, 'M325'), (8, 33, 'M325'), (9, 16, 'M325')]
+        self._assert_has_errors(code, checker, expected_errors=errors)
+
     def test_trans_add(self):
 
         checker = checks.CheckForTransAdd