Merge "Add REVIEWING documentation"
diff --git a/lower-constraints.txt b/lower-constraints.txt
index 9422676..2b77dff 100644
--- a/lower-constraints.txt
+++ b/lower-constraints.txt
@@ -16,7 +16,7 @@
 extras==1.0.0
 fasteners==0.14.1
 fixtures==3.0.0
-flake8==2.5.5
+flake8==2.6.2
 future==0.16.0
 hacking==1.0.0
 idna==2.6
diff --git a/patrole_tempest_plugin/hacking/checks.py b/patrole_tempest_plugin/hacking/checks.py
index eb73ef1..d106da8 100644
--- a/patrole_tempest_plugin/hacking/checks.py
+++ b/patrole_tempest_plugin/hacking/checks.py
@@ -17,7 +17,7 @@
 import os
 import re
 
-import pep8
+import pycodestyle
 
 
 PYTHON_CLIENTS = ['cinder', 'glance', 'keystone', 'nova', 'swift', 'neutron',
@@ -59,7 +59,7 @@
 
     T105: Tests cannot use setUpClass/tearDownClass
     """
-    if pep8.noqa(physical_line):
+    if pycodestyle.noqa(physical_line):
         return
 
     if SETUP_TEARDOWN_CLASS_DEFINITION.match(physical_line):
diff --git a/patrole_tempest_plugin/policy_authority.py b/patrole_tempest_plugin/policy_authority.py
index b813f88..3339a5d 100644
--- a/patrole_tempest_plugin/policy_authority.py
+++ b/patrole_tempest_plugin/policy_authority.py
@@ -136,7 +136,7 @@
 
         if not service or service not in cls.available_services:
             LOG.debug("%s is NOT a valid service.", service)
-            raise rbac_exceptions.RbacInvalidService(
+            raise rbac_exceptions.RbacInvalidServiceException(
                 "%s is NOT a valid service." % service)
 
     @classmethod
diff --git a/patrole_tempest_plugin/rbac_exceptions.py b/patrole_tempest_plugin/rbac_exceptions.py
index 980672a..809a7ed 100644
--- a/patrole_tempest_plugin/rbac_exceptions.py
+++ b/patrole_tempest_plugin/rbac_exceptions.py
@@ -64,7 +64,10 @@
                "instead. Actual exception: %(exception)s")
 
 
-class RbacInvalidService(exceptions.TempestException):
+class RbacInvalidServiceException(exceptions.TempestException):
+    """Raised when an invalid service is passed to ``rbac_rule_validation``
+    decorator.
+    """
     message = "Attempted to test an invalid service"
 
 
diff --git a/patrole_tempest_plugin/rbac_rule_validation.py b/patrole_tempest_plugin/rbac_rule_validation.py
index 2f6759d..e6d1e80 100644
--- a/patrole_tempest_plugin/rbac_rule_validation.py
+++ b/patrole_tempest_plugin/rbac_rule_validation.py
@@ -22,7 +22,7 @@
 import six
 
 from tempest import config
-from tempest.lib import exceptions
+from tempest.lib import exceptions as lib_exc
 from tempest import test
 
 from patrole_tempest_plugin import policy_authority
@@ -123,7 +123,7 @@
                 "os_alt.auth_provider.credentials.user_id"
             })
 
-    :raises NotFound: If ``service`` is invalid.
+    :raises RbacInvalidServiceException: If ``service`` is invalid.
     :raises RbacUnderPermissionException: For item (2) above.
     :raises RbacOverPermissionException: For item (3) above.
     :raises RbacExpectedWrongException: When a 403 is expected but a 404
@@ -184,12 +184,13 @@
 
             try:
                 test_func(*args, **kwargs)
-            except rbac_exceptions.RbacInvalidService as e:
-                msg = ("%s is not a valid service." % service)
-                test_status = ('Error, %s' % (msg))
-                LOG.error(msg)
-                raise exceptions.NotFound(
-                    "%s RbacInvalidService was: %s" % (msg, e))
+            except rbac_exceptions.RbacInvalidServiceException:
+                with excutils.save_and_reraise_exception():
+                    msg = ("%s is not a valid service." % service)
+                    # FIXME(felipemonteiro): This test_status is logged too
+                    # late. Need a function to log it before re-raising.
+                    test_status = ('Error, %s' % (msg))
+                    LOG.error(msg)
             except (expected_exception,
                     rbac_exceptions.RbacConflictingPolicies,
                     rbac_exceptions.RbacMalformedResponse) as e:
@@ -382,14 +383,13 @@
         raise rbac_exceptions.RbacInvalidErrorCode(msg)
 
     if expected_error_code == 403:
-        expected_exception = exceptions.Forbidden
+        expected_exception = lib_exc.Forbidden
     elif expected_error_code == 404:
-        expected_exception = exceptions.NotFound
+        expected_exception = lib_exc.NotFound
         irregular_msg = ("NotFound exception was caught for test %s. Expected "
                          "policies which may have caused the error: %s. The "
                          "service %s throws a 404 instead of a 403, which is "
                          "irregular.")
-
     return expected_exception, irregular_msg
 
 
@@ -431,7 +431,7 @@
 
 def _check_for_expected_mismatch_exception(expected_exception,
                                            actual_exception):
-    permission_exceptions = (exceptions.Forbidden, exceptions.NotFound)
+    permission_exceptions = (lib_exc.Forbidden, lib_exc.NotFound)
     if isinstance(actual_exception, permission_exceptions):
         if not isinstance(actual_exception, expected_exception.__class__):
             return True
diff --git a/patrole_tempest_plugin/tests/unit/test_policy_authority.py b/patrole_tempest_plugin/tests/unit/test_policy_authority.py
index 3e2cc4c..d396a29 100644
--- a/patrole_tempest_plugin/tests/unit/test_policy_authority.py
+++ b/patrole_tempest_plugin/tests/unit/test_policy_authority.py
@@ -238,7 +238,7 @@
         test_user_id = mock.sentinel.user_id
         service = 'invalid_service'
 
-        self.assertRaises(rbac_exceptions.RbacInvalidService,
+        self.assertRaises(rbac_exceptions.RbacInvalidServiceException,
                           policy_authority.PolicyAuthority,
                           test_tenant_id,
                           test_user_id,
@@ -253,7 +253,7 @@
         test_user_id = mock.sentinel.user_id
         service = None
 
-        self.assertRaises(rbac_exceptions.RbacInvalidService,
+        self.assertRaises(rbac_exceptions.RbacInvalidServiceException,
                           policy_authority.PolicyAuthority,
                           test_tenant_id,
                           test_user_id,
@@ -528,8 +528,9 @@
             policy_parser = None
 
             expected_exception = 'invalid_service is NOT a valid service'
-            with self.assertRaisesRegex(rbac_exceptions.RbacInvalidService,
-                                        expected_exception):
+            with self.assertRaisesRegex(
+                    rbac_exceptions.RbacInvalidServiceException,
+                    expected_exception):
                 policy_authority.PolicyAuthority(
                     test_tenant_id, test_user_id, "INVALID_SERVICE")
         else:
diff --git a/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py b/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
index 2e275dc..1bf5510 100644
--- a/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
+++ b/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
@@ -430,6 +430,22 @@
             "Allowed")
 
 
+class RBACRuleValidationNegativeTest(BaseRBACRuleValidationTest):
+
+    @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
+    def test_rule_validation_invalid_service_raises_exc(self, mock_authority):
+        """Test that invalid service raises the appropriate exception."""
+        mock_authority.PolicyAuthority.return_value.allowed.side_effect = (
+            rbac_exceptions.RbacInvalidServiceException)
+
+        @rbac_rv.action(mock.sentinel.service, mock.sentinel.action)
+        def test_policy(*args):
+            pass
+
+        self.assertRaises(rbac_exceptions.RbacInvalidServiceException,
+                          test_policy, self.mock_test_args)
+
+
 class RBACRuleValidationTestMultiPolicy(BaseRBACRuleValidationTest):
     """Test suite for validating multi-policy support for the
     ``rbac_rule_validation`` decorator.
diff --git a/test-requirements.txt b/test-requirements.txt
index 35e4e57..9085c07 100644
--- a/test-requirements.txt
+++ b/test-requirements.txt
@@ -1,7 +1,7 @@
 # The order of packages is significant, because pip processes them in the order
 # of appearance. Changing the order has an impact on the overall integration
 # process, which may cause wedges in the gate later.
-hacking>=1.0.0,<1.1.0 # Apache-2.0
+hacking>=1.1.0,<1.2.0 # Apache-2.0
 fixtures>=3.0.0 # Apache-2.0/BSD
 mock>=2.0.0 # BSD
 coverage!=4.4,>=4.0 # Apache-2.0