diff --git a/HACKING.rst b/HACKING.rst index df3cec31d3..73cf0ef8d1 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -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 diff --git a/glance/hacking/checks.py b/glance/hacking/checks.py index 44108e389e..8f00eba033 100644 --- a/glance/hacking/checks.py +++ b/glance/hacking/checks.py @@ -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) diff --git a/glance/tests/test_hacking.py b/glance/tests/test_hacking.py new file mode 100644 index 0000000000..5cf875a6c5 --- /dev/null +++ b/glance/tests/test_hacking.py @@ -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()"))))