From 73659730ae56d867566a9185d7b2c3491f062c48 Mon Sep 17 00:00:00 2001
From: Vu Cong Tuan <tuanvc@vn.fujitsu.com>
Date: Mon, 5 Jun 2017 13:42:28 +0700
Subject: [PATCH] Define extra hacking rules to ensure code quality

Enforce following codestyle rules:

* no xrange
* no LOG.warn usage (deprecated in favour of LOG.warning)
* usage of assertTrue(x) instead assertEqual(True, x)
* usage of assertIsNone(x) instead assertEqual(None, x)
* usage of assertIsNotNone(x) instead assertNotEqual(None, x)
  or assertIsNot(None, x)

Change-Id: Ib5b61816faae95937c99b1e4f651c6390b888070
---
 monasca_persister/hacking/__init__.py         |  0
 monasca_persister/hacking/checks.py           | 80 +++++++++++++++++++
 .../tests/test_persister_main.py              |  2 +-
 tox.ini                                       | 15 +++-
 4 files changed, 92 insertions(+), 5 deletions(-)
 create mode 100644 monasca_persister/hacking/__init__.py
 create mode 100644 monasca_persister/hacking/checks.py

diff --git a/monasca_persister/hacking/__init__.py b/monasca_persister/hacking/__init__.py
new file mode 100644
index 00000000..e69de29b
diff --git a/monasca_persister/hacking/checks.py b/monasca_persister/hacking/checks.py
new file mode 100644
index 00000000..81b0334f
--- /dev/null
+++ b/monasca_persister/hacking/checks.py
@@ -0,0 +1,80 @@
+# Copyright 2017 FUJITSU LIMITED
+#
+# 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.
+import re
+
+assert_no_xrange_re = re.compile(r"\s*xrange\s*\(")
+assert_True = re.compile(r".*assertEqual\(True, .*\)")
+assert_None = re.compile(r".*assertEqual\(None, .*\)")
+assert_Not_Equal = re.compile(r".*assertNotEqual\(None, .*\)")
+assert_Is_Not = re.compile(r".*assertIsNot\(None, .*\)")
+assert_raises_regexp = re.compile(r"assertRaisesRegexp\(")
+no_log_warn = re.compile(r".*LOG.warn\(.*\)")
+mutable_default_args = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])")
+
+
+def no_mutable_default_args(logical_line):
+    msg = "M001: Method's default argument shouldn't be mutable!"
+    if mutable_default_args.match(logical_line):
+        yield (0, msg)
+
+
+def no_xrange(logical_line):
+    if assert_no_xrange_re.match(logical_line):
+        yield (0, "M002: Do not use xrange().")
+
+
+def validate_assertTrue(logical_line):
+    if re.match(assert_True, logical_line):
+        msg = ("M003: Unit tests should use assertTrue(value) instead"
+               " of using assertEqual(True, value).")
+        yield (0, msg)
+
+
+def validate_assertIsNone(logical_line):
+    if re.match(assert_None, logical_line):
+        msg = ("M004: Unit tests should use assertIsNone(value) instead"
+               " of using assertEqual(None, value).")
+        yield (0, msg)
+
+
+def no_log_warn_check(logical_line):
+    if re.match(no_log_warn, logical_line):
+        msg = ("M005: LOG.warn is deprecated, please use LOG.warning!")
+        yield (0, msg)
+
+
+def validate_assertIsNotNone(logical_line):
+    if re.match(assert_Not_Equal, logical_line) or \
+            re.match(assert_Is_Not, logical_line):
+        msg = ("M006: Unit tests should use assertIsNotNone(value) instead"
+               " of using assertNotEqual(None, value) or"
+               " assertIsNot(None, value).")
+        yield (0, msg)
+
+
+def assert_raisesRegexp(logical_line):
+    res = assert_raises_regexp.search(logical_line)
+    if res:
+        yield (0, "M007: assertRaisesRegex must be used instead "
+                  "of assertRaisesRegexp")
+
+
+def factory(register):
+    register(no_mutable_default_args)
+    register(no_xrange)
+    register(validate_assertTrue)
+    register(validate_assertIsNone)
+    register(no_log_warn_check)
+    register(validate_assertIsNotNone)
+    register(assert_raisesRegexp)
diff --git a/monasca_persister/tests/test_persister_main.py b/monasca_persister/tests/test_persister_main.py
index d71cc2da..63cd51f9 100644
--- a/monasca_persister/tests/test_persister_main.py
+++ b/monasca_persister/tests/test_persister_main.py
@@ -141,7 +141,7 @@ class TestPersister(base.BaseTestCase):
     def test_clean_exit_does_nothing_when_exiting_is_true(self):
         self.persister.exiting = True
 
-        self.assertEqual(None, self.persister.clean_exit(0))
+        self.assertIsNone(self.persister.clean_exit(0))
 
         self.assertFalse(self.mock_sys_exit.called)
 
diff --git a/tox.ini b/tox.ini
index 99556585..99686ef4 100644
--- a/tox.ini
+++ b/tox.ini
@@ -61,14 +61,18 @@ commands =
 [testenv:venv]
 commands = {posargs}
 
+[testenv:bindep]
+# Do not install any requirements. We want this to be fast and work even if
+# system dependencies are missing, since it's used to tell you what system
+# dependencies are missing! This also means that bindep must be installed
+# separately, outside of the requirements files.
+deps = bindep
+commands = bindep test
+
 [testenv:flake8]
 commands =
     flake8 monasca_persister
 
-[testenv:bindep]
-deps = bindep
-commands = bindep test
-
 [flake8]
 max-line-length = 120
 # TODO: ignored checks should be enabled in the future
@@ -80,3 +84,6 @@ exclude=.venv,.git,.tox,dist,*egg,build
 [bandit]
 commands =
   bandit -r monasca_persister -n5 -x monasca_persister/tests
+
+[hacking]
+local-check-factory = monasca_persister.hacking.checks.factory