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
This commit is contained in:
Monty Taylor 2019-03-01 13:38:31 +00:00
parent 9cd0ef2ac5
commit a49f2056b2
12 changed files with 137 additions and 65 deletions

View File

@ -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.

44
openstack/_hacking.py Normal file
View File

@ -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)

View File

@ -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)

View File

@ -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()

View File

@ -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()

View File

@ -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',

View File

@ -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()

View File

@ -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'

View File

@ -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

View File

@ -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()

View File

@ -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)

12
tox.ini
View File

@ -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}