From a75605d01832291444ec1072578ec7cc078a549e Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Tue, 28 Jan 2025 15:32:35 +0100 Subject: [PATCH] Cleanup tox configs and unit tests (#2219) SUMMARY As discussed in #2212 cleans up the unit tests to move importing some of the fixtures into conftest.py tox configs needed a little cleanup to ensure that things were consistently in the import path. ISSUE TYPE Feature Pull Request COMPONENT NAME pyproject.toml tests/unit/conftest.py tests/unit/plugins/ tox.ini ADDITIONAL INFORMATION Reviewed-by: Bikouo Aubin --- pyproject.toml | 30 ++ tests/unit/conftest.py | 9 + tests/unit/plugins/conftest.py | 11 + tests/unit/plugins/modules/conftest.py | 2 + .../dms_endpoint/test_compare_params.py | 4 +- .../plugins/modules/test_data_pipeline.py | 10 - .../modules/test_directconnect_connection.py | 9 - ...st_directconnect_link_aggregation_group.py | 9 - .../test_directconnect_virtual_interface.py | 9 - tox.ini | 259 +++++++++++++++--- 10 files changed, 273 insertions(+), 79 deletions(-) create mode 100644 tests/unit/conftest.py create mode 100644 tests/unit/plugins/conftest.py diff --git a/pyproject.toml b/pyproject.toml index a3810fdc1e3..fda9b612c94 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,3 +41,33 @@ known_ansible_community_aws = ["ansible_collections.community.aws"] [tool.flynt] transform-joins = true + +[tool.flake8] +# E123, E125 skipped as they are invalid PEP-8. +show-source = true +ignore = ["E123", "E125", "E203", "E402", "E501", "E741", "F401", "F811", "F841", "W503"] +max-line-length = 160 +builtins = "_" + +[tool.mypy] +disable_error_code = ["import-untyped"] + +[tool.ruff] +line-length = 120 + +[tool.ruff.lint] +# "F401" - unused-imports - We use these imports to maintain historic Interfaces +# "E402" - import not at top of file - General Ansible style puts the documentation at the top. +unfixable = ["F401"] +ignore = ["F401", "E402"] + +[tool.pytest] +xfail_strict = true + +[tool.coverage.report] +exclude_lines = [ + # Have to re-enable the standard pragma + "pragma: no cover", + # Don't complain if tests don't hit defensive assertion code: + "raise NotImplementedError", +] diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py new file mode 100644 index 00000000000..7eb3f21a285 --- /dev/null +++ b/tests/unit/conftest.py @@ -0,0 +1,9 @@ +# -*- coding: utf-8 -*- + +# This file is part of Ansible +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +# While it may seem appropriate to import our custom fixtures here, the pytest_ansible pytest plugin +# isn't as agressive as the ansible_test._util.target.pytest.plugins.ansible_pytest_collections plugin +# when it comes to rewriting the import paths and as such we can't import fixtures via their +# absolute import path or across collections. diff --git a/tests/unit/plugins/conftest.py b/tests/unit/plugins/conftest.py new file mode 100644 index 00000000000..e9e86fc03f4 --- /dev/null +++ b/tests/unit/plugins/conftest.py @@ -0,0 +1,11 @@ +# -*- coding: utf-8 -*- + +# This file is part of Ansible +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +# pylint: disable=unused-import + +import pytest + +from ansible_collections.amazon.aws.tests.unit.utils.amazon_placebo_fixtures import fixture_maybe_sleep +from ansible_collections.amazon.aws.tests.unit.utils.amazon_placebo_fixtures import fixture_placeboify diff --git a/tests/unit/plugins/modules/conftest.py b/tests/unit/plugins/modules/conftest.py index ba4a1adc3ec..b13e748939c 100644 --- a/tests/unit/plugins/modules/conftest.py +++ b/tests/unit/plugins/modules/conftest.py @@ -1,3 +1,5 @@ +# -*- coding: utf-8 -*- + # Copyright (c) 2017 Ansible Project # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) diff --git a/tests/unit/plugins/modules/dms_endpoint/test_compare_params.py b/tests/unit/plugins/modules/dms_endpoint/test_compare_params.py index 06a2e1cf509..de090f8ee50 100644 --- a/tests/unit/plugins/modules/dms_endpoint/test_compare_params.py +++ b/tests/unit/plugins/modules/dms_endpoint/test_compare_params.py @@ -3,11 +3,13 @@ # Copyright: (c) 2023, Ansible Project # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) -import pytest from unittest.mock import patch +import pytest + from ansible_collections.community.aws.plugins.modules.dms_endpoint import compare_params + @pytest.mark.parametrize( "described_params,created_params,expected_result", [ diff --git a/tests/unit/plugins/modules/test_data_pipeline.py b/tests/unit/plugins/modules/test_data_pipeline.py index a79f1c0012e..27355069438 100644 --- a/tests/unit/plugins/modules/test_data_pipeline.py +++ b/tests/unit/plugins/modules/test_data_pipeline.py @@ -25,16 +25,6 @@ from ansible_collections.amazon.aws.plugins.module_utils.botocore import HAS_BOTO3 -# Magic... Incorrectly identified by pylint as unused -# isort: off -# pylint: disable=unused-import - -from ansible_collections.amazon.aws.tests.unit.utils.amazon_placebo_fixtures import fixture_maybe_sleep -from ansible_collections.amazon.aws.tests.unit.utils.amazon_placebo_fixtures import fixture_placeboify - -# pylint: enable=unused-import -# isort: on - from ansible_collections.community.aws.plugins.modules import data_pipeline if not HAS_BOTO3: diff --git a/tests/unit/plugins/modules/test_directconnect_connection.py b/tests/unit/plugins/modules/test_directconnect_connection.py index 55e0e5470f4..c3abb2a21b8 100644 --- a/tests/unit/plugins/modules/test_directconnect_connection.py +++ b/tests/unit/plugins/modules/test_directconnect_connection.py @@ -14,15 +14,6 @@ from ansible_collections.amazon.aws.plugins.module_utils.botocore import HAS_BOTO3 -# Magic... Incorrectly identified by pylint as unused -# isort: off -# pylint: disable=unused-import -from ansible_collections.amazon.aws.tests.unit.utils.amazon_placebo_fixtures import fixture_maybe_sleep -from ansible_collections.amazon.aws.tests.unit.utils.amazon_placebo_fixtures import fixture_placeboify - -# pylint: enable=unused-import -# isort: on - from ansible_collections.community.aws.plugins.modules import directconnect_connection if not HAS_BOTO3: diff --git a/tests/unit/plugins/modules/test_directconnect_link_aggregation_group.py b/tests/unit/plugins/modules/test_directconnect_link_aggregation_group.py index f774f6d23fb..ebc509ffba6 100644 --- a/tests/unit/plugins/modules/test_directconnect_link_aggregation_group.py +++ b/tests/unit/plugins/modules/test_directconnect_link_aggregation_group.py @@ -19,15 +19,6 @@ from ansible_collections.amazon.aws.plugins.module_utils.ec2 import boto3_conn from ansible_collections.amazon.aws.plugins.module_utils.ec2 import get_aws_connection_info -# Magic... Incorrectly identified by pylint as unused -# isort: off -# pylint: disable=unused-import -from ansible_collections.amazon.aws.tests.unit.utils.amazon_placebo_fixtures import fixture_maybe_sleep -from ansible_collections.amazon.aws.tests.unit.utils.amazon_placebo_fixtures import fixture_placeboify - -# pylint: enable=unused-import -# isort: on - from ansible_collections.community.aws.plugins.modules import directconnect_link_aggregation_group as lag_module if not HAS_BOTO3: diff --git a/tests/unit/plugins/modules/test_directconnect_virtual_interface.py b/tests/unit/plugins/modules/test_directconnect_virtual_interface.py index b1a3e0bbdd7..c885d503cf0 100644 --- a/tests/unit/plugins/modules/test_directconnect_virtual_interface.py +++ b/tests/unit/plugins/modules/test_directconnect_virtual_interface.py @@ -14,15 +14,6 @@ from ansible_collections.amazon.aws.plugins.module_utils.botocore import HAS_BOTO3 -# Magic... Incorrectly identified by pylint as unused -# isort: off -# pylint: disable=unused-import -from ansible_collections.amazon.aws.tests.unit.utils.amazon_placebo_fixtures import fixture_maybe_sleep -from ansible_collections.amazon.aws.tests.unit.utils.amazon_placebo_fixtures import fixture_placeboify - -# pylint: enable=unused-import -# isort: on - from ansible_collections.community.aws.plugins.modules import directconnect_virtual_interface if not HAS_BOTO3: diff --git a/tox.ini b/tox.ini index 179ed761c7c..42020519a77 100644 --- a/tox.ini +++ b/tox.ini @@ -1,104 +1,281 @@ +# It would be nice to merge this into pyproject.toml, unfortunately as of 4.23.2 they don't support generative environments when using TOML + [tox] skipsdist = True -envlist = clean,ansible{2.12,2.13}-py{38,39,310}-{with_constraints,without_constraints},linters -# Tox4 supports labels which allow us to group the environments rather than dumping all commands into a single environment -labels = - format = flynt, black, isort - lint = complexity-report, ansible-lint, black-lint, isort-lint, flake8-lint, flynt-lint - units = ansible{2.12,2.13}-py{38,39,310}-{with_constraints,without_constraints} +skip_missing_interpreters = True +envlist = + ansible{2.15}-py{39,310,311}-{with_constraints,without_constraints} + ansible{2.16,2.17}-py{310,311,312}-{with_constraints,without_constraints} +# ansible{2.18}-py{311,312,313}-{with_constraints,without_constraints} [common] +collection_name = community.aws +collection_path = community/aws + format_dirs = {toxinidir}/plugins {toxinidir}/tests +lint_dirs = {toxinidir}/plugins {toxinidir}/tests + +ansible_desc = + ansible2.15: Ansible-core 2.15 + ansible2.16: Ansible-core 2.16 + ansible2.17: Ansible-core 2.17 + ansible2.18: Ansible-core 2.18 +const_desc = + with_constraints: (With boto3/botocore constraints) + +ansible_home = {envtmpdir}/ansible_home +ansible_collections_path = {[common]ansible_home}/collections +full_collection_path = {[common]ansible_home}/collections/ansible_collections/{[common]collection_path} [testenv] -description = Run the test-suite and generate a HTML coverage report +description = Run the unit tests {[common]ansible_desc}/{base_python} {[common]const_desc} +set_env = + ANSIBLE_HOME={[common]ansible_home} + ANSIBLE_COLLECTIONS_PATH={[common]ansible_collections_path} +# ansible_pytest_collections is more aggressive than pytest_ansible when injecting collections into the import path +# not needed if unit tests are under tests/unit/plugins rather than directly under tests/unit +# ANSIBLE_CONTROLLER_MIN_PYTHON_VERSION=3.11 +# PYTEST_PLUGINS=ansible_test._util.target.pytest.plugins.ansible_pytest_collections +labels = unit deps = pytest + mock + pytest-mock pytest-cov - ansible2.12: ansible-core>2.12,<2.13 - ansible2.13: ansible-core>2.13,<2.14 - !ansible2.12-!ansible2.13: ansible-core pytest-ansible + pytest-xdist -rtest-requirements.txt + -rtests/unit/requirements.txt + ansible2.15: ansible-core>2.15,<2.16 + ansible2.16: ansible-core>2.16,<2.17 + ansible2.17: ansible-core>2.17,<2.18 + ansible2.18: ansible-core>2.18,<2.19 with_constraints: -rtests/unit/constraints.txt -commands = pytest --cov-report html --cov plugins/callback --cov plugins/inventory --cov plugins/lookup --cov plugins/module_utils --cov plugins/modules --cov plugins/plugin_utils plugins {posargs:tests/} +allowlist_externals = rsync +change_dir = {[common]full_collection_path} +commands_pre = + rsync --delete --exclude=.tox -qraugpo {toxinidir}/ {[common]full_collection_path}/ + ansible-galaxy collection install git+https://github.com/ansible-collections/amazon.aws.git,stable-9 +commands = + pytest \ + --cov-report html \ + --cov plugins/inventory \ + --cov plugins/module_utils \ + --cov plugins/modules \ + --cov plugins \ + --ansible-host-pattern localhost \ + {posargs:tests/unit/} +# We don't have these in community +# --cov plugins/plugin_utils \ +# --cov plugins/callback \ +# --cov plugins/lookup \ [testenv:clean] +description = Remove test results and caches +allowlist_externals = rm deps = coverage skip_install = true -commands = coverage erase +change_dir = {toxinidir} +commands_pre = +commands = + coverage erase + rm -rf tests/output/ htmlcov/ .mypy_cache/ complexity/ .ruff_cache/ [testenv:complexity-report] +labels = future-lint description = Generate a HTML complexity report in the complexity directory deps = - # See: https://github.com/lordmauve/flake8-html/issues/30 - flake8>=3.3.0,<5.0.0 + flake8-pyproject flake8-html -commands = -flake8 --select C90 --max-complexity 10 --format=html --htmldir={posargs:complexity} plugins +change_dir = {toxinidir} +commands_pre = +commands = + -flake8 \ + --select C90 \ + --max-complexity 10 \ + --format=html \ + --htmldir={posargs:complexity} \ + {posargs:{[common]lint_dirs}} [testenv:ansible-lint] +labels = lint +description = Run ansible-lint deps = - ansible-lint + ansible-lint >= 24.7.0 + jmespath +change_dir = {toxinidir} +commands_pre = commands = - ansible-lint {toxinidir}/plugins + ansible-lint \ + --skip-list=name[missing],yaml[line-length],args[module],run-once[task],ignore-errors,sanity[cannot-ignore],run-once[play] \ + {posargs:plugins} +# {posargs:{[common]lint_dirs}} [testenv:black] +labels = format +description = Apply "black" formatting depends = flynt, isort deps = black >=23.0, <24.0 +change_dir = {toxinidir} +commands_pre = commands = - black {[common]format_dirs} + black {posargs:{[common]format_dirs}} [testenv:black-lint] +labels = lint +description = Lint against "black" formatting standards deps = {[testenv:black]deps} +change_dir = {toxinidir} +commands_pre = commands = - black -v --check --diff {[common]format_dirs} + black --check --diff {posargs:{[common]format_dirs}} [testenv:isort] +labels = format +description = Sort imports deps = isort +change_dir = {toxinidir} +commands_pre = commands = - isort {[common]format_dirs} + isort {posargs:{[common]format_dirs}} [testenv:isort-lint] +labels = lint +description = Lint for import sorting deps = {[testenv:isort]deps} +change_dir = {toxinidir} +commands_pre = commands = - isort --check-only --diff {[common]format_dirs} - -[testenv:flake8-lint] -deps = - flake8 -commands = - flake8 {posargs} {[common]format_dirs} + isort --check-only --diff {posargs:{[common]format_dirs}} [testenv:flynt] +labels = format +description = Apply flint (f-string) formatting deps = flynt +change_dir = {toxinidir} +commands_pre = commands = - flynt {[common]format_dirs} + flynt {posargs:{[common]format_dirs}} [testenv:flynt-lint] +labels = lint +description = Run flint (f-string) linting deps = flynt +change_dir = {toxinidir} +commands_pre = commands = - flynt --dry-run {[common]format_dirs} + flynt --dry-run --fail-on-change {posargs:{[common]format_dirs}} -[testenv:linters] +[testenv:flake8-lint] +labels = lint +description = Run FLAKE8 linting deps = - {[testenv:black]deps} - {[testenv:isort]deps} flake8 + flake8-pyproject +change_dir = {toxinidir} +commands_pre = +commands = + flake8 {posargs:{[common]format_dirs}} + +[testenv:pylint-lint] +labels = future-lint +description = Run pylint tests that are disabled by the default Ansible sanity tests +deps = + pylint +change_dir = {toxinidir} +commands_pre = +commands = + pylint \ + --disable R,C,W,E \ + --enable pointless-statement \ + --enable consider-using-dict-items \ + --enable assignment-from-no-return \ + --enable no-else-continue \ + --enable no-else-break \ + --enable simplifiable-if-statement \ + --enable pointless-string-statement \ + --enable redefined-outer-name \ + --enable redefined-builtin \ + --enable unused-import \ + {posargs:{[common]lint_dirs}} + +[testenv:ruff] +description = lint source code +labels = format-future +deps = + ruff +change_dir = {toxinidir} +commands_pre = commands = - black -v --check {toxinidir}/plugins {toxinidir}/tests - isort --check-only --diff {toxinidir}/plugins {toxinidir}/tests - flake8 {posargs} {toxinidir}/plugins {toxinidir}/tests + ruff check --fix {posargs:{[common]lint_dirs}} + ruff format {posargs:{[common]lint_dirs}} -[flake8] -# E123, E125 skipped as they are invalid PEP-8. -show-source = True -ignore = E123,E125,E203,E402,E501,E741,F401,F811,F841,W503 -max-line-length = 160 -builtins = _ +[testenv:ruff-lint] +description = lint source code +labels = lint-future +deps = + ruff +change_dir = {toxinidir} +commands_pre = +commands = + ruff check --diff --unsafe-fixes {posargs:{[common]lint_dirs}} + ruff check {posargs:{[common]lint_dirs}} + +[testenv:ansible-lint-future] +labels = future-lint +description = Run ansible-lint +deps = + ansible-lint + jmespath + git+https://github.com/ansible/ansible.git@devel + shellcheck-py +commands = + ansible-lint \ + {posargs:{[common]lint_dirs}} + +[testenv:ansible-sanity] +labels = future-lint +description = Run latest (devel) Ansible sanity tests +deps = + git+https://github.com/ansible/ansible.git@devel + shellcheck-py +commands = + ansible-test sanity + +[testenv:mypy-lint] +allowlist_externals = rsync,ln +labels = future-lint +description = Run mypi type tests +set_env = + ANSIBLE_HOME={[common]ansible_home} + ANSIBLE_COLLECTIONS_PATH={[common]ansible_collections_path} + MYPYPATH={[common]ansible_home} +deps = + mypy + # ansible-core + git+https://github.com/ansible/ansible.git@devel + botocore + boto3 + placebo + typing_extensions +commands_pre = + rsync --delete --exclude=.tox -qraugpo {toxinidir}/ {[common]full_collection_path}/ + ansible-galaxy collection install git+https://github.com/ansible-collections/amazon.aws.git,stable-9 + ln -s {env_site_packages_dir}/ansible {[common]ansible_collections_path}/ansible + ln -s {[common]ansible_home}/collections/ansible_collections {[common]ansible_home}/ansible_collections +commands = + # TODO: passing directories doesn't work well, it's better to pass the package + # we might want to consider manipulating posargs/directories into the package names + mypy \ + --check-untyped-defs \ + --namespace-packages --explicit-package-bases \ + --follow-imports silent \ + -p ansible_collections.community.aws + # {posargs:plugins/module_utils plugins/plugin_utils}