From 3197f4b71b25411a665c60f660987a39d328ffeb Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Fri, 15 Mar 2024 10:26:14 +0000 Subject: [PATCH 1/4] Test installed scripts instead of internal modules This replaces the `test_binary_runs` test with `test_script_runs` test. The former would just enumerate all the modules present in the `nav.bin` packages, extract their test args and run them directly from that location. However, not all the scripts here are compatible with being invoked from a pip-installed bin-directory shim that calls their main method directly. I.e. this updates the test suite to attempt to call all the defined NAV scripts in the manner they would be run in an installed system, rather than in an artificial manner. This should detect additional problems with the scripts. --- tests/integration/bin_test.py | 6 ++-- tests/integration/conftest.py | 52 ++++++++++++++++++++--------------- tests/requirements.txt | 1 + 3 files changed, 34 insertions(+), 25 deletions(-) diff --git a/tests/integration/bin_test.py b/tests/integration/bin_test.py index 5e8947fb2f..126dff7c81 100644 --- a/tests/integration/bin_test.py +++ b/tests/integration/bin_test.py @@ -6,9 +6,9 @@ BINDIR = './python/nav/bin' -def test_binary_runs(binary): - """Verifies that a command runs with a zero exit code""" - proc = subprocess.Popen(binary, stderr=subprocess.STDOUT, stdout=subprocess.PIPE) +def test_script_runs(script): + """Verifies that a script defined in pyproject.toml runs with a zero exit code""" + proc = subprocess.Popen(script, stderr=subprocess.STDOUT, stdout=subprocess.PIPE) (done, fail) = proc.communicate() retcode = proc.wait() diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 65c7a13c62..8f29749f02 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -1,5 +1,6 @@ from __future__ import print_function import os +import importlib.util import io import re import shlex @@ -8,6 +9,7 @@ import subprocess import time +import toml import pytest from django.test import Client @@ -90,10 +92,10 @@ def stop_gunicorn(): def pytest_generate_tests(metafunc): - if 'binary' in metafunc.fixturenames: - binaries = _nav_binary_tests() - ids = [b[0] for b in binaries] - metafunc.parametrize("binary", _nav_binary_tests(), ids=ids) + if 'script' in metafunc.fixturenames: + scripts = _nav_script_tests() + ids = [s[0] for s in scripts] + metafunc.parametrize("script", _nav_script_tests(), ids=ids) elif 'admin_navlet' in metafunc.fixturenames: from nav.models.profiles import AccountNavlet @@ -101,27 +103,33 @@ def pytest_generate_tests(metafunc): metafunc.parametrize("admin_navlet", navlets) -def _nav_binary_tests(): - for binary in _nav_binary_list(): - for args in _scan_testargs(binary): - if args: - yield args - +def _nav_script_tests(): + """Generates a list of command lines to run to test all the NAV scripts defined + in pyproject.toml. -def _nav_binary_list(): - files = sorted( - os.path.join(BINDIR, f) for f in os.listdir(BINDIR) if not _is_excluded(f) - ) - return (f for f in files if os.path.isfile(f)) + Each NAV script can define 0 to many sets of test arguments that should be used + when executing the script, using comments at the top of the file. This is because + some of the scripts are designed to require some arguments to be present in order + to run without exiting with an error. + """ + for script, module_name in _nav_scripts_map().items(): + spec = importlib.util.find_spec(module_name) + for args in _scan_testargs(spec.origin): + if args: + yield [script] + args[1:] -def _is_excluded(filename): - return ( - filename.endswith('~') - or filename.startswith('.') - or filename.startswith('__') - or filename.startswith('Makefile') - ) +def _nav_scripts_map() -> dict[str, str]: + """Returns a map of installable script names to NAV module names from + pyproject.toml. + """ + data = toml.load('pyproject.toml') + scripts: dict[str, str] = data.get('project', {}).get('scripts', {}) + return { + script: module.split(':', maxsplit=1)[0] + for script, module in scripts.items() + if module.startswith('nav.') + } def _scan_testargs(filename): diff --git a/tests/requirements.txt b/tests/requirements.txt index cbbb04d3ee..10fadce19c 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -12,6 +12,7 @@ pytest-timeout pytest-twisted~=1.13.0 pytidylib==0.3.2 selenium==3.141.0 +toml whisper>=0.9.9 whitenoise==4.1.4 # Our version of selenium breaks down if it is allowed to pull in the newest version of urllib3 From db9d2c5171f20c567bf021fab464b6c0e31013f0 Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Fri, 15 Mar 2024 10:30:22 +0000 Subject: [PATCH 2/4] Remove unused args from navclean.main() This required-but-unused argument to main prevented `navclean` from running properly when invoked from the binary shim installed by pip/setuptools. Fixes #2874 --- python/nav/bin/navclean.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/python/nav/bin/navclean.py b/python/nav/bin/navclean.py index 80b3b35235..f32f980dcc 100755 --- a/python/nav/bin/navclean.py +++ b/python/nav/bin/navclean.py @@ -34,7 +34,7 @@ import nav.db -def main(args): +def main(): """Main execution function.""" parser = make_argparser() args = parser.parse_args() @@ -210,7 +210,7 @@ class RadiusAcctDeleter(RecordDeleter): selector = """ WHERE (acctstoptime < {expiry}) OR ((acctstarttime + (acctsessiontime * interval '1 sec')) < {expiry}) - OR (acctstarttime < {expiry} + OR (acctstarttime < {expiry} AND (acctstarttime + (acctsessiontime * interval '1 sec')) IS NULL) """ @@ -245,4 +245,4 @@ def delete(self, expiry, dry_run=False): if __name__ == '__main__': - main(sys.argv[1:]) + main() From 62b3ff93c48a3a8055419e6d3f05e18cb5af4056 Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Fri, 15 Mar 2024 10:31:31 +0000 Subject: [PATCH 3/4] Remove unused args from navsynctypes.main() This required-but-unused argument to main prevented `navsynctypes` from running properly when invoked from the binary shim installed by pip/setuptools. The argparser is only there to provide a nice help screen if the `--help` argument is provided. --- python/nav/bin/navsynctypes.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/python/nav/bin/navsynctypes.py b/python/nav/bin/navsynctypes.py index dca743d744..aceaae2e2a 100755 --- a/python/nav/bin/navsynctypes.py +++ b/python/nav/bin/navsynctypes.py @@ -28,12 +28,12 @@ VENDOR_INSERT_SQL_TEMPLATE = """ -INSERT INTO vendor +INSERT INTO vendor (SELECT {vendorid} AS vendorid WHERE NOT EXISTS ( SELECT vendorid FROM vendor WHERE vendorid ILIKE {vendorid} ) -); +); """ TYPE_UPSERT_SQL_TEMPLATE = """ @@ -43,7 +43,7 @@ {typename}, {sysobjectid}, {descr} -) ON CONFLICT (sysobjectid) DO UPDATE +) ON CONFLICT (sysobjectid) DO UPDATE SET vendorid=(SELECT vendorid FROM vendor WHERE vendorid ILIKE {vendorid}), typename={typename}, @@ -52,8 +52,9 @@ """ -def main(_args): +def main(): """Main program""" + parse_args() types = NetboxType.objects.all().select_related("vendor") used_vendors = {t.vendor.id for t in types} if types: @@ -109,4 +110,4 @@ def parse_args(): if __name__ == "__main__": - main(parse_args()) + main() From 005d1aa760191c366501450412423422535e525f Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Fri, 15 Mar 2024 10:40:50 +0000 Subject: [PATCH 4/4] fixup! Test installed scripts instead of internal modules Python 3.7, damn you! --- tests/integration/conftest.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 8f29749f02..4d479fb0a5 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -8,6 +8,7 @@ from shutil import which import subprocess import time +from typing import Dict import toml import pytest @@ -119,7 +120,7 @@ def _nav_script_tests(): yield [script] + args[1:] -def _nav_scripts_map() -> dict[str, str]: +def _nav_scripts_map() -> Dict[str, str]: """Returns a map of installable script names to NAV module names from pyproject.toml. """