From 88950e33bace2f7f844cf306d78fbd82d8b31f16 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Fri, 14 Jul 2023 11:31:14 +0100 Subject: [PATCH] tests: Enable SQLAlchemy 2.0 deprecation warnings Well, sort of. We enable them but immediately filter out the ones we're actually seeing, the rationale being that we can address these in a piecemeal fashion without the risk of introducing new issues. There's a lot more to be done here. However, the work done in oslo.db [1], nova [2], cinder [2] etc. should provide a guide for how to resolve the outstanding issues. [1] https://review.opendev.org/q/topic:sqlalchemy-20+project:openstack/oslo.db [2] https://review.opendev.org/q/topic:sqlalchemy-20+project:openstack/nova [3] https://review.opendev.org/q/topic:sqlalchemy-20+project:openstack/cinder Change-Id: If13c14f1d968f1ef968ae440087227691cf966b0 Signed-off-by: Stephen Finucane --- masakari/db/sqlalchemy/api.py | 2 + masakari/test.py | 2 + masakari/tests/base.py | 22 ------ masakari/tests/fixtures.py | 77 +++++++++++++++++++ masakari/tests/functional/base.py | 6 +- tox.ini | 120 ++++++++++++++++-------------- 6 files changed, 147 insertions(+), 82 deletions(-) delete mode 100644 masakari/tests/base.py diff --git a/masakari/db/sqlalchemy/api.py b/masakari/db/sqlalchemy/api.py index cb4f7283..1d7e5088 100644 --- a/masakari/db/sqlalchemy/api.py +++ b/masakari/db/sqlalchemy/api.py @@ -726,6 +726,8 @@ def vmove_delete(context, vmove_uuid): class DeleteFromSelect(sa_sql.expression.UpdateBase): + inherit_cache = False + def __init__(self, table, select, column): self.table = table self.select = select diff --git a/masakari/test.py b/masakari/test.py index e279754a..cc4b19a2 100644 --- a/masakari/test.py +++ b/masakari/test.py @@ -91,6 +91,8 @@ class TestCase(testtools.TestCase): else: self.useFixture(masakari_fixtures.DatabasePoisonFixture()) + self.useFixture(masakari_fixtures.WarningsFixture()) + def stub_out(self, old, new): """Replace a function for the duration of the test. diff --git a/masakari/tests/base.py b/masakari/tests/base.py deleted file mode 100644 index e2a34ade..00000000 --- a/masakari/tests/base.py +++ /dev/null @@ -1,22 +0,0 @@ -# -*- coding: utf-8 -*- - -# Copyright (c) 2016 NTT Data. -# -# 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 oslotest import base - - -class TestCase(base.BaseTestCase): - - """Test case base class for all unit tests.""" diff --git a/masakari/tests/fixtures.py b/masakari/tests/fixtures.py index f55f1ca8..479e3d23 100644 --- a/masakari/tests/fixtures.py +++ b/masakari/tests/fixtures.py @@ -14,8 +14,11 @@ """Fixtures for Masakari tests.""" +import warnings + import fixtures from oslo_config import cfg +from sqlalchemy import exc as sqla_exc from masakari.db import migration from masakari.db.sqlalchemy import api as session @@ -139,3 +142,77 @@ class Database(fixtures.Fixture): super(Database, self).setUp() self.reset() self.addCleanup(self.cleanup) + + +class WarningsFixture(fixtures.Fixture): + """Filters out warnings during test runs.""" + + def setUp(self): + super().setUp() + + self._original_warning_filters = warnings.filters[:] + + warnings.simplefilter('once', DeprecationWarning) + + # The UUIDFields emits a warning if the value is not a valid UUID. + # Let's escalate that to an exception in the test to prevent adding + # violations. + + # warnings.filterwarnings('error', message='.*invalid UUID.*') + + # Enable deprecation warnings for nova itself to capture upcoming + # SQLAlchemy changes + + warnings.filterwarnings( + 'ignore', + category=sqla_exc.SADeprecationWarning, + ) + + warnings.filterwarnings( + 'error', + module='masakari', + category=sqla_exc.SADeprecationWarning, + ) + + warnings.filterwarnings( + 'ignore', + message='The current statement is being autocommitted', + module='masakari', + category=sqla_exc.SADeprecationWarning, + ) + + warnings.filterwarnings( + 'ignore', + message='Using strings to indicate column or relationship paths', + module='masakari', + category=sqla_exc.SADeprecationWarning, + ) + + warnings.filterwarnings( + 'ignore', + message=r'Passing a string to Connection.execute\(\)', + module='masakari', + category=sqla_exc.SADeprecationWarning, + ) + + warnings.filterwarnings( + 'ignore', + message=r'The legacy calling style of select\(\)', + module='masakari', + category=sqla_exc.SADeprecationWarning, + ) + + # Enable general SQLAlchemy warnings also to ensure we're not doing + # silly stuff. It's possible that we'll need to filter things out here + # with future SQLAlchemy versions, but that's a good thing + + warnings.filterwarnings( + 'error', + module='masakari', + category=sqla_exc.SAWarning, + ) + + self.addCleanup(self._reset_warning_filters) + + def _reset_warning_filters(self): + warnings.filters[:] = self._original_warning_filters diff --git a/masakari/tests/functional/base.py b/masakari/tests/functional/base.py index f44e934b..fa210d83 100644 --- a/masakari/tests/functional/base.py +++ b/masakari/tests/functional/base.py @@ -19,9 +19,7 @@ import sys import openstack.config from openstack import connection - -from masakari.tests import base - +from oslotest import base #: Defines the OpenStack Client Config (OCC) cloud key in your OCC config #: file, typically in /etc/openstack/clouds.yaml. That configuration @@ -30,7 +28,7 @@ from masakari.tests import base TEST_CLOUD_NAME = os.getenv('OS_CLOUD', 'devstack-admin') -class BaseFunctionalTest(base.TestCase): +class BaseFunctionalTest(base.BaseTestCase): def setUp(self, ha_api_version="1.0"): super(BaseFunctionalTest, self).setUp() diff --git a/tox.ini b/tox.ini index 472fa1eb..be7b33df 100644 --- a/tox.ini +++ b/tox.ini @@ -1,32 +1,40 @@ [tox] -minversion = 3.1.1 +minversion = 3.18.0 envlist = pep8,py3 -ignore_basepython_conflict = True [testenv] -basepython = python3 usedevelop = True -setenv = VIRTUAL_ENV={envdir} - LANGUAGE=en_US - LC_ALL=en_US.utf-8 +setenv = + VIRTUAL_ENV={envdir} + LANGUAGE=en_US + LC_ALL=en_US.utf-8 +# TODO(stephenfin): Remove once we bump our upper-constraint to SQLAlchemy 2.0 + SQLALCHEMY_WARN_20=1 deps = - -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master} - -r{toxinidir}/requirements.txt - -r{toxinidir}/test-requirements.txt + -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master} + -r{toxinidir}/requirements.txt + -r{toxinidir}/test-requirements.txt commands = stestr run {posargs} -passenv = HTTP_PROXY, HTTPS_PROXY, NO_PROXY, OS_DEBUG, GENERATE_HASHES +passenv = + HTTP_PROXY + HTTPS_PROXY + NO_PROXY + OS_DEBUG + GENERATE_HASHES [testenv:functional] commands = stestr --test-path=./masakari/tests/functional run --concurrency=1 --slowest {posargs} [testenv:genconfig] -commands = oslo-config-generator --config-file=etc/masakari/masakari-config-generator.conf - oslo-config-generator --config-file=etc/masakari/masakari-customized-recovery-flow-config-generator.conf +commands = + oslo-config-generator --config-file=etc/masakari/masakari-config-generator.conf + oslo-config-generator --config-file=etc/masakari/masakari-customized-recovery-flow-config-generator.conf [testenv:genpolicy] -commands = oslopolicy-sample-generator --config-file=etc/masakari/masakari-policy-generator.conf +commands = + oslopolicy-sample-generator --config-file=etc/masakari/masakari-policy-generator.conf [testenv:linters] skip_install = True @@ -72,21 +80,22 @@ commands = {posargs} [testenv:cover] setenv = - VIRTUAL_ENV={envdir} - PYTHON=coverage run --source masakari --parallel-mode + VIRTUAL_ENV={envdir} + PYTHON=coverage run --source masakari --parallel-mode commands = - stestr run {posargs} - coverage combine - coverage html -d cover - coverage xml -o cover/coverage.xml + stestr run {posargs} + coverage combine + coverage html -d cover + coverage xml -o cover/coverage.xml [testenv:docs] # NOTE(elod.illes): requirements.txt is needed because otherwise # dependencies are installed during 'develop-inst' tox phase without # constraints which could cause failures in stable branches. -deps = -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master} - -r{toxinidir}/requirements.txt - -r{toxinidir}/doc/requirements.txt +deps = + -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master} + -r{toxinidir}/requirements.txt + -r{toxinidir}/doc/requirements.txt commands = sphinx-build -W -b html doc/source doc/build/html @@ -100,12 +109,13 @@ commands = [testenv:releasenotes] skip_install = True -deps = -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master} - -r{toxinidir}/doc/requirements.txt +deps = + -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master} + -r{toxinidir}/doc/requirements.txt allowlist_externals = rm commands = - rm -fr releasenotes/build + rm -rf releasenotes/build sphinx-build -a -E -W -d releasenotes/build/doctrees -b html releasenotes/source releasenotes/build/html [testenv:debug] @@ -122,10 +132,8 @@ commands = sphinx-build -W -b html -d api-ref/build/doctrees api-ref/source api-ref/build/html [flake8] -# E123, E125 skipped as they are invalid PEP-8. - show-source = True - +# E123, E125 skipped as they are invalid PEP-8. # The below hacking rules by default are disabled should be enabled: # [H106] Don't put vim configuration in source files. # [H203] Use assertIs(Not)None to check for None. @@ -134,40 +142,40 @@ enable-extensions = H106,H203,H904 # [W504] line break after binary operator (use W503 instead) ignore = E123,E125,E128,E731,H405,W504 builtins = _ -exclude=.venv,.git,.tox,dist,doc,*lib/python*,*egg,build +exclude = .venv,.git,.tox,dist,doc,*lib/python*,*egg,build [hacking] import_exceptions = masakari.i18n [flake8:local-plugins] extension = - M301 = checks:no_db_session_in_public_api - M302 = checks:use_timeutils_utcnow - M303 = checks:capital_cfg_help - M305 = checks:assert_true_instance - M306 = checks:assert_equal_type - M308 = checks:no_translate_logs - M309 = checks:no_import_translation_in_tests - M310 = checks:no_setting_conf_directly_in_tests - M315 = checks:no_mutable_default_args - M316 = checks:check_explicit_underscore_import - M317 = checks:use_jsonutils - M318 = checks:assert_true_or_false_with_in - M319 = checks:assert_raises_regexp - M320 = checks:dict_constructor_with_list_copy - M321 = checks:assert_equal_in - M322 = checks:check_greenthread_spawns - M323 = checks:check_no_contextlib_nested - M324 = checks:check_config_option_in_central_place - M325 = checks:check_doubled_words - M326 = checks:check_python3_no_iteritems - M327 = checks:check_python3_no_iterkeys - M328 = checks:check_python3_no_itervalues - M329 = checks:no_os_popen - M331 = checks:no_log_warn - M332 = checks:yield_followed_by_space - M333 = checks:check_policy_registration_in_central_place - M334 = checks:check_policy_enforce + M301 = checks:no_db_session_in_public_api + M302 = checks:use_timeutils_utcnow + M303 = checks:capital_cfg_help + M305 = checks:assert_true_instance + M306 = checks:assert_equal_type + M308 = checks:no_translate_logs + M309 = checks:no_import_translation_in_tests + M310 = checks:no_setting_conf_directly_in_tests + M315 = checks:no_mutable_default_args + M316 = checks:check_explicit_underscore_import + M317 = checks:use_jsonutils + M318 = checks:assert_true_or_false_with_in + M319 = checks:assert_raises_regexp + M320 = checks:dict_constructor_with_list_copy + M321 = checks:assert_equal_in + M322 = checks:check_greenthread_spawns + M323 = checks:check_no_contextlib_nested + M324 = checks:check_config_option_in_central_place + M325 = checks:check_doubled_words + M326 = checks:check_python3_no_iteritems + M327 = checks:check_python3_no_iterkeys + M328 = checks:check_python3_no_itervalues + M329 = checks:no_os_popen + M331 = checks:no_log_warn + M332 = checks:yield_followed_by_space + M333 = checks:check_policy_registration_in_central_place + M334 = checks:check_policy_enforce paths = ./masakari/hacking [testenv:bindep]