Get rid of setUpClass and block it for forever

setUpClass is evil. It lures you in to a sense of complacency thinking
that reality exists and makes sense. But it's all just a cruel joke
designed to increase your personal karmic debt.

In order that we might collectively one day reach nirvana, remove all
senseless and misleading instances of setUpClass. Then add local hacking
checks to prevent the introduction of such scourge ever again.

Change-Id: Ifd43958bf47981aedad639bef61769ddb37618d3

HACKING.rst

@@ -51,3 +51,13 @@ in a way that protects against local environment.
 
 Test cases should use requests-mock to mock out HTTP interactions rather than
 using mock to mock out object access.
+
+Don't Use setUpClass
+--------------------
+
+setUpClass looks like it runs once for the class. In parallel test execution
+environments though, it runs once per execution context. This makes reasoning
+about when it is going to actually run and what is going to happen extremely
+difficult and can produce hard to debug test issues.
+
+Don't ever use it. It makes baby pandas cry.

openstack/_hacking.py

@@ -0,0 +1,44 @@
+# Copyright (c) 2019, Red Hat, Inc.
+#
+# 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
+
+"""
+Guidelines for writing new hacking checks
+
+ - Use only for openstacksdk specific tests. OpenStack general tests
+   should be submitted to the common 'hacking' module.
+ - Pick numbers in the range O3xx. Find the current test with
+   the highest allocated number and then pick the next value.
+ - Keep the test method code in the source file ordered based
+   on the O3xx value.
+ - List the new rule in the top level HACKING.rst file
+ - Add test cases for each new rule to nova/tests/unit/test_hacking.py
+
+"""
+
+SETUPCLASS_RE = re.compile(r"def setUpClass\(")
+
+
+def assert_no_setupclass(logical_line):
+    """Check for use of setUpClass
+
+    O300
+    """
+    if SETUPCLASS_RE.match(logical_line):
+        yield (0, "O300: setUpClass not allowed")
+
+
+def factory(register):
+    register(assert_no_setupclass)

openstack/tests/functional/base.py

@@ -45,14 +45,7 @@ FLAVOR_NAME = _get_resource_value('flavor_name', 'm1.small')
 
 class BaseFunctionalTest(base.TestCase):
 
-    @classmethod
-    def setUpClass(cls):
-        super(BaseFunctionalTest, cls).setUpClass()
-        # Defines default timeout for wait_for methods used
-        # in the functional tests
-        cls._wait_for_timeout = int(os.getenv(
-            'OPENSTACKSDK_FUNC_TEST_TIMEOUT',
-            300))
+    _wait_for_timeout_key = ''
 
     def setUp(self):
         super(BaseFunctionalTest, self).setUp()
@@ -70,6 +63,12 @@ class BaseFunctionalTest(base.TestCase):
         self.identity_version = \
             self.operator_cloud.config.get_api_version('identity')
 
+        # Defines default timeout for wait_for methods used
+        # in the functional tests
+        self._wait_for_timeout = int(
+            os.getenv(self._wait_for_timeout_key, os.getenv(
+                'OPENSTACKSDK_FUNC_TEST_TIMEOUT', 300)))
+
     def _set_user_cloud(self, **kwargs):
         user_config = self.config.get_one(
             cloud=self._demo_name, **kwargs)

openstack/tests/functional/block_storage/v2/base.py

@@ -10,19 +10,12 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
-import os
-
 from openstack.tests.functional import base
 
 
 class BaseBlockStorageTest(base.BaseFunctionalTest):
 
-    @classmethod
-    def setUpClass(cls):
-        super(BaseBlockStorageTest, cls).setUpClass()
-        cls._wait_for_timeout = int(os.getenv(
-            'OPENSTACKSDK_FUNC_TEST_TIMEOUT_BLOCK_STORAGE',
-            cls._wait_for_timeout))
+    _wait_for_timeout_key = 'OPENSTACKSDK_FUNC_TEST_TIMEOUT_BLOCK_STORAGE'
 
     def setUp(self):
         super(BaseBlockStorageTest, self).setUp()

openstack/tests/functional/block_storage/v3/base.py

@@ -10,19 +10,12 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
-import os
-
 from openstack.tests.functional import base
 
 
 class BaseBlockStorageTest(base.BaseFunctionalTest):
 
-    @classmethod
-    def setUpClass(cls):
-        super(BaseBlockStorageTest, cls).setUpClass()
-        cls._wait_for_timeout = int(os.getenv(
-            'OPENSTACKSDK_FUNC_TEST_TIMEOUT_BLOCK_STORAGE',
-            cls._wait_for_timeout))
+    _wait_for_timeout_key = 'OPENSTACKSDK_FUNC_TEST_TIMEOUT_BLOCK_STORAGE'
 
     def setUp(self):
         super(BaseBlockStorageTest, self).setUp()

openstack/tests/functional/block_store/v2/test_stats.py

@@ -17,12 +17,11 @@ from openstack.tests.functional import base
 
 class TestStats(base.BaseFunctionalTest):
 
-    @classmethod
-    def setUpClass(cls):
-        super(TestStats, cls).setUpClass()
-        sot = cls.conn.block_storage.backend_pools()
+    def setUp(self):
+        super(TestStats, self).setUp()
+        sot = self.conn.block_storage.backend_pools()
         for pool in sot:
-            assert isinstance(pool, _stats.Pools)
+            self.assertIsInstance(pool, _stats.Pools)
 
     def test_list(self):
         capList = ['volume_backend_name', 'storage_protocol',

openstack/tests/functional/clustering/test_cluster.py

@@ -10,7 +10,6 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
-import os
 import time
 
 from openstack.clustering.v1 import cluster
@@ -20,12 +19,7 @@ from openstack.tests.functional.network.v2 import test_network
 
 class TestCluster(base.BaseFunctionalTest):
 
-    @classmethod
-    def setUpClass(cls):
-        super(TestCluster, cls).setUpClass()
-        cls._wait_for_timeout = int(os.getenv(
-            'OPENSTACKSDK_FUNC_TEST_TIMEOUT_CLUSTER',
-            cls._wait_for_timeout))
+    _wait_for_timeout_key = 'OPENSTACKSDK_FUNC_TEST_TIMEOUT_CLUSTER'
 
     def setUp(self):
         super(TestCluster, self).setUp()

openstack/tests/functional/compute/base.py

@@ -10,16 +10,9 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
-import os
-
 from openstack.tests.functional import base
 
 
 class BaseComputeTest(base.BaseFunctionalTest):
 
-    @classmethod
-    def setUpClass(cls):
-        super(BaseComputeTest, cls).setUpClass()
-        cls._wait_for_timeout = int(os.getenv(
-            'OPENSTACKSDK_FUNC_TEST_TIMEOUT_COMPUTE',
-            cls._wait_for_timeout))
+    _wait_for_timeout_key = 'OPENSTACKSDK_FUNC_TEST_TIMEOUT_COMPUTE'

openstack/tests/functional/load_balancer/v2/test_load_balancer.py

@@ -10,8 +10,6 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
-import os
-
 from openstack.load_balancer.v2 import flavor
 from openstack.load_balancer.v2 import flavor_profile
 from openstack.load_balancer.v2 import health_monitor
@@ -56,12 +54,7 @@ class TestLoadBalancer(base.BaseFunctionalTest):
     FLAVOR_DATA = '{"loadbalancer_topology": "SINGLE"}'
     DESCRIPTION = 'Test description'
 
-    @classmethod
-    def setUpClass(cls):
-        super(TestLoadBalancer, cls).setUpClass()
-        cls._wait_for_timeout = int(os.getenv(
-            'OPENSTACKSDK_FUNC_TEST_TIMEOUT_LOAD_BALANCER',
-            cls._wait_for_timeout))
+    _wait_for_timeout_key = 'OPENSTACKSDK_FUNC_TEST_TIMEOUT_LOAD_BALANCER'
 
     # TODO(shade): Creating load balancers can be slow on some hosts due to
     #              nova instance boot times (up to ten minutes). This used to

openstack/tests/functional/orchestration/v1/test_stack.py

@@ -10,7 +10,6 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
-import os
 import yaml
 
 from openstack import exceptions
@@ -27,12 +26,7 @@ class TestStack(base.BaseFunctionalTest):
     subnet = None
     cidr = '10.99.99.0/16'
 
-    @classmethod
-    def setUpClass(cls):
-        super(TestStack, cls).setUpClass()
-        cls._wait_for_timeout = int(os.getenv(
-            'OPENSTACKSDK_FUNC_TEST_TIMEOUT_ORCHESTRATION',
-            cls._wait_for_timeout))
+    _wait_for_timeout_key = 'OPENSTACKSDK_FUNC_TEST_TIMEOUT_ORCHESTRATION'
 
     def setUp(self):
         super(TestStack, self).setUp()

openstack/tests/unit/test_hacking.py

@@ -0,0 +1,60 @@
+# Copyright 2019 Red Hat, Inc.
+#
+# 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 openstack import _hacking
+from openstack.tests.unit import base
+
+
+class HackingTestCase(base.TestCase):
+    """This class tests the hacking checks in openstack._hacking.checks.
+
+    It works by passing strings to the check methods like the pep8/flake8
+    parser would. The parser loops over each line in the file and then passes
+    the parameters to the check method. The parameter names in the check method
+    dictate what type of object is passed to the check method.
+
+    The parameter types are::
+
+        logical_line: A processed line with the following modifications:
+            - Multi-line statements converted to a single line.
+            - Stripped left and right.
+            - Contents of strings replaced with "xxx" of same length.
+            - Comments removed.
+        physical_line: Raw line of text from the input file.
+        lines: a list of the raw lines from the input file
+        tokens: the tokens that contribute to this logical line
+        line_number: line number in the input file
+        total_lines: number of lines in the input file
+        blank_lines: blank lines before this one
+        indent_char: indentation character in this file (" " or "\t")
+        indent_level: indentation (with tabs expanded to multiples of 8)
+        previous_indent_level: indentation on previous line
+        previous_logical: previous logical line
+        filename: Path of the file being run through pep8
+
+    When running a test on a check method the return will be False/None if
+    there is no violation in the sample input. If there is an error a tuple is
+    returned with a position in the line, and a message. So to check the result
+    just assertTrue if the check is expected to fail and assertFalse if it
+    should pass.
+    """
+    def test_assert_no_setupclass(self):
+        self.assertEqual(len(list(_hacking.assert_no_setupclass(
+            "def setUpClass(cls)"))), 1)
+
+        self.assertEqual(len(list(_hacking.assert_no_setupclass(
+            "# setUpClass is evil"))), 0)
+
+        self.assertEqual(len(list(_hacking.assert_no_setupclass(
+            "def setUpClassyDrinkingLocation(cls)"))), 0)

tox.ini

@@ -39,18 +39,18 @@ commands = stestr --test-path ./openstack/tests/functional/{env:OPENSTACKSDK_TES
            stestr slowest
 
 [testenv:pep8]
-usedevelop = False
-skip_install = True
 deps =
-    -c{env:UPPER_CONSTRAINTS_FILE:https://git.openstack.org/cgit/openstack/requirements/plain/upper-constraints.txt}
-    doc8
-    flake8
+    {[testenv]deps}
     hacking
+    doc8
     pygments
     readme
 commands =
-    doc8 doc/source
     flake8
+    doc8 doc/source
+
+[hacking]
+local-check-factory = openstack._hacking.factory
 
 [testenv:venv]
 commands = {posargs}