Add hacking checks

There have been a handful of changes recently to fix stylistic bugs
in the unit tests (example: If41a7). The changes themselves were fine,
but there are two problems:

1) The stylistic changes they made were not listed in HACKING.rst
2) They were one-time changes; the rules that were violated will
   continue to be violated.

This change updates HACKING.rst to include a few of the stylistic
rules that have been fixed recently, and adds checks to ensure that
they will be enforced going forward. This is based on nova's hacking
checks.

Change-Id: Ic115342605c472f3a5d255aa570ecb60175ca087
This commit is contained in:
Chris Buccella 2014-03-31 04:58:13 +00:00
parent 1069159701
commit 3994eeb862
3 changed files with 111 additions and 0 deletions

View File

@ -8,4 +8,10 @@ glance Style Commandments
glance Specific Commandments
--------------------------
- [G316] Change assertTrue(isinstance(A, B)) by optimal assert like
assertIsInstance(A, B)
- [G317] Change assertEqual(type(A), B) by optimal assert like
assertIsInstance(A, B)
- [G318] Change assertEqual(A, None) or assertEqual(None, A) by optimal assert like
assertIsNone(A)
- [G319] Validate that debug level logs are not translated

View File

@ -12,6 +12,65 @@
# License for the specific language governing permissions and limitations
# under the License.
import re
"""
Guidelines for writing new hacking checks
- Use only for Glance-specific tests. OpenStack general tests
should be submitted to the common 'hacking' module.
- Pick numbers in the range G3xx. Find the current test with
the highest allocated number and then pick the next value.
If nova has an N3xx code for that test, use the same number.
- Keep the test method code in the source file ordered based
on the G3xx value.
- List the new rule in the top level HACKING.rst file
- Add test cases for each new rule to glance/tests/test_hacking.py
"""
asse_trueinst_re = re.compile(
r"(.)*assertTrue\(isinstance\((\w|\.|\'|\"|\[|\])+, "
"(\w|\.|\'|\"|\[|\])+\)\)")
asse_equal_type_re = re.compile(
r"(.)*assertEqual\(type\((\w|\.|\'|\"|\[|\])+\), "
"(\w|\.|\'|\"|\[|\])+\)")
asse_equal_end_with_none_re = re.compile(
r"(.)*assertEqual\((\w|\.|\'|\"|\[|\])+, None\)")
asse_equal_start_with_none_re = re.compile(
r"(.)*assertEqual\(None, (\w|\.|\'|\"|\[|\])+\)")
def assert_true_instance(logical_line):
"""Check for assertTrue(isinstance(a, b)) sentences
G316
"""
if asse_trueinst_re.match(logical_line):
yield (0, "G316: assertTrue(isinstance(a, b)) sentences not allowed")
def assert_equal_type(logical_line):
"""Check for assertEqual(type(A), B) sentences
G317
"""
if asse_equal_type_re.match(logical_line):
yield (0, "G317: assertEqual(type(A), B) sentences not allowed")
def assert_equal_none(logical_line):
"""Check for assertEqual(A, None) or assertEqual(None, A) sentences
G318
"""
res = (asse_equal_start_with_none_re.match(logical_line) or
asse_equal_end_with_none_re.match(logical_line))
if res:
yield (0, "G318: assertEqual(A, None) or assertEqual(None, A) "
"sentences not allowed")
def no_translate_debug_logs(logical_line, filename):
dirs = [
@ -33,4 +92,7 @@ def no_translate_debug_logs(logical_line, filename):
def factory(register):
register(assert_true_instance)
register(assert_equal_type)
register(assert_equal_none)
register(no_translate_debug_logs)

View File

@ -0,0 +1,43 @@
# Copyright 2014 IBM Corp.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
from glance.hacking import checks
from glance.tests import utils
class HackingTestCase(utils.BaseTestCase):
def test_assert_true_instance(self):
self.assertEqual(1, len(list(checks.assert_true_instance(
"self.assertTrue(isinstance(e, "
"exception.BuildAbortException))"))))
self.assertEqual(
0, len(list(checks.assert_true_instance("self.assertTrue()"))))
def test_assert_equal_type(self):
self.assertEqual(1, len(list(checks.assert_equal_type(
"self.assertEqual(type(als['QuicAssist']), list)"))))
self.assertEqual(
0, len(list(checks.assert_equal_type("self.assertTrue()"))))
def test_assert_equal_none(self):
self.assertEqual(1, len(list(checks.assert_equal_none(
"self.assertEqual(A, None)"))))
self.assertEqual(1, len(list(checks.assert_equal_none(
"self.assertEqual(None, A)"))))
self.assertEqual(
0, len(list(checks.assert_equal_none("self.assertIsNone()"))))