From 2bf7bed9e634efc50d87e6a76c8cf5b8249de70e Mon Sep 17 00:00:00 2001 From: Leonardo Fagundes Luz Serrano Date: Wed, 25 Sep 2024 18:59:29 -0300 Subject: [PATCH] Zuul/Tox: Update software tests Updated tox configs on software and software-client and update zuul jobs to call python tests on them using their individual configs. Fixed a few flake8 issues on both directories. software-client pylint test set to non-voting for now. Test Plan: In a venv: pass: tox -c software/tox.ini -eflake8 pass: tox -c software/tox.ini -epylint pass: tox -c software-client/tox.ini -eflake8 Story: 2010676 Task: 51082 Depends-On: https://review.opendev.org/c/starlingx/update/+/930489 Change-Id: I3b8e2612c4bc579e2284383f14b2879a07d0d7bd Signed-off-by: Leonardo Fagundes Luz Serrano --- .gitignore | 11 +- .zuul.yaml | 51 ++++-- .../software_client/common/http.py | 1 - software-client/test-requirements.txt | 5 +- software-client/tox.ini | 144 +++++++++++++---- software/software/software_functions.py | 3 +- software/software/utilities/migrate.py | 5 +- software/test-requirements.txt | 5 +- software/tox.ini | 149 ++++++++++++++---- 9 files changed, 291 insertions(+), 83 deletions(-) diff --git a/.gitignore b/.gitignore index 68155890..ebcf6b8c 100644 --- a/.gitignore +++ b/.gitignore @@ -1,9 +1,11 @@ *.a +*.coverage* *.egg *.egg-info *.o -*.py[co] +*.pyo *.pyc +*__pycache__* *.so *.sqlite .*.swp @@ -12,7 +14,7 @@ .stestr .testrepository .tox -.venv +.vscode AUTHORS ChangeLog _build @@ -23,6 +25,11 @@ dist eggs sdist +.env +.venv +env +venv + # Sphinx documentation doc/build diff --git a/.zuul.yaml b/.zuul.yaml index ee0e5a47..6b141fac 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -11,8 +11,10 @@ - py3-flake8 - patch-alarm-tox-pylint - patch-alarm-tox-py39 - - stx-software-tox-pylint - - stx-software-tox-py39 + - software-tox-flake8 + - software-tox-pylint + - software-client-tox-flake8 + - software-client-tox-pylint - sw-patch-tox-pylint - sw-patch-tox-py39 gate: @@ -22,8 +24,10 @@ - py3-flake8 - patch-alarm-tox-pylint - patch-alarm-tox-py39 - - stx-software-tox-pylint - - stx-software-tox-py39 + - software-tox-flake8 + - software-tox-pylint + - software-client-tox-flake8 + - software-client-tox-pylint - sw-patch-tox-pylint - sw-patch-tox-py39 post: @@ -47,9 +51,10 @@ vars: tox_envlist: flake8 + - job: - name: stx-software-tox-py39 - parent: tox-py39 + name: software-tox-flake8 + parent: tox nodeset: debian-bullseye required-projects: - starlingx/config @@ -57,12 +62,12 @@ files: - software/* vars: - tox_envlist: py39 - python_version: 3.9 + tox_envlist: flake8 tox_extra_args: -c software/tox.ini + - job: - name: stx-software-tox-pylint + name: software-tox-pylint parent: tox nodeset: debian-bullseye required-projects: @@ -72,10 +77,36 @@ - software/* vars: tox_envlist: pylint - python_version: 3.9 tox_extra_args: -c software/tox.ini +- job: + name: software-client-tox-flake8 + parent: tox + nodeset: debian-bullseye + required-projects: + - starlingx/config + files: + - software-client/* + vars: + tox_envlist: flake8 + tox_extra_args: -c software-client/tox.ini + + +- job: + name: software-client-tox-pylint + parent: tox + nodeset: debian-bullseye + required-projects: + - starlingx/config + files: + - software-client/* + vars: + tox_envlist: pylint + tox_extra_args: -c software-client/tox.ini + voting: false + + - job: name: sw-patch-tox-py39 parent: tox-py39 diff --git a/software-client/software_client/common/http.py b/software-client/software_client/common/http.py index 563c6300..67f5d5cf 100644 --- a/software-client/software_client/common/http.py +++ b/software-client/software_client/common/http.py @@ -24,7 +24,6 @@ import os from oslo_serialization import jsonutils from oslo_utils import encodeutils import requests -from requests_toolbelt import MultipartEncoder import socket from pecan.core import Response as PCResponse diff --git a/software-client/test-requirements.txt b/software-client/test-requirements.txt index 578639e2..d15228e3 100644 --- a/software-client/test-requirements.txt +++ b/software-client/test-requirements.txt @@ -1,9 +1,12 @@ # The order of packages is significant, because pip processes them in this order -hacking + +hacking # This installs a modified version of flake8 and some plugins bandit +bashate coverage httplib2 pylint +shellcheck-py stestr tabulate diff --git a/software-client/tox.ini b/software-client/tox.ini index 7dd12acb..6bff7b22 100644 --- a/software-client/tox.ini +++ b/software-client/tox.ini @@ -1,26 +1,43 @@ # -# Copyright (c) 2023 Wind River Systems, Inc. +# Copyright (c) 2024 Wind River Systems, Inc. # # SPDX-License-Identifier: Apache-2.0 # +# Tox is a tool for running tests in multiple virtualenvs. +# +# To run this, from the update repo root directory, execute the following: +# - Install python3.9: apt install python3.9 python3.9-pip python3.9-venv +# - Create a py39 venv: python3.9 -m venv .venv +# - Source the venv: source .venv/bin/activate +# - Install tox in the venv: pip install tox +# - Run tox: tox -c software-client/tox.ini [tox] -envlist = pep8,py39,pylint -minversion = 2.3.2 +envlist = bandit, cover, flake8, py39, pylint, shellcheck, bashate +minversion = 4 skipsdist = True + +# Default value would be {work_dir}/.tmp +# Setting to /tmp makes paths shorter, preventing issues with long paths +temp_dir = /tmp/update_repo_tox + +# Custom variables stxdir = {toxinidir}/../.. +exclude_dirs = .env,.venv,.git,.tox +exclude_dirs_glob = *.env*,*.venv*,*.git*,*.tox* + [testenv] -allowlist_externals = find - sh -basepython = python3 -deps = -r{toxinidir}/requirements.txt - -r{toxinidir}/test-requirements.txt - -e{[tox]stxdir}/config/tsconfig/tsconfig +allowlist_externals = + bash +basepython = python3.9 # Matching debian bullseye, base OS used on STX + +deps = + -r{toxinidir}/requirements.txt + -r{toxinidir}/test-requirements.txt + -e{[tox]stxdir}/config/tsconfig/tsconfig + -c{env:UPPER_CONSTRAINTS_FILE:https://opendev.org/starlingx/root/raw/branch/master/build-tools/requirements/debian/upper-constraints.txt} -install_command = pip install \ - -c{env:UPPER_CONSTRAINTS_FILE:https://opendev.org/starlingx/root/raw/branch/master/build-tools/requirements/debian/upper-constraints.txt} \ - {opts} {packages} passenv = XDG_CACHE_HOME setenv = VIRTUAL_ENV={envdir} @@ -35,18 +52,22 @@ setenv = VIRTUAL_ENV={envdir} sitepackages = False usedevelop = true -[bandit] -exclude = tests -skips = [testenv:bandit] -commands = bandit --ini tox.ini -n 5 -r software_client +description = + Find common security issues in Python code +commands = + # Using --silent flag so bandit does not report on files with no issues + bandit --recursive --silent --exclude {[tox]exclude_dirs_glob},*tests* . +commands_post = + bandit --version + [testenv:cover] +description = Measures effectiveness of python tests setenv = {[testenv]setenv} PYTHON=coverage run --parallel-mode - commands = coverage erase stestr run {posargs} @@ -55,32 +76,91 @@ commands = coverage xml -o cover/coverage.xml coverage report + [flake8] +description = + Flake8 settings. + While some linters have separate configuration files, + flake8 configuration is integrated into Tox. + +# Set max line length allowed in python scripts +max-line-length = 120 + +# List number of errors of each type +statistics = True + +# Show code line that triggered the error +show-source = True + # H106: Don't put vim configuration in source files (off by default). # H203: Use assertIs(Not)None to check for None (off by default). -# H904 Delay string interpolations at logging calls (off by default) -enable-extensions = H106 H203,H904 -exclude = .venv,.git,.tox,dist,doc,*lib/python*,*egg,build,release-tag-* -max-line-length = 80 -show-source = True -ignore = E402,H306,H404,H405,W504,E501 +# H904: Delay string interpolations at logging calls (off by default) +enable-extensions = H106, H203, H904 + +# TODO: Fix these errors and remove these error suppressions +# H306 imports not in alphabetical order ==> ~10 instances +# H404 multi line docstring should start without a leading new line ==> ~30 instances +# H405 multi line docstring summary not separated with an empty line ==> ~100 instances +extend-ignore = H306, H404, H405 + [testenv:flake8] -commands = flake8 {posargs} +description = Checks PEP8 style formatting in python scripts. +commands = + flake8 --extend-exclude {[tox]exclude_dirs} {posargs} +commands_post = + flake8 --version + + +[stestr] +description = Settings for testenv:py39 tox env +test_path=./software_client/tests +top_dir=./ +group_regex=([^\.]*\.)* -[testenv:pep8] -commands = flake8 {posargs} [testenv:py39] +description = Run python unit tests using py39 basepython = python3.9 commands = stestr run {posargs} stestr slowest -[testenv:pylint] -commands = pylint software_client --rcfile=./pylint.rc -[stestr] -test_path=./software_client/tests -top_dir=./ -group_regex=([^\.]*\.)* +# TODO: Review pylintrc configs and error suppressions +[testenv:pylint] +description = + Run pylint on update/software_client/software_client + Configs in update/software_client/pylint.rc +commands = + pylint software_client --rcfile=./pylint.rc + + +[testenv:shellcheck] +description = + Runs a shell/bash linter on scripts with a shebang containing bash or sh +commands = + # Shellcheck does not have a recursive option. Needs to be run on each file individually. + bash -c "grep -Rl . -e '\#\!.*\(sh\|bash\)' --exclude-dir={{[tox]exclude_dirs}} | \ + xargs --verbose --no-run-if-empty -I {} \ + shellcheck {} \ + " +commands_post = + # List files checked + bash -c "grep -Rl . -e '\#\!.*\(sh\|bash\)' --exclude-dir={{[tox]exclude_dirs}}" + shellcheck --version + + +[testenv:bashate] +description = + Runs a shell/bash formatting and style check on scripts with a shebang containing bash or sh +commands = + # Bashate does not have a recursive option. Needs to be run on each file individually. + bash -c "grep -Rl . -e '\#\!.*\(sh\|bash\)' --exclude-dir={{[tox]exclude_dirs}} | \ + xargs --verbose --no-run-if-empty -I {} \ + bashate --verbose --max-line-length 120 {} \ + " +commands_post = + # List files checked + bash -c "grep -Rl . -e '\#\!.*\(sh\|bash\)' --exclude-dir={{[tox]exclude_dirs}}" + bashate --version diff --git a/software/software/software_functions.py b/software/software/software_functions.py index 0938d77f..df614e22 100644 --- a/software/software/software_functions.py +++ b/software/software/software_functions.py @@ -939,7 +939,8 @@ class PatchFile(object): try: patch_sw_version = utils.get_major_release_version( - thispatch.metadata[patch_id]["sw_version"]) + thispatch.metadata[patch_id]["sw_version"] + ) abs_ostree_tar_dir = package_dir[patch_sw_version] os.remove("%s/%s-software.tar" % (abs_ostree_tar_dir, patch_id)) except Exception: diff --git a/software/software/utilities/migrate.py b/software/software/utilities/migrate.py index 21aeada2..7c6d177f 100644 --- a/software/software/utilities/migrate.py +++ b/software/software/utilities/migrate.py @@ -65,15 +65,16 @@ def migrate_keyring_data(from_release, to_release): try: LOG.info("Executing keyring migrate command: %s" % chgrp_cmd) subprocess.check_call([chgrp_cmd], - shell=True, stdout=sout, stderr=sout) + shell=True, stdout=sout, stderr=sout) except subprocess.CalledProcessError as ex: LOG.exception("Failed to execute command: '%s' during upgrade " - "processing, return code: %d" % (chgrp_cmd, ex.returncode)) + "processing, return code: %d" % (chgrp_cmd, ex.returncode)) raise else: LOG.error("Directory %s does not exist" % constants.KEYRING_DIR_PATH) raise Exception("keyring directory cannot be found") + def migrate_pxeboot_config(from_release, to_release): """Migrates pxeboot configuration. """ LOG.info("Migrating pxeboot config") diff --git a/software/test-requirements.txt b/software/test-requirements.txt index 43806539..c7ee56c3 100644 --- a/software/test-requirements.txt +++ b/software/test-requirements.txt @@ -1,7 +1,10 @@ # The order of packages is significant, because pip processes them in this order -hacking + +hacking # This installs a modified version of flake8 and some plugins bandit +bashate coverage pylint +shellcheck-py stestr diff --git a/software/tox.ini b/software/tox.ini index 77283e99..18554742 100644 --- a/software/tox.ini +++ b/software/tox.ini @@ -1,27 +1,44 @@ # -# Copyright (c) 2023 Wind River Systems, Inc. +# Copyright (c) 2024 Wind River Systems, Inc. # # SPDX-License-Identifier: Apache-2.0 # +# Tox is a tool for running tests in multiple virtualenvs. +# +# To run this, from the update repo root directory, execute the following: +# - Install python3.9: apt install python3.9 python3.9-pip python3.9-venv +# - Create a py39 venv: python3.9 -m venv .venv +# - Source the venv: source .venv/bin/activate +# - Install tox in the venv: pip install tox +# - Run tox: tox -c software/tox.ini [tox] -envlist = pep8,py39,pylint -minversion = 2.3.2 +envlist = bandit, cover, flake8, py39, pylint, shellcheck, bashate +minversion = 4 skipsdist = True + +# Default value would be {work_dir}/.tmp +# Setting to /tmp makes paths shorter, preventing issues with long paths +temp_dir = /tmp/update_repo_tox + +# Custom variables stxdir = {toxinidir}/../.. +exclude_dirs = .env,.venv,.git,.tox +exclude_dirs_glob = *.env*,*.venv*,*.git*,*.tox* + [testenv] -allowlist_externals = find - sh -basepython = python3 -deps = -r{toxinidir}/requirements.txt - -r{toxinidir}/test-requirements.txt - -e{[tox]stxdir}/fault/fm-api/source - -e{[tox]stxdir}/config/tsconfig/tsconfig +allowlist_externals = + bash +basepython = python3.9 # Matching debian bullseye, base OS used on STX + +deps = + -r{toxinidir}/requirements.txt + -r{toxinidir}/test-requirements.txt + -e{[tox]stxdir}/fault/fm-api/source + -e{[tox]stxdir}/config/tsconfig/tsconfig + -c{env:UPPER_CONSTRAINTS_FILE:https://opendev.org/starlingx/root/raw/branch/master/build-tools/requirements/debian/upper-constraints.txt} -install_command = pip install -v -v -v \ - -c {env:UPPER_CONSTRAINTS_FILE:https://opendev.org/starlingx/root/raw/branch/master/build-tools/requirements/debian/upper-constraints.txt} \ - {opts} {packages} passenv = XDG_CACHE_HOME setenv = VIRTUAL_ENV={envdir} @@ -36,18 +53,22 @@ setenv = VIRTUAL_ENV={envdir} sitepackages = False usedevelop = true -[bandit] -exclude = tests -skips = [testenv:bandit] -commands = bandit --ini tox.ini -n 5 -r software +description = + Find common security issues in Python code +commands = + # Using --silent flag so bandit does not report on files with no issues + bandit --recursive --silent --exclude {[tox]exclude_dirs_glob},*tests* . +commands_post = + bandit --version + [testenv:cover] -setenv = +description = Measures effectiveness of python tests +setenv = {[testenv]setenv} PYTHON=coverage run --parallel-mode - commands = coverage erase stestr run {posargs} @@ -56,31 +77,93 @@ commands = coverage xml -o cover/coverage.xml coverage report + [flake8] +description = + Flake8 settings. + While some linters have separate configuration files, + flake8 configuration is integrated into Tox. + +# Set max line length allowed in python scripts +max-line-length = 120 + +# List number of errors of each type +statistics = True + +# Show code line that triggered the error +show-source = True + # H106: Don't put vim configuration in source files (off by default). # H203: Use assertIs(Not)None to check for None (off by default). -enable-extensions = H106,H203 -exclude = .venv,.git,.tox,dist,doc,*lib/python*,*egg,build,release-tag-* -max-line-length = 120 -show-source = True -ignore = E402,H306,H404,H405,W504,E501,H105 +enable-extensions = H106, H203 + +# TODO: Fix these errors and remove these error suppressions +# E402 module level import not at top of file => ~90 instances +# E501 line too long => ~15 instances +# H105 Don't use author tags => 1 instance +# H306 imports not in alphabetical order => ~80 instances +# H404 multi line docstring should start without a leading new line => ~80 instances +# H405 multi line docstring summary not separated with an empty line => ~930 instances +extend-ignore = E402, E501, H105, H306, H404, H405 + [testenv:flake8] -commands = flake8 {posargs} +description = Checks PEP8 style formatting in python scripts. +commands = + flake8 --extend-exclude {[tox]exclude_dirs} {posargs} +commands_post = + flake8 --version + + +[stestr] +description = Settings for testenv:py39 tox env +test_path=./software/tests +top_dir=./ +group_regex=([^\.]*\.)* -[testenv:pep8] -commands = flake8 {posargs} [testenv:py39] +description = Run python unit tests using py39 basepython = python3.9 commands = stestr run {posargs} stestr slowest -[testenv:pylint] -commands = pylint software --rcfile=./pylint.rc -[stestr] -test_path=./software/tests -top_dir=./ -group_regex=([^\.]*\.)* +# TODO: Review pylintrc configs and error suppressions +[testenv:pylint] +description = + Run pylint on update/software/software. + Configs in update/software/pylint.rc +commands = + pylint software --rcfile=./pylint.rc + + +[testenv:shellcheck] +description = + Runs a shell/bash linter on scripts with a shebang containing bash or sh +commands = + # Shellcheck does not have a recursive option. Needs to be run on each file individually. + bash -c "grep -Rl . -e '\#\!.*\(sh\|bash\)' --exclude-dir={{[tox]exclude_dirs}} | \ + xargs --verbose --no-run-if-empty -I {} \ + shellcheck {} \ + " +commands_post = + # List files checked + bash -c "grep -Rl . -e '\#\!.*\(sh\|bash\)' --exclude-dir={{[tox]exclude_dirs}}" + shellcheck --version + + +[testenv:bashate] +description = + Runs a shell/bash formatting and style check on scripts with a shebang containing bash or sh +commands = + # Bashate does not have a recursive option. Needs to be run on each file individually. + bash -c "grep -Rl . -e '\#\!.*\(sh\|bash\)' --exclude-dir={{[tox]exclude_dirs}} | \ + xargs --verbose --no-run-if-empty -I {} \ + bashate --verbose --max-line-length 120 {} \ + " +commands_post = + # List files checked + bash -c "grep -Rl . -e '\#\!.*\(sh\|bash\)' --exclude-dir={{[tox]exclude_dirs}}" + bashate --version