From 0dc9747519cb352dec099302be6d0173837afd18 Mon Sep 17 00:00:00 2001
From: Ken'ichi Ohmichi <ken-oomichi@wx.jp.nec.com>
Date: Fri, 25 Mar 2016 15:10:08 -0700
Subject: [PATCH] Add pep8 check for tempest.lib import

tempest.lib should not import local tempest code to avoid circular dependency,
so this patch adds pep8 check to block such kind of code.

Change-Id: I392d28b3195040a800d96171ef275c6e73f9fef4
---
 HACKING.rst                   |  1 +
 tempest/hacking/checks.py     | 22 ++++++++++++++++++++++
 tempest/tests/test_hacking.py | 20 ++++++++++++++++++++
 3 files changed, 43 insertions(+)

diff --git a/HACKING.rst b/HACKING.rst
index efabaf69b1..44519d4c9a 100644
--- a/HACKING.rst
+++ b/HACKING.rst
@@ -19,6 +19,7 @@ Tempest Specific Commandments
          decorators.skip_because from tempest-lib
 - [T110] Check that service client names of GET should be consistent
 - [T111] Check that service client names of DELETE should be consistent
+- [T112] Check that tempest.lib should not import local tempest code
 - [N322] Method's default argument shouldn't be mutable
 
 Test Data/Configuration
diff --git a/tempest/hacking/checks.py b/tempest/hacking/checks.py
index c666c96e21..f4b76e5e9e 100644
--- a/tempest/hacking/checks.py
+++ b/tempest/hacking/checks.py
@@ -225,6 +225,27 @@ def delete_resources_on_service_clients(logical_line, physical_line, filename,
         yield (0, msg)
 
 
+def dont_import_local_tempest_into_lib(logical_line, filename):
+    """Check that tempest.lib should not import local tempest code
+
+    T112
+    """
+    if 'tempest/lib/' not in filename:
+        return
+
+    if not ('from tempest' in logical_line
+            or 'import tempest' in logical_line):
+        return
+
+    if ('from tempest.lib' in logical_line
+        or 'import tempest.lib' in logical_line):
+        return
+
+    msg = ("T112: tempest.lib should not import local tempest code to avoid "
+           "circular dependency")
+    yield (0, msg)
+
+
 def factory(register):
     register(import_no_clients_in_api_and_scenario_tests)
     register(scenario_tests_need_service_tags)
@@ -236,3 +257,4 @@ def factory(register):
     register(no_testtools_skip_decorator)
     register(get_resources_on_service_clients)
     register(delete_resources_on_service_clients)
+    register(dont_import_local_tempest_into_lib)
diff --git a/tempest/tests/test_hacking.py b/tempest/tests/test_hacking.py
index 55f00efa4b..aba2aabce3 100644
--- a/tempest/tests/test_hacking.py
+++ b/tempest/tests/test_hacking.py
@@ -147,3 +147,23 @@ class HackingTestCase(base.TestCase):
             " @testtools.skipUnless(CONF.something, 'msg')"))))
         self.assertEqual(0, len(list(checks.no_testtools_skip_decorator(
             " @testtools.skipIf(CONF.something, 'msg')"))))
+
+    def test_dont_import_local_tempest_code_into_lib(self):
+        self.assertEqual(0, len(list(checks.dont_import_local_tempest_into_lib(
+            "from tempest.common import waiters",
+            './tempest/common/compute.py'))))
+        self.assertEqual(0, len(list(checks.dont_import_local_tempest_into_lib(
+            "from tempest import config",
+            './tempest/common/compute.py'))))
+        self.assertEqual(0, len(list(checks.dont_import_local_tempest_into_lib(
+            "import tempest.exception",
+            './tempest/common/compute.py'))))
+        self.assertEqual(1, len(list(checks.dont_import_local_tempest_into_lib(
+            "from tempest.common import waiters",
+            './tempest/lib/common/compute.py'))))
+        self.assertEqual(1, len(list(checks.dont_import_local_tempest_into_lib(
+            "from tempest import config",
+            './tempest/lib/common/compute.py'))))
+        self.assertEqual(1, len(list(checks.dont_import_local_tempest_into_lib(
+            "import tempest.exception",
+            './tempest/lib/common/compute.py'))))