From 2bec119fb185675c5fcd7f73f5e4c42d7210c26f Mon Sep 17 00:00:00 2001
From: Andrey Kurilin <akurilin@mirantis.com>
Date: Tue, 29 Dec 2015 16:47:25 +0200
Subject: [PATCH] Split hacking check for db and objects call from cli

This hacking rule checks different imports, so it should be splitted
to separete rules.

Also, this patch:
 - fixes unit tests for these rules(previously it do not check anything)
 - removes db call from ShowCommand

Change-Id: I3dbb27cd7d53eabee13f679e5ab6ed17819c3f3e
---
 tests/hacking/README.rst   |  4 +++-
 tests/hacking/checks.py    | 37 ++++++++++++++++++++++++++-----------
 tests/unit/test_hacking.py | 25 ++++++++++++++++++-------
 3 files changed, 47 insertions(+), 19 deletions(-)

diff --git a/tests/hacking/README.rst b/tests/hacking/README.rst
index 437b5120..d1a4fa73 100644
--- a/tests/hacking/README.rst
+++ b/tests/hacking/README.rst
@@ -28,4 +28,6 @@ Rally Specific Commandments
 * [N352] - Ensure that string formatting only uses a mapping if multiple mapping keys are used.
 * [N353] - Ensure that unicode() function is not uset because of absence in py3
 * [N354] - Ensure that ``:raises: Exception`` is not used
-* [N355] - Ensure that CLI modules do not work with ``rally.common.db`` and ``rally.common.objects``
+* [N360-N370] - Reserved for rules related to CLI
+ * [N360] - Ensure that CLI modules do not use ``rally.common.db``
+ * [N361] - Ensure that CLI modules do not use ``rally.common.objects``
diff --git a/tests/hacking/checks.py b/tests/hacking/checks.py
index 9d4ece13..5f1db629 100644
--- a/tests/hacking/checks.py
+++ b/tests/hacking/checks.py
@@ -61,8 +61,8 @@ re_str_format = re.compile(r"""
 """, re.X)
 re_raises = re.compile(
     r"\s:raise[^s] *.*$|\s:raises *:.*$|\s:raises *[^:]+$")
-re_import_common_db_or_objects = re.compile(
-    r"^from rally.common import (db|objects)")
+re_db_import = re.compile(r"^from rally.common import db")
+re_objects_import = re.compile(r"^from rally.common import objects")
 
 
 def skip_ignored_lines(func):
@@ -465,17 +465,31 @@ def check_raises(physical_line, filename):
 
 
 @skip_ignored_lines
-def check_import_of_cli(logical_line, filename):
-    """Check imports of CLI module
+def check_db_imports_in_cli(logical_line, filename):
+    """Ensure that CLI modules do not use ``rally.common.db``
 
-    N355
+    N360
     """
-    ignored_files = ["./rally/cli/manage.py", "./rally/cli/commands/show.py"]
+    if (not filename.startswith("./rally/cli")
+            or filename == "./rally/cli/manage.py"):
+        return
+    if re_db_import.search(logical_line):
+        yield (0, "N360 CLI modules do not allow to work with "
+                  "`rally.common.db``.")
 
-    if filename.startswith("./rally/cli") and filename not in ignored_files:
-        if re_import_common_db_or_objects.search(logical_line):
-            yield (0, "N355 CLI modules do not allow to work with "
-                      "`rally.common.db`` and ``rally.common.objects`.")
+
+@skip_ignored_lines
+def check_objects_imports_in_cli(logical_line, filename):
+    """Ensure that CLI modules do not use ``rally.common.objects``
+
+    N361
+    """
+    if (not filename.startswith("./rally/cli")
+            or filename == "./rally/cli/commands/show.py"):
+        return
+    if re_objects_import.search(logical_line):
+        yield (0, "N360 CLI modules do not allow to work with "
+                  "`rally.common.objects``.")
 
 
 def factory(register):
@@ -495,4 +509,5 @@ def factory(register):
     register(check_dict_formatting_in_string)
     register(check_using_unicode)
     register(check_raises)
-    register(check_import_of_cli)
+    register(check_db_imports_in_cli)
+    register(check_objects_imports_in_cli)
diff --git a/tests/unit/test_hacking.py b/tests/unit/test_hacking.py
index de810976..3ae338bf 100644
--- a/tests/unit/test_hacking.py
+++ b/tests/unit/test_hacking.py
@@ -329,11 +329,22 @@ class HackingTestCase(test.TestCase):
             "text = :raises Exception: if conditions", "fakefile")
         self.assertIsNone(checkres)
 
-    def test_check_import_of_cli(self):
-        checkres = checks.check_import_of_cli(
-            "from rally.common import db", "fakefile")
-        self.assertIsNotNone(checkres)
+    def test_check_db_imports_of_cli(self):
+        next(checks.check_db_imports_in_cli(
+            "from rally.common import db",
+            "./rally/cli/filename"))
 
-        checkres = checks.check_import_of_cli(
-            "from rally.common import objects", "fakefile")
-        self.assertIsNotNone(checkres)
+        checkres = checks.check_db_imports_in_cli(
+            "from rally.common import db",
+            "./filename")
+        self.assertRaises(StopIteration, next, checkres)
+
+    def test_check_objects_imports_of_cli(self):
+        next(checks.check_objects_imports_in_cli(
+            "from rally.common import objects",
+            "./rally/cli/filename"))
+
+        checkres = checks.check_objects_imports_in_cli(
+            "from rally.common import objects",
+            "./filename")
+        self.assertRaises(StopIteration, next, checkres)