From f68932803f47537d484199d1d88fd52a83921f30 Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Fri, 10 Feb 2023 11:10:01 +0100 Subject: [PATCH 01/20] Replace create-db.sh with PostgreSQL fixtures This creates a PostgreSQL fixture that can be depended on by tests that require a running PostgreSQL server with the NAV database to be present. The fixture will work with externally set up PostgreSQL servers, like in the case of GitHub Actions, while it can also provision a PostgreSQL server with the NAV schema using Docker if run locally. --- tests/conftest.py | 83 +++++++++++++++++++++++++++++-- tests/docker-compose.yml | 18 +++++++ tests/docker/scripts/create-db.sh | 60 ---------------------- tests/functional/conftest.py | 20 -------- tests/integration/conftest.py | 8 --- tests/requirements.txt | 2 + 6 files changed, 100 insertions(+), 91 deletions(-) create mode 100644 tests/docker-compose.yml delete mode 100755 tests/docker/scripts/create-db.sh diff --git a/tests/conftest.py b/tests/conftest.py index e5e485eb64..d35072bcdb 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,5 +1,4 @@ -"""pytest setup and fixtures common for all tests, regardless of suite""" - +"""Pytest config and fixtures for all test suites""" import os import platform import subprocess @@ -7,6 +6,7 @@ import pytest import requests from requests.adapters import HTTPAdapter, Retry +from retry import retry def pytest_configure(config): @@ -27,6 +27,83 @@ def pytest_configure(config): install() +def is_running_in_github_actions(): + """Returns True if running under GitHub Actions""" + return os.getenv("GITHUB_ACTIONS") + + +def _get_preferred_database_name(): + if is_running_in_github_actions(): + if not os.getenv("PGDATABASE") and os.getenv("TOX_ENV_NAME"): + # Generate an appropriately unique database name for this test run + return "{prefix}_{suffix}".format( + prefix=os.getenv("GITHUB_RUN_ID", "tox"), + suffix=os.getenv("TOX_ENV_NAME").replace("-", "_"), + ) + return "nav" + + +@pytest.fixture(scope='session') +def postgresql(request): + """Fixture for all tests that depend on a running PostgreSQL server. This fixture + will try to detect and use an existing PostgreSQL instance (like if running in a + GitHub action), otherwise it will set up a temporary PostgreSQL server for the test + session.""" + if not is_running_in_github_actions(): + request.getfixturevalue("docker_services") + + dbname = _get_preferred_database_name() + _update_db_conf_for_test_run(dbname) + _populate_test_database(dbname) + yield dbname + print("postgres fixture is done") + + +def _update_db_conf_for_test_run(database_name): + db_conf_path = os.path.join(os.getenv("BUILDDIR"), "etc/db.conf") + + pghost = os.getenv('PGHOST', 'localhost') + pgport = os.getenv('PGPORT', '5432') + pguser = os.getenv('PGUSER', 'nav') + pgpassword = os.getenv('PGPASSWORD', 'nav') + with open(db_conf_path, "w") as output: + output.writelines( + [ + f"dbhost={pghost}\n", + f"dbport={pgport}\n", + f"db_nav={database_name}\n", + f"script_default={pguser}\n", + f"userpw_{pguser}={pgpassword}\n", + "\n", + ] + ) + return db_conf_path + + +@retry(Exception, tries=3, delay=2, backoff=2) +def _populate_test_database(database_name, admin_username, admin_password): + # Init/sync db schema + env = { + 'PGHOST': 'localhost', + 'PGUSER': 'nav', + 'PGDATABASE': database_name, + 'PGPORT': '5432', + 'PGPASSWORD': 'nav', + 'PATH': os.getenv("PATH"), + } + navsyncdb_path = os.path.join(os.getenv("BUILDDIR"), 'bin', 'navsyncdb') + subprocess.check_call([navsyncdb_path], env=env) # , '-c'], #, '--drop-database'], + + # reset password of NAV admin account if indicated by environment + if admin_password: + sql = f"UPDATE account SET password = {adminpassword!r} WHERE login={admin_username!r}" + subprocess.check_call(["psql", "-c", sql, database_name]) + + # Add generic test data set + test_data_path = './tests/docker/scripts/test-data.sql' + subprocess.check_call(["psql", "-f", test_data_path, database_name], env=env) + + @pytest.fixture(scope='session') def admin_username(): return os.environ.get('ADMINUSERNAME', 'admin') @@ -38,7 +115,7 @@ def admin_password(): @pytest.fixture(scope='session') -def gunicorn(): +def gunicorn(postgresql): """Sets up NAV to be served by a gunicorn instance. Useful for tests that need to make external HTTP requests to NAV. diff --git a/tests/docker-compose.yml b/tests/docker-compose.yml new file mode 100644 index 0000000000..5b7ce8ab79 --- /dev/null +++ b/tests/docker-compose.yml @@ -0,0 +1,18 @@ +# This defines external services that the integration tests depend on. Under +# various CI systems, such as GitHub Actions, these services may be provided +# externally. If the test suite deems that they aren't available externally, +# they may be run locally using these definitions instead. +--- +version: '3.5' + +services: + postgres: + image: docker.io/postgres:11 + environment: + - POSTGRES_USER=nav + - POSTGRES_PASSWORD=nav + - POSTGRES_DB=nav + ports: + - 5432:5432 + tmpfs: + /var/lib/postgresql/data diff --git a/tests/docker/scripts/create-db.sh b/tests/docker/scripts/create-db.sh deleted file mode 100755 index 4ee2691a4c..0000000000 --- a/tests/docker/scripts/create-db.sh +++ /dev/null @@ -1,60 +0,0 @@ -#!/bin/bash -xe -# Creates and initializes a NAV database for the test runner - -maybesudo() { - # Run command using gosu or sudo, unless we're running on GitHub Actions - user="$1" - shift - if [ -n "$GITHUB_ACTIONS" ]; then - $@ - else - gosu "$user" $@ - fi -} - -update_nav_db_conf() { - # Update db config - DBCONF="${BUILDDIR}/etc/db.conf" - echo "Updating $DBCONF" - maybesudo root tee "$DBCONF" <=1.0,!=1.1.6 toml +retry 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 5023079e706176733f2d2136eba478c5ed902c52 Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Fri, 10 Feb 2023 11:13:09 +0100 Subject: [PATCH 02/20] Make other database fixtures dependent on Postgres This ensures any tests that rely on the various pre-existing db-related fixtures will have a running PostgreSQL server available. --- tests/integration/conftest.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 38771b1912..6fcd2a0036 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -109,7 +109,7 @@ def _scan_testargs(filename): @pytest.fixture() -def management_profile(): +def management_profile(postgresql): from nav.models.manage import ManagementProfile profile = ManagementProfile( @@ -145,7 +145,7 @@ def localhost(management_profile): @pytest.fixture() -def localhost_using_legacy_db(): +def localhost_using_legacy_db(postgresql): """Alternative to the Django-based localhost fixture, for tests that operate on code that uses legacy database connections. """ @@ -186,7 +186,7 @@ def client(admin_username, admin_password): @pytest.fixture(scope='function') -def db(request): +def db(request, postgresql): """Ensures db modifications are rolled back after the test ends. This is done by disabling transaction management, running everything From 9d1ed801a128f14e832fefafc0cfe403a20ea574 Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Tue, 21 Feb 2023 09:45:19 +0100 Subject: [PATCH 03/20] Make DjangoTransactionTestCase depend on postgres This ensures the postgres fixture is started for all tests that inherit from DjangoTransactionTestCase --- python/nav/tests/cases.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/python/nav/tests/cases.py b/python/nav/tests/cases.py index 0a7c83627e..dbac351821 100644 --- a/python/nav/tests/cases.py +++ b/python/nav/tests/cases.py @@ -14,8 +14,14 @@ # License along with NAV. If not, see . # """NAV test cases""" +import pytest import django.test class DjangoTransactionTestCase(django.test.TestCase): serialized_rollback = True + + @pytest.fixture(autouse=True) + def requirements(self, postgresql): + """Ensures the required pytest fixtures are loaded implicitly for all these tests""" + pass From f398e95dad68947801db06aa830b7c87cae874c5 Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Tue, 21 Feb 2023 09:46:36 +0100 Subject: [PATCH 04/20] Inherit DjangoTransactionTestCase Change TestCase implementations that should really be transactional test cases (which in turn ensures they have PostgreSQL available through the postgres fixture, and that transactions are properly rolled back after the tests have run) --- tests/integration/auditlog_test.py | 6 +++--- tests/integration/querysets_test.py | 5 ++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/integration/auditlog_test.py b/tests/integration/auditlog_test.py index f104a0cb1d..3e75dcb495 100644 --- a/tests/integration/auditlog_test.py +++ b/tests/integration/auditlog_test.py @@ -1,4 +1,4 @@ -from django.test import TestCase +from nav.tests.cases import DjangoTransactionTestCase from nav.models.arnold import Justification @@ -7,7 +7,7 @@ from nav.auditlog.utils import get_auditlog_entries -class AuditlogModelTestCase(TestCase): +class AuditlogModelTestCase(DjangoTransactionTestCase): def setUp(self): # This specific model is used because it is very simple self.justification = Justification.objects.create(name='testarossa') @@ -89,7 +89,7 @@ def test_find_name(self): self.assertEqual(name, 'blocked_reason') -class AuditlogUtilsTestCase(TestCase): +class AuditlogUtilsTestCase(DjangoTransactionTestCase): def setUp(self): # This specific model is used because it is very simple self.justification = Justification.objects.create(name='testarossa') diff --git a/tests/integration/querysets_test.py b/tests/integration/querysets_test.py index c8b97ec55a..53df2ad01e 100644 --- a/tests/integration/querysets_test.py +++ b/tests/integration/querysets_test.py @@ -1,7 +1,6 @@ import datetime as dt -from django.test import TestCase - +from nav.tests.cases import DjangoTransactionTestCase from nav.models.event import AlertHistory from nav.models.event import AlertHistoryVariable from nav.models.event import EventType @@ -13,7 +12,7 @@ from nav.models.manage import Organization -class NetboxQuerysetTest(TestCase): +class NetboxQuerysetTest(DjangoTransactionTestCase): def setUp(self): # Some rows have already been created _netbox_data = { From 1b530d9da8710154f89bcf32a281a1e0323ba54a Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Tue, 21 Feb 2023 09:48:50 +0100 Subject: [PATCH 05/20] Make tests properly depend on postgresql Declares a dependency on the postgresql fixture to ensure the NAV database is actually available for these tests --- tests/integration/bin_test.py | 2 +- tests/integration/networkexplorer_test.py | 17 +++++++++++++++++ tests/integration/seeddb_test.py | 2 +- tests/integration/snmptrapd_test.py | 2 +- tests/integration/sql_test.py | 2 +- tests/integration/watchdog_test.py | 2 +- 6 files changed, 22 insertions(+), 5 deletions(-) diff --git a/tests/integration/bin_test.py b/tests/integration/bin_test.py index 741151a92b..f032ca026d 100644 --- a/tests/integration/bin_test.py +++ b/tests/integration/bin_test.py @@ -7,7 +7,7 @@ BINDIR = './python/nav/bin' -def test_script_runs(script): +def test_script_runs(postgresql, script): """Verifies that a script defined in pyproject.toml runs with a zero exit code""" if "netbiostracker" in script[0] and not which("nbtscan"): pytest.skip("nbtscan is not installed") diff --git a/tests/integration/networkexplorer_test.py b/tests/integration/networkexplorer_test.py index 2e6bde8ada..a44a5cf7a3 100644 --- a/tests/integration/networkexplorer_test.py +++ b/tests/integration/networkexplorer_test.py @@ -1,4 +1,6 @@ +import pytest from unittest import TestCase + from nav.web.networkexplorer import search from nav.web.networkexplorer.forms import NetworkSearchForm from nav.web.networkexplorer.views import ( @@ -18,6 +20,11 @@ class NetworkExplorerSearchTest(TestCase): """ + @pytest.fixture(autouse=True) + def requirements(self, postgresql): + """Ensures the required pytest fixtures are loaded implicitly for all these tests""" + pass + def test_search_expand_swport(self): search.search_expand_swport(1) @@ -73,6 +80,11 @@ class TestDataMixin(object): class ViewsTest(TestDataMixin, TestCase): + @pytest.fixture(autouse=True) + def requirements(self, postgresql): + """Ensures the required pytest fixtures are loaded implicitly for all these tests""" + pass + def setUp(self): self.factory = RequestFactory() self.url_root = '/networkexplorer/' @@ -117,6 +129,11 @@ def test_search_view_with_invalid_query(self): class FormsTest(TestDataMixin, TestCase): + @pytest.fixture(autouse=True) + def requirements(self, postgresql): + """Ensures the required pytest fixtures are loaded implicitly for all these tests""" + pass + def test_search_form(self): valid_form = NetworkSearchForm(self.valid_data) diff --git a/tests/integration/seeddb_test.py b/tests/integration/seeddb_test.py index c02b21ddf5..e7810347d2 100644 --- a/tests/integration/seeddb_test.py +++ b/tests/integration/seeddb_test.py @@ -15,7 +15,7 @@ def test_usage_edit_url_should_allow_slashes(): assert reverse('seeddb-usage-edit', args=('TEST/SLASH',)) -def test_editing_deleted_netboxes_should_raise_404(admin_account): +def test_editing_deleted_netboxes_should_raise_404(postgresql, admin_account): netboxid = 666 # Assuming no such netbox exists in test data set! factory = RequestFactory() url = reverse('seeddb-netbox-edit', args=(netboxid,)) diff --git a/tests/integration/snmptrapd_test.py b/tests/integration/snmptrapd_test.py index c4d63e09df..9fdcba5a42 100644 --- a/tests/integration/snmptrapd_test.py +++ b/tests/integration/snmptrapd_test.py @@ -28,7 +28,7 @@ def test_plugin_loader_raises_no_exception_if_plugin_has_no_initialize_method(): assert not hasattr(loader[0], 'initialize') -def test_plugin_loader_reading_in_modules_from_config_file(): +def test_plugin_loader_reading_in_modules_from_config_file(postgresql): configfile = find_config_file("snmptrapd.conf") config = configparser.ConfigParser() config.read(configfile) diff --git a/tests/integration/sql_test.py b/tests/integration/sql_test.py index 4859110ed5..d773f19fcf 100644 --- a/tests/integration/sql_test.py +++ b/tests/integration/sql_test.py @@ -2,7 +2,7 @@ from nav.db import getConnection -def test_public_namespace_should_be_empty(): +def test_public_namespace_should_be_empty(postgresql): cursor = getConnection('default').cursor() cursor.execute( """SELECT relname diff --git a/tests/integration/watchdog_test.py b/tests/integration/watchdog_test.py index 9c0f183c80..3d730607c4 100644 --- a/tests/integration/watchdog_test.py +++ b/tests/integration/watchdog_test.py @@ -1,7 +1,7 @@ from nav.watchdog.util import get_statuses -def test_get_status_cache_does_not_raise(): +def test_get_status_cache_does_not_raise(postgresql): """Regression test for issue where pickle cache is poisoned from cross-environment testing - get_statuses() cache handling was bad """ From 3dcfe6987c9a13a7a2c79053dcd942fe82e501b5 Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Tue, 21 Feb 2023 09:51:21 +0100 Subject: [PATCH 06/20] Make client test fixture depend on postgresql This ensures that any test that depends on a logged in web client also automatically depends on having the NAV database running. --- tests/integration/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 6fcd2a0036..8acdec66fc 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -174,7 +174,7 @@ def localhost_using_legacy_db(postgresql): @pytest.fixture(scope='function') -def client(admin_username, admin_password): +def client(postgresql, admin_username, admin_password): """Provides a Django test Client object already logged in to the web UI as an admin""" from django.urls import reverse From 590ed7f53bf86ceb599ae72edf1ae35d1fb3a799 Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Tue, 21 Feb 2023 09:52:46 +0100 Subject: [PATCH 07/20] Make snmpsim fixture work portably This ensure the snmpsim fixture can run both on Linux and MacOS, by replacing Linux specific parts (like looking at /proc/net/udp). This slightly changes the logic of verifying whether the snmpsimd process is up and running before yielding the fixture value: Instead of waiting for something to appear to listen to port 1024, this verifies the presence of the SNMP agent by performing an actual SNMP query. --- tests/integration/conftest.py | 40 +++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 8acdec66fc..1875b33598 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -6,13 +6,15 @@ from itertools import cycle from shutil import which import subprocess -import time import toml import pytest +from retry import retry from django.test import Client +SNMP_TEST_PORT = 1024 + ######################################################################## # # # Set up the required components for an integration test. Components # @@ -261,21 +263,26 @@ def snmpsim(): """ snmpsimd = which('snmpsim-command-responder') assert snmpsimd, "Could not find snmpsimd.py" - workspace = os.getenv('WORKSPACE', os.getenv('HOME', '/source')) + workspace = os.getenv('WORKSPACE', os.getcwd()) proc = subprocess.Popen( [ snmpsimd, '--data-dir={}/tests/integration/snmp_fixtures'.format(workspace), '--log-level=error', - '--agent-udpv4-endpoint=127.0.0.1:1024', + '--agent-udpv4-endpoint=127.0.0.1:{}'.format(SNMP_TEST_PORT), ], env={'HOME': workspace}, ) - while not _lookfor('0100007F:0400', '/proc/net/udp'): - print("Still waiting for snmpsimd to listen for queries") - proc.poll() - time.sleep(0.1) + @retry(Exception, tries=3, delay=0.5, backoff=2) + def _wait_for_snmpsimd(): + if _verify_localhost_snmp_response(): + return True + else: + proc.poll() + raise TimeoutError("Still waiting for snmpsimd to listen for queries") + + _wait_for_snmpsimd() yield proc.kill() @@ -292,7 +299,7 @@ def snmp_agent_proxy(snmpsim, snmp_ports): port = next(snmp_ports) agent = AgentProxy( '127.0.0.1', - 1024, + SNMP_TEST_PORT, community='placeholder', snmpVersion='v2c', protocol=port.protocol, @@ -316,14 +323,19 @@ def snmp_ports(): return _ports -def _lookfor(string, filename): - """Very simple grep-like function""" - data = io.open(filename, 'r', encoding='utf-8').read() - return string in data - - @pytest.fixture def admin_account(db): from nav.models.profiles import Account yield Account.objects.get(id=Account.ADMIN_ACCOUNT) + + +def _verify_localhost_snmp_response(port=SNMP_TEST_PORT): + """Verifies that the snmpsimd fixture process is responding, by using NAV's own + SNMP framework to query it. + """ + from nav.Snmp import Snmp + + session = Snmp(host="127.0.0.1", community="public", version="2c", port=port) + resp = session.jog("1.3.6.1.2.1.47.1.1.1.1.2") + return resp From 68a601395a602cc542ccb58d012a8ca08c7f87b7 Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Tue, 21 Feb 2023 09:57:10 +0100 Subject: [PATCH 08/20] Move imports from global to local These imports made the entire test module depend on the PostgreSQL database actually running. Moving the imports to be local to the tests (that depend directly or indirectly) on the postgresql fixture ensures that the imports only take place after PostgreSQL is actually running. --- tests/integration/widget_test.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/integration/widget_test.py b/tests/integration/widget_test.py index df4d354693..6be324b7ff 100644 --- a/tests/integration/widget_test.py +++ b/tests/integration/widget_test.py @@ -2,16 +2,14 @@ from django.urls import reverse -from nav.web.navlets.roomstatus import RoomStatus -from nav.web.navlets.feedreader import FeedReaderNavlet -from nav.models.event import AlertHistory, AlertHistoryMessage -from nav.models.profiles import AccountNavlet from nav.models.fields import INFINITY import pytest def test_roomstatus_should_not_fail_on_multiple_messages(alerthist_with_two_messages): + from nav.web.navlets.roomstatus import RoomStatus + widget = RoomStatus() result = widget.get_context_data_view({}) print(result) @@ -23,6 +21,8 @@ def test_roomstatus_should_not_fail_on_multiple_messages(alerthist_with_two_mess def test_feedreader_widget_should_get_nav_blog_posts(): + from nav.web.navlets.feedreader import FeedReaderNavlet + widget = FeedReaderNavlet() feed = widget._get_feed('http://blog.nav.uninett.no/rss', maxposts=0) print(repr(feed)) @@ -41,6 +41,8 @@ def test_get_navlet_should_return_200(client, admin_navlet): def test_get_pdu_navlet_in_edit_mode_should_return_200(client, admin_account): """Tests a GET request against the pdu navlet in edit mode""" + from nav.models.profiles import AccountNavlet + pdu_navlet = AccountNavlet.objects.create( navlet="nav.web.navlets.pdu.PduWidget", account=admin_account, @@ -60,6 +62,8 @@ def test_get_pdu_navlet_in_edit_mode_should_return_200(client, admin_account): @pytest.fixture() def alerthist_with_two_messages(localhost): + from nav.models.event import AlertHistory, AlertHistoryMessage + alert = AlertHistory( source_id='ipdevpoll', netbox=localhost, From 5a4317c900b137602b9a39eee7d4a6c65f019015 Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Tue, 21 Feb 2023 09:58:32 +0100 Subject: [PATCH 09/20] Add TODO for the admin_navlet fixture The admin_navlet fixture is dynamic/parametrized, which is why its implementation is a bit weird. Unfortunately, the tests that depend on it currently fail under the new regime of the postgresql fixture. Adding this note so we know where to go back and fix it. --- tests/integration/conftest.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 1875b33598..5c3a7313ec 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -46,6 +46,7 @@ def pytest_generate_tests(metafunc): ids = [s[0] for s in scripts] metafunc.parametrize("script", _nav_script_tests(), ids=ids) elif 'admin_navlet' in metafunc.fixturenames: + # TODO This needs to be reworked due to the new postgresql fixture from nav.models.profiles import AccountNavlet navlets = AccountNavlet.objects.filter(account__login='admin') From 83cbc4a575909d730bf3bfe0797b9e6aa360dfbf Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Mon, 5 Jun 2023 13:48:05 +0200 Subject: [PATCH 10/20] "Un-parametrize" test_get_navlet_should_return_200 While pytest can accomplish a lot of exciting things, it cannot use fixtures as input to test parametrization. While can make a test depend on a fixture for getting a postgresql instance, the test discovery phase that generates the tests cannot. I.e. we cannot get our test parameters from a database unless the database was already up and running when the test discovery phase started. Sad, but true,. This changes the navlet test to iterate over the available admin navlets itself. --- tests/integration/conftest.py | 6 ------ tests/integration/widget_test.py | 22 +++++++++++++++------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 5c3a7313ec..daee71bc5e 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -45,12 +45,6 @@ def pytest_generate_tests(metafunc): 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: - # TODO This needs to be reworked due to the new postgresql fixture - from nav.models.profiles import AccountNavlet - - navlets = AccountNavlet.objects.filter(account__login='admin') - metafunc.parametrize("admin_navlet", navlets) def _nav_script_tests(): diff --git a/tests/integration/widget_test.py b/tests/integration/widget_test.py index 6be324b7ff..18a13b8ecc 100644 --- a/tests/integration/widget_test.py +++ b/tests/integration/widget_test.py @@ -29,14 +29,22 @@ def test_feedreader_widget_should_get_nav_blog_posts(): assert len(feed) > 0 -def test_get_navlet_should_return_200(client, admin_navlet): +def test_get_navlets_should_return_200(client): """Tests a GET request against each of the admin user's navlets""" - url = reverse('get-user-navlet', kwargs={'navlet_id': admin_navlet.id}) - print( - "Testing admin navlet instance of {!r} at {!r}".format(admin_navlet.navlet, url) - ) - response = client.get(url) - assert response.status_code == 200 + from nav.models.profiles import AccountNavlet + + navlets = AccountNavlet.objects.filter(account__login='admin') + for admin_navlet in navlets: + url = reverse('get-user-navlet', kwargs={'navlet_id': admin_navlet.id}) + print( + "Testing admin navlet instance of {!r} at {!r}".format( + admin_navlet.navlet, url + ) + ) + response = client.get(url) + assert response.status_code == 200, "navlet {} did not respond with 200".format( + admin_navlet.navlet + ) def test_get_pdu_navlet_in_edit_mode_should_return_200(client, admin_account): From 3b309c255bfe0d2d8a26fc6fe60dbad82e1b22ec Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Mon, 5 Jun 2023 16:40:53 +0200 Subject: [PATCH 11/20] Properly mark tests as wanting postgresql These tests would only work incidentally; i.e. if they ran after another test that depended on the postgresql fixture --- python/nav/web/info/room/views.py | 2 +- tests/integration/alertengine_test.py | 2 +- tests/integration/eventengine/upgrade_test.py | 2 +- tests/integration/models/account_test.py | 25 ++++++++++++------- .../models/alerthistvarmap_test.py | 2 +- tests/integration/models/eventvarmap_test.py | 2 +- tests/integration/report/generator_test.py | 2 +- tests/integration/thresholdmon/test_events.py | 2 +- tests/integration/web/info_test.py | 4 +-- 9 files changed, 25 insertions(+), 18 deletions(-) diff --git a/python/nav/web/info/room/views.py b/python/nav/web/info/room/views.py index f50e2df831..2e22cbb963 100644 --- a/python/nav/web/info/room/views.py +++ b/python/nav/web/info/room/views.py @@ -236,7 +236,7 @@ def render_netboxes(request, roomid): @require_http_methods(['POST']) -def create_csv(request): +def create_csv(request) -> HttpResponse: """Create csv-file from form data""" roomname = request.POST.get('roomid', 'room').encode('utf-8') filename = "{}.csv".format(roomname) diff --git a/tests/integration/alertengine_test.py b/tests/integration/alertengine_test.py index 8ca0eb6fb3..09a85baa62 100644 --- a/tests/integration/alertengine_test.py +++ b/tests/integration/alertengine_test.py @@ -2,7 +2,7 @@ from nav.alertengine.dispatchers import Dispatcher -def test_all_handlers_should_be_loadable(): +def test_all_handlers_should_be_loadable(postgresql): for sender in AlertSender.objects.filter(supported=True): dispatcher = sender.load_dispatcher_class() assert issubclass(dispatcher, Dispatcher) diff --git a/tests/integration/eventengine/upgrade_test.py b/tests/integration/eventengine/upgrade_test.py index 83ca32df88..6bb8e50646 100644 --- a/tests/integration/eventengine/upgrade_test.py +++ b/tests/integration/eventengine/upgrade_test.py @@ -77,7 +77,7 @@ def test_upgrade_handler_should_not_fail_if_old_and_new_version_do_not_exist( @pytest.fixture() -def netbox_having_sw_upgrade(): +def netbox_having_sw_upgrade(postgresql): box = Netbox( ip="10.254.254.254", sysname="upgradehost.example.org", diff --git a/tests/integration/models/account_test.py b/tests/integration/models/account_test.py index cc4c7587a5..9b9bad487b 100644 --- a/tests/integration/models/account_test.py +++ b/tests/integration/models/account_test.py @@ -1,14 +1,21 @@ -import unittest from nav.models.profiles import Account +import pytest -class AccountTest(unittest.TestCase): - def setUp(self): - self.admin_user = Account.objects.get(pk=Account.ADMIN_ACCOUNT) - self.default_user = Account.objects.get(pk=Account.DEFAULT_ACCOUNT) - def test_is_admin_returns_true_if_administrator(self): - self.assertTrue(self.admin_user.is_admin()) +def test_is_admin_returns_true_if_administrator(admin_user): + assert admin_user.is_admin() - def test_is_admin_returns_false_if_default_account(self): - self.assertFalse(self.default_user.is_admin()) + +def test_is_admin_returns_false_if_default_account(default_user): + assert not default_user.is_admin() + + +@pytest.fixture +def admin_user(postgresql): + yield Account.objects.get(pk=Account.ADMIN_ACCOUNT) + + +@pytest.fixture +def default_user(postgresql): + yield Account.objects.get(pk=Account.DEFAULT_ACCOUNT) diff --git a/tests/integration/models/alerthistvarmap_test.py b/tests/integration/models/alerthistvarmap_test.py index 4856b37515..033766740e 100644 --- a/tests/integration/models/alerthistvarmap_test.py +++ b/tests/integration/models/alerthistvarmap_test.py @@ -68,7 +68,7 @@ def test_alerthist_varmap_single_key_can_be_updated_after_reload(simple_alerthis @pytest.fixture -def simple_alerthist(): +def simple_alerthist(postgresql): hist = AlertHistory( source_id='ipdevpoll', event_type_id='info', diff --git a/tests/integration/models/eventvarmap_test.py b/tests/integration/models/eventvarmap_test.py index 7fd8a80db3..412e583e0d 100644 --- a/tests/integration/models/eventvarmap_test.py +++ b/tests/integration/models/eventvarmap_test.py @@ -40,7 +40,7 @@ def test_event_varmap_single_key_can_be_updated_after_reload(simple_event): @pytest.fixture -def simple_event(): +def simple_event(postgresql): event = EventQueue( source_id='ipdevpoll', target_id='eventEngine', event_type_id='info' ) diff --git a/tests/integration/report/generator_test.py b/tests/integration/report/generator_test.py index bd342c16e3..4af4928b04 100644 --- a/tests/integration/report/generator_test.py +++ b/tests/integration/report/generator_test.py @@ -26,7 +26,7 @@ def report_list(): @pytest.mark.parametrize("report_name", report_list()) -def test_report(report_name): +def test_report(report_name, postgresql): # uri = 'http://example.com/report/%s/' % report_name uri = QueryDict('').copy() db.closeConnections() # Ensure clean connection for each test diff --git a/tests/integration/thresholdmon/test_events.py b/tests/integration/thresholdmon/test_events.py index 83ea8ad46a..803af276d7 100644 --- a/tests/integration/thresholdmon/test_events.py +++ b/tests/integration/thresholdmon/test_events.py @@ -23,7 +23,7 @@ def test_events(rule): @pytest.fixture -def rule(): +def rule(postgresql): rule = ThresholdRule(target='foo.bar>1', alert='high foobar') rule.save() yield rule diff --git a/tests/integration/web/info_test.py b/tests/integration/web/info_test.py index 79969be387..8da3ba6994 100644 --- a/tests/integration/web/info_test.py +++ b/tests/integration/web/info_test.py @@ -36,7 +36,7 @@ def test_failures_should_be_mentioned_in_search_page(client, failing_searchprovi assert failing_searchprovider in response.content.decode('utf-8') -def test_room_csv_download_should_not_produce_bytestring_representations(admin_account): +def test_room_csv_download_should_not_produce_bytestring_representations(postgresql, admin_account): factory = RequestFactory() request = factory.post( reverse("room-csv"), data={"roomid": "myroom", "rows": "one;two;three\n"} @@ -44,7 +44,7 @@ def test_room_csv_download_should_not_produce_bytestring_representations(admin_a request.account = admin_account request.session = MagicMock() - response = create_csv(request) # type: django.http.response.HttpResponse + response = create_csv(request) assert not response.content.startswith(b"b'") From 8cbbdfcf0b156cc9a7d3c4ff14b8dda04c3dc13a Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Mon, 5 Jun 2023 17:21:44 +0200 Subject: [PATCH 12/20] Add usage tips to docstring The postgresql fixture is fine to depend on if you just need a database to be present. But if you need to write data during the test, you should probably use the db fixture. --- tests/conftest.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index d35072bcdb..7d92673083 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -48,7 +48,14 @@ def postgresql(request): """Fixture for all tests that depend on a running PostgreSQL server. This fixture will try to detect and use an existing PostgreSQL instance (like if running in a GitHub action), otherwise it will set up a temporary PostgreSQL server for the test - session.""" + session. + + If your test needs to write to the database, it should ask for the `db` fixture + instead, as this ensures changes are rolled back when the test is done. However, + if your test makes db changes that need to be visible from another process, you + must make your own data fixture to ensure the data is removed when the test is + done. + """ if not is_running_in_github_actions(): request.getfixturevalue("docker_services") From 11c072636113144d1e9bc112a71b794d92f287e9 Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Mon, 5 Jun 2023 17:51:01 +0200 Subject: [PATCH 13/20] Introduce pytest mark for really slow tests --- tests/integration/web/crawler_test.py | 2 ++ tox.ini | 1 + 2 files changed, 3 insertions(+) diff --git a/tests/integration/web/crawler_test.py b/tests/integration/web/crawler_test.py index ab69fd74ff..28c9ee5e16 100644 --- a/tests/integration/web/crawler_test.py +++ b/tests/integration/web/crawler_test.py @@ -224,6 +224,7 @@ def webcrawler(gunicorn, admin_username, admin_password): # +@pytest.mark.slow def test_all_links_should_be_reachable(webcrawler): unreachable = [] for page in webcrawler.crawl(): @@ -243,6 +244,7 @@ def _content_as_string(content): return content.decode('utf-8') +@pytest.mark.slow def test_page_should_be_valid_html(webcrawler): try: tidy_document("") diff --git a/tox.ini b/tox.ini index 2f716a2e40..bcc61b4b6d 100644 --- a/tox.ini +++ b/tox.ini @@ -16,6 +16,7 @@ basepython = python3.9 addopts = --failed-first markers = twisted: marks tests as needing twisted async to run + slow: potentially really slow tests [gh-actions] python = From c35992743324417fd64c47481ce0727a520448ac Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Tue, 5 Sep 2023 16:03:08 +0200 Subject: [PATCH 14/20] Leave out optional requirements from test env For now, at least. If the Open-LDAP libraries aren't available locally, the test suite will fail because python-ldap will not build. --- tox.ini | 1 - 1 file changed, 1 deletion(-) diff --git a/tox.ini b/tox.ini index bcc61b4b6d..e10866a1a8 100644 --- a/tox.ini +++ b/tox.ini @@ -29,7 +29,6 @@ deps = pip -r tests/requirements.txt -r requirements/base.txt - -r requirements/optional.txt -r requirements/django{env:DJANGO_VER}.txt -c constraints.txt From 9ed5b1ba7fa4d728ccb3d21e9ede9ba211e6dbb0 Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Wed, 2 Oct 2024 16:43:07 +0000 Subject: [PATCH 15/20] Add support for NAV_CONFIG_DIR env var The NAV_CONFIG_DIR environment variable can be used to insert a custom directory as the first entry in the list of directories NAV will search for its config files. --- python/nav/config.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/python/nav/config.py b/python/nav/config.py index dc59fbe05b..74a26f2c4c 100644 --- a/python/nav/config.py +++ b/python/nav/config.py @@ -56,6 +56,8 @@ os.path.join(_venv, 'etc/nav'), os.path.join(_venv, buildconf.datadir, 'conf'), ] + CONFIG_LOCATIONS +if "NAV_CONFIG_DIR" in os.environ: + CONFIG_LOCATIONS.insert(0, os.environ["NAV_CONFIG_DIR"]) def list_config_files_from_dir(dirname): From 99c68062f3cc54991e62e059472b9aa56a1028c1 Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Fri, 4 Oct 2024 10:17:51 +0000 Subject: [PATCH 16/20] Set db admin password correctly Both the SQL statement and the command line were incorrect. --- tests/conftest.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 7d92673083..28515233c8 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,4 +1,5 @@ """Pytest config and fixtures for all test suites""" + import os import platform import subprocess @@ -103,8 +104,8 @@ def _populate_test_database(database_name, admin_username, admin_password): # reset password of NAV admin account if indicated by environment if admin_password: - sql = f"UPDATE account SET password = {adminpassword!r} WHERE login={admin_username!r}" - subprocess.check_call(["psql", "-c", sql, database_name]) + sql = f"UPDATE account SET password = {admin_password!r} WHERE login={admin_username!r}" + subprocess.check_call(["psql", "-c", sql, database_name], env=env) # Add generic test data set test_data_path = './tests/docker/scripts/test-data.sql' From cc71f9b27885925e89706bee729bba3c690c3518 Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Fri, 4 Oct 2024 10:18:56 +0000 Subject: [PATCH 17/20] Use fixtures to get admin user details The postgresql fixture needs to pass on the admin user's name and password as fetched from the fixtures, in order for the test database to be properly populated. --- tests/conftest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 28515233c8..fa21e44878 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -45,7 +45,7 @@ def _get_preferred_database_name(): @pytest.fixture(scope='session') -def postgresql(request): +def postgresql(request, admin_username, admin_password): """Fixture for all tests that depend on a running PostgreSQL server. This fixture will try to detect and use an existing PostgreSQL instance (like if running in a GitHub action), otherwise it will set up a temporary PostgreSQL server for the test @@ -62,7 +62,7 @@ def postgresql(request): dbname = _get_preferred_database_name() _update_db_conf_for_test_run(dbname) - _populate_test_database(dbname) + _populate_test_database(dbname, admin_username, admin_password) yield dbname print("postgres fixture is done") From f309d5aa756a95cae9b9f7d43d03dbb4de6bfe12 Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Fri, 4 Oct 2024 11:19:54 +0000 Subject: [PATCH 18/20] Move collectstatic from tox to pytest fixture This ensures static files are collected only by tests that require it. It also ensures that the static files are collected into a temporary directory and that the WhiteNoise web server is directed to base its document root on this path (so we don't try to write to some system level directory we don't have write access to) --- tests/conftest.py | 20 ++++++++++++++++++-- tests/navtest_wsgi.py | 5 ++++- tox.ini | 2 -- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index fa21e44878..04b2cf6943 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -9,6 +9,9 @@ from requests.adapters import HTTPAdapter, Retry from retry import retry +from django.core.management import call_command +from django.test import override_settings + def pytest_configure(config): # Bootstrap Django config @@ -123,7 +126,20 @@ def admin_password(): @pytest.fixture(scope='session') -def gunicorn(postgresql): +def staticfiles(tmp_path_factory): + """Collects Django static files into a temporary directory and return the web root + directory path that can be served by a web server. + """ + webroot = tmp_path_factory.mktemp("webroot") + static = webroot / "static" + with override_settings(STATIC_ROOT=static): + print(f"Collecting static files in {static!r}") + call_command('collectstatic', interactive=False) + yield webroot + + +@pytest.fixture(scope='session') +def gunicorn(postgresql, staticfiles): """Sets up NAV to be served by a gunicorn instance. Useful for tests that need to make external HTTP requests to NAV. @@ -140,7 +156,7 @@ def gunicorn(postgresql): errorlog, '--access-logfile', accesslog, - 'navtest_wsgi:application', + f'navtest_wsgi:nav_test_app(root={str(staticfiles)!r})', ] ) # Allow for gunicorn to become ready to serve requests before handing off to a test diff --git a/tests/navtest_wsgi.py b/tests/navtest_wsgi.py index a46222b9fb..f488c3ae4f 100644 --- a/tests/navtest_wsgi.py +++ b/tests/navtest_wsgi.py @@ -23,4 +23,7 @@ from nav import buildconf -application = WhiteNoise(application, root=buildconf.webrootdir) + +def nav_test_app(root=buildconf.webrootdir): + """Returns a WhiteNoise application instance of NAV with the given web root""" + return WhiteNoise(application, root=root) diff --git a/tox.ini b/tox.ini index e10866a1a8..fa37114dd2 100644 --- a/tox.ini +++ b/tox.ini @@ -82,11 +82,9 @@ commands = integration: sed -i 's/^\#DJANGO_DEBUG.*/DJANGO_DEBUG=True/' {envdir}/etc/nav.conf integration: sed -i 's/^NAV_USER.*/NAV_USER={env:USER}/' {envdir}/etc/nav.conf integration: sed -i 's,^\#base.*,base=http://localhost:9000,' {envdir}/etc/graphite.conf - integration: django-admin collectstatic --noinput integration: pytest -o junit_suite_name="{envname} integration tests" --cov-config {toxinidir}/tests/.coveragerc --cov={toxinidir}/python --cov-report=xml:reports/{envname}/coverage.xml --html reports/{envname}/integration-report.html --junitxml=reports/{envname}/integration-results.xml --verbose --showlocals {posargs:tests/integration} functional: sed -i 's/^nav.*=.*INFO/root=DEBUG/' {envdir}/etc/logging.conf - functional: django-admin collectstatic --noinput functional: pytest -o junit_suite_name="{envname} functional tests" --junitxml=reports/{envname}/functional-results.xml --verbose --driver Chrome --driver-path=/usr/local/bin/chromedriver --sensitive-url "nothing to see here" --html reports/{envname}/functional-report.html {posargs:tests/functional} From 7c55e5e58f5892f28e02ae3084080e3ed80b2d64 Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Fri, 4 Oct 2024 12:33:56 +0000 Subject: [PATCH 19/20] Move SASS build from tox to pytest fixture This ensures stylesheets are built only on-demand --- tests/conftest.py | 8 +++++++- tox.ini | 2 -- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 04b2cf6943..bbac5c5d71 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -126,7 +126,13 @@ def admin_password(): @pytest.fixture(scope='session') -def staticfiles(tmp_path_factory): +def build_sass(): + """Builds the NAV SASS files into CSS files that can be installed as static files""" + subprocess.check_call(["make", "sassbuild"]) + + +@pytest.fixture(scope='session') +def staticfiles(build_sass, tmp_path_factory): """Collects Django static files into a temporary directory and return the web root directory path that can be served by a web server. """ diff --git a/tox.ini b/tox.ini index fa37114dd2..bf02e7a223 100644 --- a/tox.ini +++ b/tox.ini @@ -73,8 +73,6 @@ commands_pre = commands = unit: pytest -o junit_suite_name="{envname} unit tests" --cov-config {toxinidir}/tests/.coveragerc --cov={toxinidir}/python --cov-report=xml:reports/{envname}/coverage.xml --junitxml=reports/{envname}/unit-results.xml --verbose {posargs:tests/unittests} - {integration,functional}: make sassbuild - integration: python -m nav.django.manage check {integration,functional}: nav config install {envdir}/etc {integration,functional}: mkdir -p {envdir}/uploads From 9168ad043fcaa6dffa0baffafde5573d04300090 Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Fri, 4 Oct 2024 12:53:40 +0000 Subject: [PATCH 20/20] Move django-admin check command to test suite Instead of running it as part of tox, this ensures it runs as an explicit test in the integration test suite. --- tests/integration/django_test.py | 8 ++++++++ tox.ini | 1 - 2 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 tests/integration/django_test.py diff --git a/tests/integration/django_test.py b/tests/integration/django_test.py new file mode 100644 index 0000000000..3539e5c501 --- /dev/null +++ b/tests/integration/django_test.py @@ -0,0 +1,8 @@ +from django.core.management import call_command + + +def test_django_manage_check(): + """Runs Django's `check` management command to verify the Django project is + correctly configured. + """ + call_command('check') diff --git a/tox.ini b/tox.ini index bf02e7a223..d384615268 100644 --- a/tox.ini +++ b/tox.ini @@ -73,7 +73,6 @@ commands_pre = commands = unit: pytest -o junit_suite_name="{envname} unit tests" --cov-config {toxinidir}/tests/.coveragerc --cov={toxinidir}/python --cov-report=xml:reports/{envname}/coverage.xml --junitxml=reports/{envname}/unit-results.xml --verbose {posargs:tests/unittests} - integration: python -m nav.django.manage check {integration,functional}: nav config install {envdir}/etc {integration,functional}: mkdir -p {envdir}/uploads {integration,functional}: sed -i 's,^\#\?UPLOAD_DIR.*.,UPLOAD_DIR={envdir}/uploads,' {envdir}/etc/nav.conf