From 53ef8835c131650b0f6a5db1652b2fec6dbec0e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Mon, 14 Mar 2022 08:59:32 +0100 Subject: [PATCH 01/10] Reuse common port-related test fixtures Most Python-based system tests need to know which ports were assigned to a given test by bin/tests/system/get_ports.sh. This is currently handled by inspecting the values of various environment variables (set by bin/tests/system/run.sh) and passing the port numbers to Python scripts via pytest fixtures. However, this glue code has so far been copy-pasted into each system test using it, rather than reused. Since pytest also looks for conftest.py files in parent directories, move commonly used fixtures to bin/tests/system/conftest.py. Set the scope of all the moved fixtures to "session" as their return values are only based on environment variables, so there is no point in recreating them for every test requesting them. Adjust test code accordingly. --- bin/tests/system/checkds/conftest.py | 25 --------------- bin/tests/system/{wildcard => }/conftest.py | 17 +++++++++-- bin/tests/system/dispatch/conftest.py | 25 --------------- bin/tests/system/dispatch/tests-connreset.py | 4 +-- bin/tests/system/doth/conftest.py | 6 ---- bin/tests/system/rpzextra/conftest.py | 12 -------- bin/tests/system/run.sh.in | 2 +- bin/tests/system/shutdown/conftest.py | 25 --------------- bin/tests/system/statschannel/conftest.py | 14 +-------- bin/tests/system/statschannel/tests-json.py | 4 +-- bin/tests/system/statschannel/tests-xml.py | 4 +-- bin/tests/system/tcp/conftest.py | 13 -------- bin/tests/system/tcp/tests-tcp.py | 8 ++--- bin/tests/system/timeouts/conftest.py | 12 -------- bin/tests/system/timeouts/tests-tcp.py | 32 ++++++++++---------- 15 files changed, 43 insertions(+), 160 deletions(-) rename bin/tests/system/{wildcard => }/conftest.py (57%) delete mode 100644 bin/tests/system/dispatch/conftest.py diff --git a/bin/tests/system/checkds/conftest.py b/bin/tests/system/checkds/conftest.py index bfe2ff40c6..5328ae0177 100644 --- a/bin/tests/system/checkds/conftest.py +++ b/bin/tests/system/checkds/conftest.py @@ -9,7 +9,6 @@ # See the COPYRIGHT file distributed with this work for additional # information regarding copyright ownership. -import os import pytest @@ -45,27 +44,3 @@ def pytest_collection_modifyitems(config, items): for item in items: if "dnspython2" in item.keywords: item.add_marker(skip_dnspython2) - - -@pytest.fixture -def named_port(request): - # pylint: disable=unused-argument - port = os.getenv("PORT") - if port is None: - port = 5301 - else: - port = int(port) - - return port - - -@pytest.fixture -def control_port(request): - # pylint: disable=unused-argument - port = os.getenv("CONTROLPORT") - if port is None: - port = 5301 - else: - port = int(port) - - return port diff --git a/bin/tests/system/wildcard/conftest.py b/bin/tests/system/conftest.py similarity index 57% rename from bin/tests/system/wildcard/conftest.py rename to bin/tests/system/conftest.py index 057c3785df..2fd05add7d 100644 --- a/bin/tests/system/wildcard/conftest.py +++ b/bin/tests/system/conftest.py @@ -1,3 +1,5 @@ +#!/usr/bin/python3 + # Copyright (C) Internet Systems Consortium, Inc. ("ISC") # # SPDX-License-Identifier: MPL-2.0 @@ -10,9 +12,20 @@ # information regarding copyright ownership. import os + import pytest -@pytest.fixture(scope='module') +@pytest.fixture(scope='session') def named_port(): - return int(os.environ.get("PORT", default=5300)) + return int(os.environ.get('PORT', default=5300)) + + +@pytest.fixture(scope='session') +def named_tlsport(): + return int(os.environ.get('TLSPORT', default=8853)) + + +@pytest.fixture(scope='session') +def control_port(): + return int(os.environ.get('CONTROLPORT', default=9953)) diff --git a/bin/tests/system/dispatch/conftest.py b/bin/tests/system/dispatch/conftest.py deleted file mode 100644 index c1c19fde61..0000000000 --- a/bin/tests/system/dispatch/conftest.py +++ /dev/null @@ -1,25 +0,0 @@ -# Copyright (C) Internet Systems Consortium, Inc. ("ISC") -# -# SPDX-License-Identifier: MPL-2.0 -# -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, you can obtain one at https://mozilla.org/MPL/2.0/. -# -# See the COPYRIGHT file distributed with this work for additional -# information regarding copyright ownership. - -import os -import pytest - - -@pytest.fixture -def port(request): - # pylint: disable=unused-argument - env_port = os.getenv("PORT") - if env_port is None: - env_port = 5300 - else: - env_port = int(env_port) - - return env_port diff --git a/bin/tests/system/dispatch/tests-connreset.py b/bin/tests/system/dispatch/tests-connreset.py index 64c61a95d5..61758ffe6d 100644 --- a/bin/tests/system/dispatch/tests-connreset.py +++ b/bin/tests/system/dispatch/tests-connreset.py @@ -19,8 +19,8 @@ import dns.query import dns.rcode -def test_connreset(port): +def test_connreset(named_port): msg = dns.message.make_query("sub.example.", "A", want_dnssec=True, use_edns=0, payload=1232) - ans = dns.query.udp(msg, "10.53.0.2", timeout=10, port=port) + ans = dns.query.udp(msg, "10.53.0.2", timeout=10, port=named_port) assert ans.rcode() == dns.rcode.SERVFAIL diff --git a/bin/tests/system/doth/conftest.py b/bin/tests/system/doth/conftest.py index 8978b652aa..ed46680c71 100644 --- a/bin/tests/system/doth/conftest.py +++ b/bin/tests/system/doth/conftest.py @@ -11,7 +11,6 @@ # See the COPYRIGHT file distributed with this work for additional # information regarding copyright ownership. -import os import shutil import subprocess @@ -36,8 +35,3 @@ def gnutls_cli_executable(): pytest.skip('gnutls-cli does not support the --logfile option') return executable - - -@pytest.fixture -def named_tlsport(): - return int(os.environ.get('TLSPORT', '853')) diff --git a/bin/tests/system/rpzextra/conftest.py b/bin/tests/system/rpzextra/conftest.py index 05bf144190..8bbcf25ad7 100644 --- a/bin/tests/system/rpzextra/conftest.py +++ b/bin/tests/system/rpzextra/conftest.py @@ -40,15 +40,3 @@ def pytest_collection_modifyitems(config, items): for item in items: if "json" in item.keywords: item.add_marker(no_jsonstats) - - -@pytest.fixture -def named_port(request): - # pylint: disable=unused-argument - port = os.getenv("PORT") - if port is None: - port = 5301 - else: - port = int(port) - - return port diff --git a/bin/tests/system/run.sh.in b/bin/tests/system/run.sh.in index c8467e6b94..532134cee4 100644 --- a/bin/tests/system/run.sh.in +++ b/bin/tests/system/run.sh.in @@ -111,7 +111,7 @@ if [ "${srcdir}" != "${builddir}" ]; then cp -a "${srcdir}/common" "${builddir}" fi # Some tests require additional files to work for out-of-tree test runs. - for file in ckdnsrps.sh digcomp.pl ditch.pl fromhex.pl kasp.sh packet.pl start.pl stop.pl testcrypto.sh; do + for file in ckdnsrps.sh conftest.py digcomp.pl ditch.pl fromhex.pl kasp.sh packet.pl start.pl stop.pl testcrypto.sh; do if [ ! -r "${file}" ]; then cp -a "${srcdir}/${file}" "${builddir}" fi diff --git a/bin/tests/system/shutdown/conftest.py b/bin/tests/system/shutdown/conftest.py index 3c3f9d1165..04741da104 100644 --- a/bin/tests/system/shutdown/conftest.py +++ b/bin/tests/system/shutdown/conftest.py @@ -9,7 +9,6 @@ # See the COPYRIGHT file distributed with this work for additional # information regarding copyright ownership. -import os import pytest @@ -32,27 +31,3 @@ def pytest_collection_modifyitems(config, items): for item in items: if "dnspython" in item.keywords: item.add_marker(skip_dnspython) - - -@pytest.fixture -def named_port(request): - # pylint: disable=unused-argument - port = os.getenv("PORT") - if port is None: - port = 5301 - else: - port = int(port) - - return port - - -@pytest.fixture -def control_port(request): - # pylint: disable=unused-argument - port = os.getenv("CONTROLPORT") - if port is None: - port = 5301 - else: - port = int(port) - - return port diff --git a/bin/tests/system/statschannel/conftest.py b/bin/tests/system/statschannel/conftest.py index 2bc5f116eb..59a903ca2b 100644 --- a/bin/tests/system/statschannel/conftest.py +++ b/bin/tests/system/statschannel/conftest.py @@ -87,21 +87,9 @@ def pytest_collection_modifyitems(config, items): def statsport(request): # pylint: disable=unused-argument env_port = os.getenv("EXTRAPORT1") - if port is None: + if env_port is None: env_port = 5301 else: env_port = int(env_port) return env_port - - -@pytest.fixture -def port(request): - # pylint: disable=unused-argument - env_port = os.getenv("PORT") - if port is None: - env_port = 5300 - else: - env_port = int(env_port) - - return env_port diff --git a/bin/tests/system/statschannel/tests-json.py b/bin/tests/system/statschannel/tests-json.py index a368260b28..6af335e1d2 100755 --- a/bin/tests/system/statschannel/tests-json.py +++ b/bin/tests/system/statschannel/tests-json.py @@ -107,7 +107,7 @@ def test_zone_with_many_keys_json(statsport): @pytest.mark.dnspython @pytest.mark.skipif(os.getenv("HAVEJSONSTATS", "unset") != "1", reason="JSON not configured") -def test_traffic_json(port, statsport): +def test_traffic_json(named_port, statsport): generic.test_traffic(fetch_traffic_json, statsip="10.53.0.2", statsport=statsport, - port=port) + port=named_port) diff --git a/bin/tests/system/statschannel/tests-xml.py b/bin/tests/system/statschannel/tests-xml.py index 912716bfd8..0dd3b6b075 100755 --- a/bin/tests/system/statschannel/tests-xml.py +++ b/bin/tests/system/statschannel/tests-xml.py @@ -137,7 +137,7 @@ def test_zone_with_many_keys_xml(statsport): @pytest.mark.dnspython @pytest.mark.skipif(os.getenv("HAVEXMLSTATS", "unset") != "1", reason="XML not configured") -def test_traffic_xml(port, statsport): +def test_traffic_xml(named_port, statsport): generic.test_traffic(fetch_traffic_xml, statsip="10.53.0.2", statsport=statsport, - port=port) + port=named_port) diff --git a/bin/tests/system/tcp/conftest.py b/bin/tests/system/tcp/conftest.py index 0ce749b3fd..4789c92d90 100644 --- a/bin/tests/system/tcp/conftest.py +++ b/bin/tests/system/tcp/conftest.py @@ -9,7 +9,6 @@ # See the COPYRIGHT file distributed with this work for additional # information regarding copyright ownership. -import os import pytest @@ -45,15 +44,3 @@ def pytest_collection_modifyitems(config, items): for item in items: if "dnspython2" in item.keywords: item.add_marker(skip_dnspython2) - - -@pytest.fixture -def port(request): - # pylint: disable=unused-argument - env_port = os.getenv("PORT") - if port is None: - env_port = 5300 - else: - env_port = int(env_port) - - return env_port diff --git a/bin/tests/system/tcp/tests-tcp.py b/bin/tests/system/tcp/tests-tcp.py index 7ff84f264b..3e6fadf890 100644 --- a/bin/tests/system/tcp/tests-tcp.py +++ b/bin/tests/system/tcp/tests-tcp.py @@ -41,10 +41,10 @@ def create_socket(host, port): @pytest.mark.dnspython @pytest.mark.dnspython2 -def test_tcp_garbage(port): +def test_tcp_garbage(named_port): import dns.query - with create_socket("10.53.0.7", port) as sock: + with create_socket("10.53.0.7", named_port) as sock: msg = create_msg("a.example.", "A") (sbytes, stime) = dns.query.send_tcp(sock, msg, timeout()) @@ -68,11 +68,11 @@ def test_tcp_garbage(port): @pytest.mark.dnspython @pytest.mark.dnspython2 -def test_tcp_garbage_response(port): +def test_tcp_garbage_response(named_port): import dns.query import dns.message - with create_socket("10.53.0.7", port) as sock: + with create_socket("10.53.0.7", named_port) as sock: msg = create_msg("a.example.", "A") (sbytes, stime) = dns.query.send_tcp(sock, msg, timeout()) diff --git a/bin/tests/system/timeouts/conftest.py b/bin/tests/system/timeouts/conftest.py index d92d87f72c..2639774c1e 100644 --- a/bin/tests/system/timeouts/conftest.py +++ b/bin/tests/system/timeouts/conftest.py @@ -55,15 +55,3 @@ def pytest_collection_modifyitems(config, items): for item in items: if "long" in item.keywords: item.add_marker(skip_long_tests) - - -@pytest.fixture -def port(request): - # pylint: disable=unused-argument - env_port = os.getenv("PORT") - if port is None: - env_port = 5300 - else: - env_port = int(env_port) - - return env_port diff --git a/bin/tests/system/timeouts/tests-tcp.py b/bin/tests/system/timeouts/tests-tcp.py index a1c94839ea..8481a3e9b3 100644 --- a/bin/tests/system/timeouts/tests-tcp.py +++ b/bin/tests/system/timeouts/tests-tcp.py @@ -34,14 +34,14 @@ def timeout(): @pytest.mark.dnspython @pytest.mark.dnspython2 -def test_initial_timeout(port): +def test_initial_timeout(named_port): # # The initial timeout is 2.5 seconds, so this should timeout # import dns.query with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: - sock.connect(("10.53.0.1", port)) + sock.connect(("10.53.0.1", named_port)) time.sleep(3) @@ -57,7 +57,7 @@ def test_initial_timeout(port): @pytest.mark.dnspython @pytest.mark.dnspython2 -def test_idle_timeout(port): +def test_idle_timeout(named_port): # # The idle timeout is 5 seconds, so the third message should fail # @@ -65,7 +65,7 @@ def test_idle_timeout(port): msg = create_msg("example.", "A") with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: - sock.connect(("10.53.0.1", port)) + sock.connect(("10.53.0.1", named_port)) time.sleep(1) @@ -89,7 +89,7 @@ def test_idle_timeout(port): @pytest.mark.dnspython @pytest.mark.dnspython2 -def test_keepalive_timeout(port): +def test_keepalive_timeout(named_port): # # Keepalive is 7 seconds, so the third message should succeed. # @@ -100,7 +100,7 @@ def test_keepalive_timeout(port): msg.use_edns(edns=True, payload=4096, options=[kopt]) with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: - sock.connect(("10.53.0.1", port)) + sock.connect(("10.53.0.1", named_port)) time.sleep(1) @@ -120,7 +120,7 @@ def test_keepalive_timeout(port): @pytest.mark.dnspython @pytest.mark.dnspython2 -def test_pipelining_timeout(port): +def test_pipelining_timeout(named_port): # # The pipelining should only timeout after the last message is received # @@ -128,7 +128,7 @@ def test_pipelining_timeout(port): msg = create_msg("example.", "A") with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: - sock.connect(("10.53.0.1", port)) + sock.connect(("10.53.0.1", named_port)) time.sleep(1) @@ -158,7 +158,7 @@ def test_pipelining_timeout(port): @pytest.mark.dnspython @pytest.mark.dnspython2 -def test_long_axfr(port): +def test_long_axfr(named_port): # # The timers should not fire during AXFR, thus the connection should not # close abruptly @@ -168,7 +168,7 @@ def test_long_axfr(port): import dns.rdatatype with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: - sock.connect(("10.53.0.1", port)) + sock.connect(("10.53.0.1", named_port)) name = dns.name.from_text("example.") msg = create_msg("example.", "AXFR") @@ -194,11 +194,11 @@ def test_long_axfr(port): @pytest.mark.dnspython @pytest.mark.dnspython2 -def test_send_timeout(port): +def test_send_timeout(named_port): import dns.query with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: - sock.connect(("10.53.0.1", port)) + sock.connect(("10.53.0.1", named_port)) # Send and receive single large RDATA over TCP msg = create_msg("large.example.", "TXT") @@ -225,13 +225,13 @@ def test_send_timeout(port): @pytest.mark.dnspython @pytest.mark.dnspython2 @pytest.mark.long -def test_max_transfer_idle_out(port): +def test_max_transfer_idle_out(named_port): import dns.query import dns.rdataclass import dns.rdatatype with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: - sock.connect(("10.53.0.1", port)) + sock.connect(("10.53.0.1", named_port)) name = dns.name.from_text("example.") msg = create_msg("example.", "AXFR") @@ -262,13 +262,13 @@ def test_max_transfer_idle_out(port): @pytest.mark.dnspython @pytest.mark.dnspython2 @pytest.mark.long -def test_max_transfer_time_out(port): +def test_max_transfer_time_out(named_port): import dns.query import dns.rdataclass import dns.rdatatype with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: - sock.connect(("10.53.0.1", port)) + sock.connect(("10.53.0.1", named_port)) name = dns.name.from_text("example.") msg = create_msg("example.", "AXFR") From 4e0d576858fafb79f191e85cc810ec4b611b00d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Mon, 14 Mar 2022 08:59:32 +0100 Subject: [PATCH 02/10] Improve test discovery logic in get_ports.sh The find invocation used by the bin/tests/system/get_ports.sh script ("find . -maxdepth 1 -mindepth 1 -type d") assumes the list of directories in bin/tests/system/ remains unchanged throughout the run time of a single system test suite. With pytest in use and the conftest.py file now present in bin/tests/system/, that assumption is no longer true as a __pycache__ directory may be created when the first pytest-based test is started. Since the list of names returned by the above find invocation serves as a fixed-size array of "port range slots", any changes to that list during a system test suite run may lead to port assignment collisions [1]. Fix by making the find invocation more nuanced, so that it only returns names of directories containing test code. Squash a grep / cut pipeline into a single awk invocation. [1] see commit 31e5ca4bd9f50b9e0ad5a88685225afacb073a8a --- bin/tests/system/get_ports.sh | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/bin/tests/system/get_ports.sh b/bin/tests/system/get_ports.sh index b1d4a8169a..b44e3ef6f7 100755 --- a/bin/tests/system/get_ports.sh +++ b/bin/tests/system/get_ports.sh @@ -14,7 +14,11 @@ # This script is a 'port' broker. It keeps track of ports given to the # individual system subtests, so every test is given a unique port range. -total_tests=$(find . -maxdepth 1 -mindepth 1 -type d | wc -l) +get_sorted_test_names() { + find . -maxdepth 2 -mindepth 2 -type f \( -name "tests.sh" -o -name "tests*.py" \) | cut -d/ -f2 | sort -u +} + +total_tests=$(get_sorted_test_names | wc -l) ports_per_test=20 port_min=5001 @@ -33,7 +37,7 @@ while getopts "p:t:-:" OPT; do case "$OPT" in p | port) baseport=$OPTARG ;; t | test) - test_index=$(find . -maxdepth 1 -mindepth 1 -type d | sort | grep -F -x -n "./${OPTARG}" | cut -d: -f1) + test_index=$(get_sorted_test_names | awk "/^${OPTARG}\$/ { print NR }") if [ -z "${test_index}" ]; then echo "Test '${OPTARG}' not found" >&2 exit 1 From 96b7f9f9aa217c93238922388a7f2b177ae43f56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Mon, 14 Mar 2022 08:59:32 +0100 Subject: [PATCH 03/10] Refactor "statschannel" test's helper modules The "statschannel" system test contains two Python helper modules: - generic.py: test functions directly invoked by both tests-json.py and test-xml.py, - helper.py: helper functions invoked by test functions in generic.py. The above logic for splitting helper functions into Python modules prevents selective test skipping from working due to unconditional import statements being present in both helper modules. For example, if dnspython is not available on the test host, tests-json.py imports generic.py, which in turn imports helper.py, which in turn attempts to import various dnspython modules, triggering ImportError exceptions during test initialization. Various decorators used for some tests (like @pytest.mark.dnspython) suggest that such a scenario should be handled gracefully, but that is not the case - modifying the test collection in conftest.py does not prevent pytest from failing due to import errors. Fix by moving helper functions around to achieve a different split: - generic.py: helper functions only relying on the Python standard library, - generic_dnspython.py: helper functions requiring dnspython. Only two tests in tests-{json,xml}.py need dnspython to work (test_traffic_json(), test_traffic_xml()). Since all dnspython-dependent code is now present in generic_dnspython.py, employ pytest.importorskip() in those two tests to ensure they can be selectively skipped when dnspython is not available. Adjust other code to account for the revised Python helper module layout. Remove all occurrences of the @pytest.mark.dnspython decorator (and all associated code) from the "statschannel" system test to prevent confusion. --- bin/tests/system/statschannel/conftest.py | 12 -- bin/tests/system/statschannel/generic.py | 111 ++++++++++-------- .../{helper.py => generic_dnspython.py} | 101 +++++++--------- bin/tests/system/statschannel/tests-json.py | 15 ++- bin/tests/system/statschannel/tests-xml.py | 15 ++- 5 files changed, 119 insertions(+), 135 deletions(-) rename bin/tests/system/statschannel/{helper.py => generic_dnspython.py} (61%) diff --git a/bin/tests/system/statschannel/conftest.py b/bin/tests/system/statschannel/conftest.py index 59a903ca2b..798bee7300 100644 --- a/bin/tests/system/statschannel/conftest.py +++ b/bin/tests/system/statschannel/conftest.py @@ -23,9 +23,6 @@ def pytest_configure(config): config.addinivalue_line( "markers", "xml: mark tests that need xml.etree to function" ) - config.addinivalue_line( - "markers", "dnspython: mark tests that need dnspython to function" - ) def pytest_collection_modifyitems(config, items): @@ -72,15 +69,6 @@ def pytest_collection_modifyitems(config, items): for item in items: if "xml" in item.keywords: item.add_marker(no_xmlstats) - # Test for dnspython module - skip_dnspython = pytest.mark.skip( - reason="need dnspython module to run") - try: - import dns.query # noqa: F401 - except ModuleNotFoundError: - for item in items: - if "dnspython" in item.keywords: - item.add_marker(skip_dnspython) @pytest.fixture diff --git a/bin/tests/system/statschannel/generic.py b/bin/tests/system/statschannel/generic.py index 1c5fda8092..e4cf82cec5 100644 --- a/bin/tests/system/statschannel/generic.py +++ b/bin/tests/system/statschannel/generic.py @@ -9,7 +9,64 @@ # See the COPYRIGHT file distributed with this work for additional # information regarding copyright ownership. -import helper +from datetime import datetime, timedelta +import os + + +# ISO datetime format without msec +fmt = '%Y-%m-%dT%H:%M:%SZ' + +# The constants were taken from BIND 9 source code (lib/dns/zone.c) +max_refresh = timedelta(seconds=2419200) # 4 weeks +max_expires = timedelta(seconds=14515200) # 24 weeks +now = datetime.utcnow().replace(microsecond=0) +dayzero = datetime.utcfromtimestamp(0).replace(microsecond=0) + + +# Generic helper functions +def check_expires(expires, min_time, max_time): + assert expires >= min_time + assert expires <= max_time + + +def check_refresh(refresh, min_time, max_time): + assert refresh >= min_time + assert refresh <= max_time + + +def check_loaded(loaded, expected): + # Sanity check the zone timers values + assert loaded == expected + assert loaded < now + + +def check_zone_timers(loaded, expires, refresh, loaded_exp): + # Sanity checks the zone timers values + if expires is not None: + check_expires(expires, now, now + max_expires) + if refresh is not None: + check_refresh(refresh, now, now + max_refresh) + check_loaded(loaded, loaded_exp) + + +# +# The output is gibberish, but at least make sure it does not crash. +# +def check_manykeys(name, zone=None): + # pylint: disable=unused-argument + assert name == "manykeys" + + +def zone_mtime(zonedir, name): + + try: + si = os.stat(os.path.join(zonedir, "{}.db".format(name))) + except FileNotFoundError: + return dayzero + + mtime = datetime.utcfromtimestamp(si.st_mtime).replace(microsecond=0) + + return mtime def test_zone_timers_primary(fetch_zones, load_timers, **kwargs): @@ -22,8 +79,8 @@ def test_zone_timers_primary(fetch_zones, load_timers, **kwargs): for zone in zones: (name, loaded, expires, refresh) = load_timers(zone, True) - mtime = helper.zone_mtime(zonedir, name) - helper.check_zone_timers(loaded, expires, refresh, mtime) + mtime = zone_mtime(zonedir, name) + check_zone_timers(loaded, expires, refresh, mtime) def test_zone_timers_secondary(fetch_zones, load_timers, **kwargs): @@ -36,8 +93,8 @@ def test_zone_timers_secondary(fetch_zones, load_timers, **kwargs): for zone in zones: (name, loaded, expires, refresh) = load_timers(zone, False) - mtime = helper.zone_mtime(zonedir, name) - helper.check_zone_timers(loaded, expires, refresh, mtime) + mtime = zone_mtime(zonedir, name) + check_zone_timers(loaded, expires, refresh, mtime) def test_zone_with_many_keys(fetch_zones, load_zone, **kwargs): @@ -50,46 +107,4 @@ def test_zone_with_many_keys(fetch_zones, load_zone, **kwargs): for zone in zones: name = load_zone(zone) if name == 'manykeys': - helper.check_manykeys(name) - - -def test_traffic(fetch_traffic, **kwargs): - - statsip = kwargs['statsip'] - statsport = kwargs['statsport'] - port = kwargs['port'] - - data = fetch_traffic(statsip, statsport) - exp = helper.create_expected(data) - - msg = helper.create_msg("short.example.", "TXT") - helper.update_expected(exp, "dns-udp-requests-sizes-received-ipv4", msg) - ans = helper.udp_query(statsip, port, msg) - helper.update_expected(exp, "dns-udp-responses-sizes-sent-ipv4", ans) - data = fetch_traffic(statsip, statsport) - - helper.check_traffic(data, exp) - - msg = helper.create_msg("long.example.", "TXT") - helper.update_expected(exp, "dns-udp-requests-sizes-received-ipv4", msg) - ans = helper.udp_query(statsip, port, msg) - helper.update_expected(exp, "dns-udp-responses-sizes-sent-ipv4", ans) - data = fetch_traffic(statsip, statsport) - - helper.check_traffic(data, exp) - - msg = helper.create_msg("short.example.", "TXT") - helper.update_expected(exp, "dns-tcp-requests-sizes-received-ipv4", msg) - ans = helper.tcp_query(statsip, port, msg) - helper.update_expected(exp, "dns-tcp-responses-sizes-sent-ipv4", ans) - data = fetch_traffic(statsip, statsport) - - helper.check_traffic(data, exp) - - msg = helper.create_msg("long.example.", "TXT") - helper.update_expected(exp, "dns-tcp-requests-sizes-received-ipv4", msg) - ans = helper.tcp_query(statsip, port, msg) - helper.update_expected(exp, "dns-tcp-responses-sizes-sent-ipv4", ans) - data = fetch_traffic(statsip, statsport) - - helper.check_traffic(data, exp) + check_manykeys(name) diff --git a/bin/tests/system/statschannel/helper.py b/bin/tests/system/statschannel/generic_dnspython.py similarity index 61% rename from bin/tests/system/statschannel/helper.py rename to bin/tests/system/statschannel/generic_dnspython.py index 0a44333e14..88dabbca08 100644 --- a/bin/tests/system/statschannel/helper.py +++ b/bin/tests/system/statschannel/generic_dnspython.py @@ -9,75 +9,16 @@ # See the COPYRIGHT file distributed with this work for additional # information regarding copyright ownership. -import os -import os.path - from collections import defaultdict -from datetime import datetime, timedelta import dns.message import dns.query import dns.rcode -# ISO datetime format without msec -fmt = '%Y-%m-%dT%H:%M:%SZ' - -# The constants were taken from BIND 9 source code (lib/dns/zone.c) -max_refresh = timedelta(seconds=2419200) # 4 weeks -max_expires = timedelta(seconds=14515200) # 24 weeks -now = datetime.utcnow().replace(microsecond=0) -dayzero = datetime.utcfromtimestamp(0).replace(microsecond=0) - TIMEOUT = 10 -# Generic helper functions -def check_expires(expires, min_time, max_time): - assert expires >= min_time - assert expires <= max_time - - -def check_refresh(refresh, min_time, max_time): - assert refresh >= min_time - assert refresh <= max_time - - -def check_loaded(loaded, expected): - # Sanity check the zone timers values - assert loaded == expected - assert loaded < now - - -def check_zone_timers(loaded, expires, refresh, loaded_exp): - # Sanity checks the zone timers values - if expires is not None: - check_expires(expires, now, now + max_expires) - if refresh is not None: - check_refresh(refresh, now, now + max_refresh) - check_loaded(loaded, loaded_exp) - - -# -# The output is gibberish, but at least make sure it does not crash. -# -def check_manykeys(name, zone=None): - # pylint: disable=unused-argument - assert name == "manykeys" - - -def zone_mtime(zonedir, name): - - try: - si = os.stat(os.path.join(zonedir, "{}.db".format(name))) - except FileNotFoundError: - return dayzero - - mtime = datetime.utcfromtimestamp(si.st_mtime).replace(microsecond=0) - - return mtime - - def create_msg(qname, qtype): msg = dns.message.make_query(qname, qtype, want_dnssec=True, use_edns=0, payload=4096) @@ -144,3 +85,45 @@ def check_traffic(data, expected): assert len(expected) == len(ordered_expected) assert ordered_data == ordered_expected + + +def test_traffic(fetch_traffic, **kwargs): + + statsip = kwargs['statsip'] + statsport = kwargs['statsport'] + port = kwargs['port'] + + data = fetch_traffic(statsip, statsport) + exp = create_expected(data) + + msg = create_msg("short.example.", "TXT") + update_expected(exp, "dns-udp-requests-sizes-received-ipv4", msg) + ans = udp_query(statsip, port, msg) + update_expected(exp, "dns-udp-responses-sizes-sent-ipv4", ans) + data = fetch_traffic(statsip, statsport) + + check_traffic(data, exp) + + msg = create_msg("long.example.", "TXT") + update_expected(exp, "dns-udp-requests-sizes-received-ipv4", msg) + ans = udp_query(statsip, port, msg) + update_expected(exp, "dns-udp-responses-sizes-sent-ipv4", ans) + data = fetch_traffic(statsip, statsport) + + check_traffic(data, exp) + + msg = create_msg("short.example.", "TXT") + update_expected(exp, "dns-tcp-requests-sizes-received-ipv4", msg) + ans = tcp_query(statsip, port, msg) + update_expected(exp, "dns-tcp-responses-sizes-sent-ipv4", ans) + data = fetch_traffic(statsip, statsport) + + check_traffic(data, exp) + + msg = create_msg("long.example.", "TXT") + update_expected(exp, "dns-tcp-requests-sizes-received-ipv4", msg) + ans = tcp_query(statsip, port, msg) + update_expected(exp, "dns-tcp-responses-sizes-sent-ipv4", ans) + data = fetch_traffic(statsip, statsport) + + check_traffic(data, exp) diff --git a/bin/tests/system/statschannel/tests-json.py b/bin/tests/system/statschannel/tests-json.py index 6af335e1d2..1f4d6d6e65 100755 --- a/bin/tests/system/statschannel/tests-json.py +++ b/bin/tests/system/statschannel/tests-json.py @@ -19,7 +19,6 @@ import pytest import requests import generic -from helper import fmt # JSON helper functions @@ -50,7 +49,7 @@ def load_timers_json(zone, primary=True): # Check if the primary zone timer exists assert 'loaded' in zone - loaded = datetime.strptime(zone['loaded'], fmt) + loaded = datetime.strptime(zone['loaded'], generic.fmt) if primary: # Check if the secondary zone timers does not exist @@ -61,8 +60,8 @@ def load_timers_json(zone, primary=True): else: assert 'expires' in zone assert 'refresh' in zone - expires = datetime.strptime(zone['expires'], fmt) - refresh = datetime.strptime(zone['refresh'], fmt) + expires = datetime.strptime(zone['expires'], generic.fmt) + refresh = datetime.strptime(zone['refresh'], generic.fmt) return (name, loaded, expires, refresh) @@ -104,10 +103,10 @@ def test_zone_with_many_keys_json(statsport): @pytest.mark.json @pytest.mark.requests -@pytest.mark.dnspython @pytest.mark.skipif(os.getenv("HAVEJSONSTATS", "unset") != "1", reason="JSON not configured") def test_traffic_json(named_port, statsport): - generic.test_traffic(fetch_traffic_json, - statsip="10.53.0.2", statsport=statsport, - port=named_port) + generic_dnspython = pytest.importorskip('generic_dnspython') + generic_dnspython.test_traffic(fetch_traffic_json, + statsip="10.53.0.2", statsport=statsport, + port=named_port) diff --git a/bin/tests/system/statschannel/tests-xml.py b/bin/tests/system/statschannel/tests-xml.py index 0dd3b6b075..efd66a5b68 100755 --- a/bin/tests/system/statschannel/tests-xml.py +++ b/bin/tests/system/statschannel/tests-xml.py @@ -20,7 +20,6 @@ import pytest import requests import generic -from helper import fmt # XML helper functions @@ -79,7 +78,7 @@ def load_timers_xml(zone, primary=True): loaded_el = zone.find('loaded') assert loaded_el is not None - loaded = datetime.strptime(loaded_el.text, fmt) + loaded = datetime.strptime(loaded_el.text, generic.fmt) expires_el = zone.find('expires') refresh_el = zone.find('refresh') @@ -91,8 +90,8 @@ def load_timers_xml(zone, primary=True): else: assert expires_el is not None assert refresh_el is not None - expires = datetime.strptime(expires_el.text, fmt) - refresh = datetime.strptime(refresh_el.text, fmt) + expires = datetime.strptime(expires_el.text, generic.fmt) + refresh = datetime.strptime(refresh_el.text, generic.fmt) return (name, loaded, expires, refresh) @@ -134,10 +133,10 @@ def test_zone_with_many_keys_xml(statsport): @pytest.mark.xml @pytest.mark.requests -@pytest.mark.dnspython @pytest.mark.skipif(os.getenv("HAVEXMLSTATS", "unset") != "1", reason="XML not configured") def test_traffic_xml(named_port, statsport): - generic.test_traffic(fetch_traffic_xml, - statsip="10.53.0.2", statsport=statsport, - port=named_port) + generic_dnspython = pytest.importorskip('generic_dnspython') + generic_dnspython.test_traffic(fetch_traffic_xml, + statsip="10.53.0.2", statsport=statsport, + port=named_port) From 0a76f186a5ad4bb22e27c36f2a29ddb9e5398ae7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Mon, 14 Mar 2022 08:59:32 +0100 Subject: [PATCH 04/10] Simplify skipping tests depending on json-c All tests in bin/tests/system/statschannel/tests-json.py require json-c support to be enabled in BIND 9 at build-time. Instead of applying the same pytest.mark.skipif() decorator to every test in that file, set the 'pytestmark' global accordingly in order to immediately skip all tests in tests-json.py if json-c support is not compiled in. Remove all occurrences of the @pytest.mark.json decorator (and all associated code) from the "statschannel" system test as the json module is a part of the Python standard library since Python 2.6 (so checking whether it is available is redundant) and checking for json-c support in the tested BIND 9 build is already handled by setting the 'pytestmark' global accordingly. Also remove a related excerpt from bin/tests/system/rpzextra/conftest.py as it is a copy-paste artifact that serves no purpose in the "rpzextra" system test. --- bin/tests/system/rpzextra/conftest.py | 7 ------- bin/tests/system/statschannel/conftest.py | 19 ------------------- bin/tests/system/statschannel/tests-json.py | 15 +++------------ 3 files changed, 3 insertions(+), 38 deletions(-) diff --git a/bin/tests/system/rpzextra/conftest.py b/bin/tests/system/rpzextra/conftest.py index 8bbcf25ad7..aa2034fcb0 100644 --- a/bin/tests/system/rpzextra/conftest.py +++ b/bin/tests/system/rpzextra/conftest.py @@ -9,7 +9,6 @@ # See the COPYRIGHT file distributed with this work for additional # information regarding copyright ownership. -import os import pytest try: @@ -34,9 +33,3 @@ def pytest_collection_modifyitems(config, items): for item in items: if "dnspython" in item.keywords: item.add_marker(skip_requests) - # Test if JSON statistics channel was enabled - no_jsonstats = pytest.mark.skip(reason="need JSON statistics to be enabled") - if os.getenv("HAVEJSONSTATS") is None: - for item in items: - if "json" in item.keywords: - item.add_marker(no_jsonstats) diff --git a/bin/tests/system/statschannel/conftest.py b/bin/tests/system/statschannel/conftest.py index 798bee7300..0a766781fa 100644 --- a/bin/tests/system/statschannel/conftest.py +++ b/bin/tests/system/statschannel/conftest.py @@ -17,9 +17,6 @@ def pytest_configure(config): config.addinivalue_line( "markers", "requests: mark tests that need requests to function" ) - config.addinivalue_line( - "markers", "json: mark tests that need json to function" - ) config.addinivalue_line( "markers", "xml: mark tests that need xml.etree to function" ) @@ -37,15 +34,6 @@ def pytest_collection_modifyitems(config, items): for item in items: if "requests" in item.keywords: item.add_marker(skip_requests) - # Test for json module - skip_json = pytest.mark.skip( - reason="need json module to run") - try: - import json # noqa: F401 - except ModuleNotFoundError: - for item in items: - if "json" in item.keywords: - item.add_marker(skip_json) # Test for xml module skip_xml = pytest.mark.skip( reason="need xml module to run") @@ -55,13 +43,6 @@ def pytest_collection_modifyitems(config, items): for item in items: if "xml" in item.keywords: item.add_marker(skip_xml) - # Test if JSON statistics channel was enabled - no_jsonstats = pytest.mark.skip( - reason="need JSON statistics to be enabled") - if os.getenv("HAVEJSONSTATS") is None: - for item in items: - if "json" in item.keywords: - item.add_marker(no_jsonstats) # Test if XML statistics channel was enabled no_xmlstats = pytest.mark.skip( reason="need XML statistics to be enabled") diff --git a/bin/tests/system/statschannel/tests-json.py b/bin/tests/system/statschannel/tests-json.py index 1f4d6d6e65..84337a7a8b 100755 --- a/bin/tests/system/statschannel/tests-json.py +++ b/bin/tests/system/statschannel/tests-json.py @@ -20,6 +20,9 @@ import requests import generic +pytestmark = pytest.mark.skipif(not os.environ.get('HAVEJSONSTATS'), + reason='json-c support disabled in the build') + # JSON helper functions def fetch_zones_json(statsip, statsport): @@ -72,39 +75,27 @@ def load_zone_json(zone): return name -@pytest.mark.json @pytest.mark.requests -@pytest.mark.skipif(os.getenv("HAVEJSONSTATS", "unset") != "1", - reason="JSON not configured") def test_zone_timers_primary_json(statsport): generic.test_zone_timers_primary(fetch_zones_json, load_timers_json, statsip="10.53.0.1", statsport=statsport, zonedir="ns1") -@pytest.mark.json @pytest.mark.requests -@pytest.mark.skipif(os.getenv("HAVEJSONSTATS", "unset") != "1", - reason="JSON not configured") def test_zone_timers_secondary_json(statsport): generic.test_zone_timers_secondary(fetch_zones_json, load_timers_json, statsip="10.53.0.3", statsport=statsport, zonedir="ns3") -@pytest.mark.json @pytest.mark.requests -@pytest.mark.skipif(os.getenv("HAVEJSONSTATS", "unset") != "1", - reason="JSON not configured") def test_zone_with_many_keys_json(statsport): generic.test_zone_with_many_keys(fetch_zones_json, load_zone_json, statsip="10.53.0.2", statsport=statsport) -@pytest.mark.json @pytest.mark.requests -@pytest.mark.skipif(os.getenv("HAVEJSONSTATS", "unset") != "1", - reason="JSON not configured") def test_traffic_json(named_port, statsport): generic_dnspython = pytest.importorskip('generic_dnspython') generic_dnspython.test_traffic(fetch_traffic_json, From 286b57c7f165c441170bc3aa6f7faa097161b7ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Mon, 14 Mar 2022 08:59:32 +0100 Subject: [PATCH 05/10] Simplify skipping tests depending on libxml2 All tests in bin/tests/system/statschannel/tests-xml.py require libxml2 support to be enabled in BIND 9 at build-time. Instead of applying the same pytest.mark.skipif() decorator to every test in that file, set the 'pytestmark' global accordingly in order to immediately skip all tests in tests-xml.py if libxml2 support is not compiled in. Remove all occurrences of the @pytest.mark.xml decorator (and all associated code) from the "statschannel" system test as the xml.etree.ElementTree module is a part of the Python standard library since Python 2.5 (so checking whether it is available is redundant) and checking for libxml2 support in the tested BIND 9 build is already handled by setting the 'pytestmark' global accordingly. --- bin/tests/system/statschannel/conftest.py | 19 ------------------- bin/tests/system/statschannel/tests-xml.py | 15 +++------------ 2 files changed, 3 insertions(+), 31 deletions(-) diff --git a/bin/tests/system/statschannel/conftest.py b/bin/tests/system/statschannel/conftest.py index 0a766781fa..f876f5d56e 100644 --- a/bin/tests/system/statschannel/conftest.py +++ b/bin/tests/system/statschannel/conftest.py @@ -17,9 +17,6 @@ def pytest_configure(config): config.addinivalue_line( "markers", "requests: mark tests that need requests to function" ) - config.addinivalue_line( - "markers", "xml: mark tests that need xml.etree to function" - ) def pytest_collection_modifyitems(config, items): @@ -34,22 +31,6 @@ def pytest_collection_modifyitems(config, items): for item in items: if "requests" in item.keywords: item.add_marker(skip_requests) - # Test for xml module - skip_xml = pytest.mark.skip( - reason="need xml module to run") - try: - import xml.etree.ElementTree # noqa: F401 - except ModuleNotFoundError: - for item in items: - if "xml" in item.keywords: - item.add_marker(skip_xml) - # Test if XML statistics channel was enabled - no_xmlstats = pytest.mark.skip( - reason="need XML statistics to be enabled") - if os.getenv("HAVEXMLSTATS") is None: - for item in items: - if "xml" in item.keywords: - item.add_marker(no_xmlstats) @pytest.fixture diff --git a/bin/tests/system/statschannel/tests-xml.py b/bin/tests/system/statschannel/tests-xml.py index efd66a5b68..75edc1a8c4 100755 --- a/bin/tests/system/statschannel/tests-xml.py +++ b/bin/tests/system/statschannel/tests-xml.py @@ -21,6 +21,9 @@ import requests import generic +pytestmark = pytest.mark.skipif(not os.environ.get('HAVEXMLSTATS'), + reason='libxml2 support disabled in the build') + # XML helper functions def fetch_zones_xml(statsip, statsport): @@ -102,39 +105,27 @@ def load_zone_xml(zone): return name -@pytest.mark.xml @pytest.mark.requests -@pytest.mark.skipif(os.getenv("HAVEXMLSTATS", "unset") != "1", - reason="XML not configured") def test_zone_timers_primary_xml(statsport): generic.test_zone_timers_primary(fetch_zones_xml, load_timers_xml, statsip="10.53.0.1", statsport=statsport, zonedir="ns1") -@pytest.mark.xml @pytest.mark.requests -@pytest.mark.skipif(os.getenv("HAVEXMLSTATS", "unset") != "1", - reason="XML not configured") def test_zone_timers_secondary_xml(statsport): generic.test_zone_timers_secondary(fetch_zones_xml, load_timers_xml, statsip="10.53.0.3", statsport=statsport, zonedir="ns3") -@pytest.mark.xml @pytest.mark.requests -@pytest.mark.skipif(os.getenv("HAVEXMLSTATS", "unset") != "1", - reason="XML not configured") def test_zone_with_many_keys_xml(statsport): generic.test_zone_with_many_keys(fetch_zones_xml, load_zone_xml, statsip="10.53.0.2", statsport=statsport) -@pytest.mark.xml @pytest.mark.requests -@pytest.mark.skipif(os.getenv("HAVEXMLSTATS", "unset") != "1", - reason="XML not configured") def test_traffic_xml(named_port, statsport): generic_dnspython = pytest.importorskip('generic_dnspython') generic_dnspython.test_traffic(fetch_traffic_xml, From 704ad2907ff84e84de2f0cfab33ff6ea3d2ea8e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Mon, 14 Mar 2022 08:59:32 +0100 Subject: [PATCH 06/10] Fix skipping tests requiring the requests module The intended purpose of the @pytest.mark.requests decorator was to cause Python-based parts of the "statschannel" system test to be skipped if the requests Python module is not available. However, both tests-json.py and tests-xml.py contain a global "import requests" statement which triggers ImportError exceptions during test initialization if the requests module is not available. In other words, the @pytest.mark.requests decorator serves no useful purpose. Since all tests in both tests-json.py and tests-xml.py depend on the requests Python module, employ pytest.importorskip() to ensure the Python-based parts of the "statschannel" system test are skipped when the requests module is not available. Remove all occurrences of the @pytest.mark.requests decorator (and all associated code) to prevent confusion. --- bin/tests/system/statschannel/conftest.py | 20 -------------------- bin/tests/system/statschannel/tests-json.py | 6 +----- bin/tests/system/statschannel/tests-xml.py | 6 +----- 3 files changed, 2 insertions(+), 30 deletions(-) diff --git a/bin/tests/system/statschannel/conftest.py b/bin/tests/system/statschannel/conftest.py index f876f5d56e..363dd7ad1b 100644 --- a/bin/tests/system/statschannel/conftest.py +++ b/bin/tests/system/statschannel/conftest.py @@ -13,26 +13,6 @@ import os import pytest -def pytest_configure(config): - config.addinivalue_line( - "markers", "requests: mark tests that need requests to function" - ) - - -def pytest_collection_modifyitems(config, items): - # pylint: disable=unused-argument,unused-import,too-many-branches - # pylint: disable=import-outside-toplevel - # Test for requests module - skip_requests = pytest.mark.skip( - reason="need requests module to run") - try: - import requests # noqa: F401 - except ModuleNotFoundError: - for item in items: - if "requests" in item.keywords: - item.add_marker(skip_requests) - - @pytest.fixture def statsport(request): # pylint: disable=unused-argument diff --git a/bin/tests/system/statschannel/tests-json.py b/bin/tests/system/statschannel/tests-json.py index 84337a7a8b..373f98d30b 100755 --- a/bin/tests/system/statschannel/tests-json.py +++ b/bin/tests/system/statschannel/tests-json.py @@ -16,12 +16,12 @@ from datetime import datetime import os import pytest -import requests import generic pytestmark = pytest.mark.skipif(not os.environ.get('HAVEJSONSTATS'), reason='json-c support disabled in the build') +requests = pytest.importorskip('requests') # JSON helper functions @@ -75,27 +75,23 @@ def load_zone_json(zone): return name -@pytest.mark.requests def test_zone_timers_primary_json(statsport): generic.test_zone_timers_primary(fetch_zones_json, load_timers_json, statsip="10.53.0.1", statsport=statsport, zonedir="ns1") -@pytest.mark.requests def test_zone_timers_secondary_json(statsport): generic.test_zone_timers_secondary(fetch_zones_json, load_timers_json, statsip="10.53.0.3", statsport=statsport, zonedir="ns3") -@pytest.mark.requests def test_zone_with_many_keys_json(statsport): generic.test_zone_with_many_keys(fetch_zones_json, load_zone_json, statsip="10.53.0.2", statsport=statsport) -@pytest.mark.requests def test_traffic_json(named_port, statsport): generic_dnspython = pytest.importorskip('generic_dnspython') generic_dnspython.test_traffic(fetch_traffic_json, diff --git a/bin/tests/system/statschannel/tests-xml.py b/bin/tests/system/statschannel/tests-xml.py index 75edc1a8c4..73b6e909f3 100755 --- a/bin/tests/system/statschannel/tests-xml.py +++ b/bin/tests/system/statschannel/tests-xml.py @@ -17,12 +17,12 @@ from datetime import datetime import os import pytest -import requests import generic pytestmark = pytest.mark.skipif(not os.environ.get('HAVEXMLSTATS'), reason='libxml2 support disabled in the build') +requests = pytest.importorskip('requests') # XML helper functions @@ -105,27 +105,23 @@ def load_zone_xml(zone): return name -@pytest.mark.requests def test_zone_timers_primary_xml(statsport): generic.test_zone_timers_primary(fetch_zones_xml, load_timers_xml, statsip="10.53.0.1", statsport=statsport, zonedir="ns1") -@pytest.mark.requests def test_zone_timers_secondary_xml(statsport): generic.test_zone_timers_secondary(fetch_zones_xml, load_timers_xml, statsip="10.53.0.3", statsport=statsport, zonedir="ns3") -@pytest.mark.requests def test_zone_with_many_keys_xml(statsport): generic.test_zone_with_many_keys(fetch_zones_xml, load_zone_xml, statsip="10.53.0.2", statsport=statsport) -@pytest.mark.requests def test_traffic_xml(named_port, statsport): generic_dnspython = pytest.importorskip('generic_dnspython') generic_dnspython.test_traffic(fetch_traffic_xml, From 05c97f2329309506060e29c2935739aed6a815dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Mon, 14 Mar 2022 08:59:32 +0100 Subject: [PATCH 07/10] Fix skipping tests requiring dnspython The intended purpose of the @pytest.mark.dnspython{,2} decorators was to cause dnspython-based tests to be skipped if dnspython is not available (or not recent enough). However, a number of system tests employing those decorators contain global "import dns.resolver" statements which trigger ImportError exceptions during test initialization if dnspython is not available. In other words, the @pytest.mark.dnspython{,2} decorators serve no useful purpose. Currently, whenever a Python-based test requires dnspython, that requirement applies to all tests in a given *.py file. Given that, employ global pytest.importorskip() calls to ensure dnspython-based parts of various system tests are skipped when dnspython is not available. Remove all occurrences of the @pytest.mark.dnspython{,2} decorators (and all associated code) to prevent confusion. --- bin/tests/system/checkds/conftest.py | 46 ------------------- bin/tests/system/checkds/tests-checkds.py | 6 +-- bin/tests/system/rpzextra/conftest.py | 35 -------------- .../rpzextra/tests-rpz-passthru-logging.py | 4 +- bin/tests/system/shutdown/conftest.py | 33 ------------- bin/tests/system/shutdown/tests-shutdown.py | 3 +- bin/tests/system/tcp/conftest.py | 46 ------------------- bin/tests/system/tcp/tests-tcp.py | 7 ++- bin/tests/system/timeouts/conftest.py | 27 ----------- bin/tests/system/timeouts/tests-tcp.py | 19 ++------ 10 files changed, 13 insertions(+), 213 deletions(-) delete mode 100644 bin/tests/system/checkds/conftest.py delete mode 100644 bin/tests/system/rpzextra/conftest.py delete mode 100644 bin/tests/system/shutdown/conftest.py delete mode 100644 bin/tests/system/tcp/conftest.py diff --git a/bin/tests/system/checkds/conftest.py b/bin/tests/system/checkds/conftest.py deleted file mode 100644 index 5328ae0177..0000000000 --- a/bin/tests/system/checkds/conftest.py +++ /dev/null @@ -1,46 +0,0 @@ -# Copyright (C) Internet Systems Consortium, Inc. ("ISC") -# -# SPDX-License-Identifier: MPL-2.0 -# -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, you can obtain one at https://mozilla.org/MPL/2.0/. -# -# See the COPYRIGHT file distributed with this work for additional -# information regarding copyright ownership. - -import pytest - - -def pytest_configure(config): - config.addinivalue_line( - "markers", "dnspython: mark tests that need dnspython to function" - ) - config.addinivalue_line( - "markers", "dnspython2: mark tests that need dnspython >= 2.0.0" - ) - - -def pytest_collection_modifyitems(config, items): - # pylint: disable=unused-argument,unused-import,too-many-branches - # pylint: disable=import-outside-toplevel - - # Test for dnspython module - skip_dnspython = pytest.mark.skip( - reason="need dnspython module to run") - try: - import dns.query # noqa: F401 - except ModuleNotFoundError: - for item in items: - if "dnspython" in item.keywords: - item.add_marker(skip_dnspython) - - # Test for dnspython >= 2.0.0 module - skip_dnspython2 = pytest.mark.skip( - reason="need dnspython >= 2.0.0 module to run") - try: - from dns.query import udp_with_fallback # noqa: F401 - except ImportError: - for item in items: - if "dnspython2" in item.keywords: - item.add_marker(skip_dnspython2) diff --git a/bin/tests/system/checkds/tests-checkds.py b/bin/tests/system/checkds/tests-checkds.py index a774d43345..40611e8d05 100755 --- a/bin/tests/system/checkds/tests-checkds.py +++ b/bin/tests/system/checkds/tests-checkds.py @@ -20,6 +20,8 @@ import time import dns.resolver import pytest +pytest.importorskip('dns', minversion='2.0.0') + def has_signed_apex_nsec(zone, response): has_nsec = False @@ -220,8 +222,6 @@ def wait_for_log(filename, log): assert found -@pytest.mark.dnspython -@pytest.mark.dnspython2 def test_checkds_dspublished(named_port): # We create resolver instances that will be used to send queries. server = dns.resolver.Resolver() @@ -304,8 +304,6 @@ def test_checkds_dspublished(named_port): # TBD: Check with TLS -@pytest.mark.dnspython -@pytest.mark.dnspython2 def test_checkds_dswithdrawn(named_port): # We create resolver instances that will be used to send queries. server = dns.resolver.Resolver() diff --git a/bin/tests/system/rpzextra/conftest.py b/bin/tests/system/rpzextra/conftest.py deleted file mode 100644 index aa2034fcb0..0000000000 --- a/bin/tests/system/rpzextra/conftest.py +++ /dev/null @@ -1,35 +0,0 @@ -# Copyright (C) Internet Systems Consortium, Inc. ("ISC") -# -# SPDX-License-Identifier: MPL-2.0 -# -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, you can obtain one at https://mozilla.org/MPL/2.0/. -# -# See the COPYRIGHT file distributed with this work for additional -# information regarding copyright ownership. - -import pytest - -try: - import dns.resolver # noqa: F401 # pylint: disable=unused-import -except ModuleNotFoundError: - dns_resolver_module_found = False -else: - dns_resolver_module_found = True - - -def pytest_configure(config): - config.addinivalue_line( - "markers", "dnspython: mark tests that need dnspython to function" - ) - - -def pytest_collection_modifyitems(config, items): - # pylint: disable=unused-argument - # Test for dnspython module - if not dns_resolver_module_found: - skip_requests = pytest.mark.skip(reason="need dnspython module to run") - for item in items: - if "dnspython" in item.keywords: - item.add_marker(skip_requests) diff --git a/bin/tests/system/rpzextra/tests-rpz-passthru-logging.py b/bin/tests/system/rpzextra/tests-rpz-passthru-logging.py index 0bd75ebe4d..4785babfba 100755 --- a/bin/tests/system/rpzextra/tests-rpz-passthru-logging.py +++ b/bin/tests/system/rpzextra/tests-rpz-passthru-logging.py @@ -12,11 +12,13 @@ # information regarding copyright ownership. import os + import pytest + +pytest.importorskip('dns') import dns.resolver -# @pytest.mark.dnspython def test_rpz_passthru_logging(named_port): resolver = dns.resolver.Resolver() resolver.nameservers = ['10.53.0.1'] diff --git a/bin/tests/system/shutdown/conftest.py b/bin/tests/system/shutdown/conftest.py deleted file mode 100644 index 04741da104..0000000000 --- a/bin/tests/system/shutdown/conftest.py +++ /dev/null @@ -1,33 +0,0 @@ -# Copyright (C) Internet Systems Consortium, Inc. ("ISC") -# -# SPDX-License-Identifier: MPL-2.0 -# -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, you can obtain one at https://mozilla.org/MPL/2.0/. -# -# See the COPYRIGHT file distributed with this work for additional -# information regarding copyright ownership. - -import pytest - - -def pytest_configure(config): - config.addinivalue_line( - "markers", "dnspython: mark tests that need dnspython to function" - ) - - -def pytest_collection_modifyitems(config, items): - # pylint: disable=unused-argument,unused-import,too-many-branches - # pylint: disable=import-outside-toplevel - - # Test for dnspython module - skip_dnspython = pytest.mark.skip( - reason="need dnspython module to run") - try: - import dns.query # noqa: F401 - except ModuleNotFoundError: - for item in items: - if "dnspython" in item.keywords: - item.add_marker(skip_dnspython) diff --git a/bin/tests/system/shutdown/tests-shutdown.py b/bin/tests/system/shutdown/tests-shutdown.py index d8c2b1f662..9ed222bb1a 100755 --- a/bin/tests/system/shutdown/tests-shutdown.py +++ b/bin/tests/system/shutdown/tests-shutdown.py @@ -22,6 +22,8 @@ import time import dns.resolver import pytest +pytest.importorskip('dns') + def do_work(named_proc, resolver, rndc_cmd, kill_method, n_workers, n_queries): """Creates a number of A queries to run in parallel @@ -126,7 +128,6 @@ def do_work(named_proc, resolver, rndc_cmd, kill_method, n_workers, n_queries): assert ret_code == 0 -@pytest.mark.dnspython def test_named_shutdown(named_port, control_port): # pylint: disable-msg=too-many-locals cfg_dir = os.path.join(os.getcwd(), "resolver") diff --git a/bin/tests/system/tcp/conftest.py b/bin/tests/system/tcp/conftest.py deleted file mode 100644 index 4789c92d90..0000000000 --- a/bin/tests/system/tcp/conftest.py +++ /dev/null @@ -1,46 +0,0 @@ -# Copyright (C) Internet Systems Consortium, Inc. ("ISC") -# -# SPDX-License-Identifier: MPL-2.0 -# -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, you can obtain one at https://mozilla.org/MPL/2.0/. -# -# See the COPYRIGHT file distributed with this work for additional -# information regarding copyright ownership. - -import pytest - - -def pytest_configure(config): - config.addinivalue_line( - "markers", "dnspython: mark tests that need dnspython to function" - ) - config.addinivalue_line( - "markers", "dnspython2: mark tests that need dnspython >= 2.0.0" - ) - - -def pytest_collection_modifyitems(config, items): - # pylint: disable=unused-argument,unused-import,too-many-branches - # pylint: disable=import-outside-toplevel - - # Test for dnspython module - skip_dnspython = pytest.mark.skip( - reason="need dnspython module to run") - try: - import dns.query # noqa: F401 - except ModuleNotFoundError: - for item in items: - if "dnspython" in item.keywords: - item.add_marker(skip_dnspython) - - # Test for dnspython >= 2.0.0 module - skip_dnspython2 = pytest.mark.skip( - reason="need dnspython >= 2.0.0 module to run") - try: - from dns.query import send_tcp # noqa: F401 - except ImportError: - for item in items: - if "dnspython2" in item.keywords: - item.add_marker(skip_dnspython2) diff --git a/bin/tests/system/tcp/tests-tcp.py b/bin/tests/system/tcp/tests-tcp.py index 3e6fadf890..7a12dc2be2 100644 --- a/bin/tests/system/tcp/tests-tcp.py +++ b/bin/tests/system/tcp/tests-tcp.py @@ -19,6 +19,9 @@ import time import pytest +pytest.importorskip('dns', minversion='2.0.0') + + TIMEOUT = 10 @@ -39,8 +42,6 @@ def create_socket(host, port): return sock -@pytest.mark.dnspython -@pytest.mark.dnspython2 def test_tcp_garbage(named_port): import dns.query @@ -66,8 +67,6 @@ def test_tcp_garbage(named_port): raise EOFError from e -@pytest.mark.dnspython -@pytest.mark.dnspython2 def test_tcp_garbage_response(named_port): import dns.query import dns.message diff --git a/bin/tests/system/timeouts/conftest.py b/bin/tests/system/timeouts/conftest.py index 2639774c1e..3a4375232d 100644 --- a/bin/tests/system/timeouts/conftest.py +++ b/bin/tests/system/timeouts/conftest.py @@ -14,12 +14,6 @@ import pytest def pytest_configure(config): - config.addinivalue_line( - "markers", "dnspython: mark tests that need dnspython to function" - ) - config.addinivalue_line( - "markers", "dnspython2: mark tests that need dnspython >= 2.0.0" - ) config.addinivalue_line( "markers", "long: mark tests that take a long time to run" ) @@ -28,27 +22,6 @@ def pytest_configure(config): def pytest_collection_modifyitems(config, items): # pylint: disable=unused-argument,unused-import,too-many-branches # pylint: disable=import-outside-toplevel - - # Test for dnspython module - skip_dnspython = pytest.mark.skip( - reason="need dnspython module to run") - try: - import dns.query # noqa: F401 - except ModuleNotFoundError: - for item in items: - if "dnspython" in item.keywords: - item.add_marker(skip_dnspython) - - # Test for dnspython >= 2.0.0 module - skip_dnspython2 = pytest.mark.skip( - reason="need dnspython >= 2.0.0 module to run") - try: - from dns.query import send_tcp # noqa: F401 - except ImportError: - for item in items: - if "dnspython2" in item.keywords: - item.add_marker(skip_dnspython2) - skip_long_tests = pytest.mark.skip( reason="need CI_ENABLE_ALL_TESTS environment variable") if not os.environ.get("CI_ENABLE_ALL_TESTS"): diff --git a/bin/tests/system/timeouts/tests-tcp.py b/bin/tests/system/timeouts/tests-tcp.py index 8481a3e9b3..a17380af58 100644 --- a/bin/tests/system/timeouts/tests-tcp.py +++ b/bin/tests/system/timeouts/tests-tcp.py @@ -18,6 +18,9 @@ import time import pytest +pytest.importorskip('dns', minversion='2.0.0') + + TIMEOUT = 10 @@ -32,8 +35,6 @@ def timeout(): return time.time() + TIMEOUT -@pytest.mark.dnspython -@pytest.mark.dnspython2 def test_initial_timeout(named_port): # # The initial timeout is 2.5 seconds, so this should timeout @@ -55,8 +56,6 @@ def test_initial_timeout(named_port): raise EOFError from e -@pytest.mark.dnspython -@pytest.mark.dnspython2 def test_idle_timeout(named_port): # # The idle timeout is 5 seconds, so the third message should fail @@ -87,8 +86,6 @@ def test_idle_timeout(named_port): raise EOFError from e -@pytest.mark.dnspython -@pytest.mark.dnspython2 def test_keepalive_timeout(named_port): # # Keepalive is 7 seconds, so the third message should succeed. @@ -118,8 +115,6 @@ def test_keepalive_timeout(named_port): (response, rtime) = dns.query.receive_tcp(sock, timeout()) -@pytest.mark.dnspython -@pytest.mark.dnspython2 def test_pipelining_timeout(named_port): # # The pipelining should only timeout after the last message is received @@ -156,8 +151,6 @@ def test_pipelining_timeout(named_port): raise EOFError from e -@pytest.mark.dnspython -@pytest.mark.dnspython2 def test_long_axfr(named_port): # # The timers should not fire during AXFR, thus the connection should not @@ -192,8 +185,6 @@ def test_long_axfr(named_port): assert soa is not None -@pytest.mark.dnspython -@pytest.mark.dnspython2 def test_send_timeout(named_port): import dns.query @@ -222,8 +213,6 @@ def test_send_timeout(named_port): raise EOFError from e -@pytest.mark.dnspython -@pytest.mark.dnspython2 @pytest.mark.long def test_max_transfer_idle_out(named_port): import dns.query @@ -259,8 +248,6 @@ def test_max_transfer_idle_out(named_port): assert soa is None -@pytest.mark.dnspython -@pytest.mark.dnspython2 @pytest.mark.long def test_max_transfer_time_out(named_port): import dns.query From 49312d6bb2e4d4c7fe513e6832f0521394bb37f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Mon, 14 Mar 2022 08:59:32 +0100 Subject: [PATCH 08/10] Rework imports in dnspython-based system tests Ensure all "import dns.*" statements are always placed after pytest.importorskip('dns') calls, in order to allow the latter to fulfill their purpose. Explicitly import all dnspython modules used by each dnspython-based test to avoid relying on nested imports. Replace function-scoped imports with global imports to reduce code duplication. --- bin/tests/system/checkds/tests-checkds.py | 9 ++++++- bin/tests/system/doth/tests_gnutls.py | 2 ++ bin/tests/system/shutdown/tests-shutdown.py | 3 ++- bin/tests/system/tcp/tests-tcp.py | 8 ++---- bin/tests/system/timeouts/tests-tcp.py | 29 +++++---------------- bin/tests/system/wildcard/tests-wildcard.py | 2 ++ 6 files changed, 22 insertions(+), 31 deletions(-) diff --git a/bin/tests/system/checkds/tests-checkds.py b/bin/tests/system/checkds/tests-checkds.py index 40611e8d05..68b197c73e 100755 --- a/bin/tests/system/checkds/tests-checkds.py +++ b/bin/tests/system/checkds/tests-checkds.py @@ -17,10 +17,17 @@ import subprocess import sys import time -import dns.resolver import pytest pytest.importorskip('dns', minversion='2.0.0') +import dns.exception +import dns.message +import dns.name +import dns.query +import dns.rcode +import dns.rdataclass +import dns.rdatatype +import dns.resolver def has_signed_apex_nsec(zone, response): diff --git a/bin/tests/system/doth/tests_gnutls.py b/bin/tests/system/doth/tests_gnutls.py index 0e7879e04b..ea86a961b2 100644 --- a/bin/tests/system/doth/tests_gnutls.py +++ b/bin/tests/system/doth/tests_gnutls.py @@ -21,6 +21,8 @@ import pytest pytest.importorskip('dns') import dns.exception import dns.message +import dns.name +import dns.rdataclass import dns.rdatatype diff --git a/bin/tests/system/shutdown/tests-shutdown.py b/bin/tests/system/shutdown/tests-shutdown.py index 9ed222bb1a..b8de379d66 100755 --- a/bin/tests/system/shutdown/tests-shutdown.py +++ b/bin/tests/system/shutdown/tests-shutdown.py @@ -19,10 +19,11 @@ import subprocess from string import ascii_lowercase as letters import time -import dns.resolver import pytest pytest.importorskip('dns') +import dns.exception +import dns.resolver def do_work(named_proc, resolver, rndc_cmd, kill_method, n_workers, n_queries): diff --git a/bin/tests/system/tcp/tests-tcp.py b/bin/tests/system/tcp/tests-tcp.py index 7a12dc2be2..da70b1ef38 100644 --- a/bin/tests/system/tcp/tests-tcp.py +++ b/bin/tests/system/tcp/tests-tcp.py @@ -20,13 +20,14 @@ import time import pytest pytest.importorskip('dns', minversion='2.0.0') +import dns.message +import dns.query TIMEOUT = 10 def create_msg(qname, qtype): - import dns.message msg = dns.message.make_query(qname, qtype, want_dnssec=True, use_edns=0, payload=4096) return msg @@ -43,8 +44,6 @@ def create_socket(host, port): def test_tcp_garbage(named_port): - import dns.query - with create_socket("10.53.0.7", named_port) as sock: msg = create_msg("a.example.", "A") @@ -68,9 +67,6 @@ def test_tcp_garbage(named_port): def test_tcp_garbage_response(named_port): - import dns.query - import dns.message - with create_socket("10.53.0.7", named_port) as sock: msg = create_msg("a.example.", "A") diff --git a/bin/tests/system/timeouts/tests-tcp.py b/bin/tests/system/timeouts/tests-tcp.py index a17380af58..e883dddda4 100644 --- a/bin/tests/system/timeouts/tests-tcp.py +++ b/bin/tests/system/timeouts/tests-tcp.py @@ -19,13 +19,18 @@ import time import pytest pytest.importorskip('dns', minversion='2.0.0') +import dns.edns +import dns.message +import dns.name +import dns.query +import dns.rdataclass +import dns.rdatatype TIMEOUT = 10 def create_msg(qname, qtype): - import dns.message msg = dns.message.make_query(qname, qtype, want_dnssec=True, use_edns=0, payload=4096) return msg @@ -39,8 +44,6 @@ def test_initial_timeout(named_port): # # The initial timeout is 2.5 seconds, so this should timeout # - import dns.query - with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: sock.connect(("10.53.0.1", named_port)) @@ -60,8 +63,6 @@ def test_idle_timeout(named_port): # # The idle timeout is 5 seconds, so the third message should fail # - import dns.rcode - msg = create_msg("example.", "A") with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: sock.connect(("10.53.0.1", named_port)) @@ -90,8 +91,6 @@ def test_keepalive_timeout(named_port): # # Keepalive is 7 seconds, so the third message should succeed. # - import dns.rcode - msg = create_msg("example.", "A") kopt = dns.edns.GenericOption(11, b'\x00') msg.use_edns(edns=True, payload=4096, options=[kopt]) @@ -119,8 +118,6 @@ def test_pipelining_timeout(named_port): # # The pipelining should only timeout after the last message is received # - import dns.query - msg = create_msg("example.", "A") with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: sock.connect(("10.53.0.1", named_port)) @@ -156,10 +153,6 @@ def test_long_axfr(named_port): # The timers should not fire during AXFR, thus the connection should not # close abruptly # - import dns.query - import dns.rdataclass - import dns.rdatatype - with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: sock.connect(("10.53.0.1", named_port)) @@ -186,8 +179,6 @@ def test_long_axfr(named_port): def test_send_timeout(named_port): - import dns.query - with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: sock.connect(("10.53.0.1", named_port)) @@ -215,10 +206,6 @@ def test_send_timeout(named_port): @pytest.mark.long def test_max_transfer_idle_out(named_port): - import dns.query - import dns.rdataclass - import dns.rdatatype - with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: sock.connect(("10.53.0.1", named_port)) @@ -250,10 +237,6 @@ def test_max_transfer_idle_out(named_port): @pytest.mark.long def test_max_transfer_time_out(named_port): - import dns.query - import dns.rdataclass - import dns.rdatatype - with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: sock.connect(("10.53.0.1", named_port)) diff --git a/bin/tests/system/wildcard/tests-wildcard.py b/bin/tests/system/wildcard/tests-wildcard.py index 3c724b1335..f4134225e1 100755 --- a/bin/tests/system/wildcard/tests-wildcard.py +++ b/bin/tests/system/wildcard/tests-wildcard.py @@ -35,7 +35,9 @@ import dns.message import dns.name import dns.query import dns.rcode +import dns.rdataclass import dns.rdatatype +import dns.rrset pytest.importorskip("hypothesis") from hypothesis import given From 00392921f0983e26813f8dd0a6d2589629bcee5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Mon, 14 Mar 2022 08:59:32 +0100 Subject: [PATCH 09/10] Rework skipping long tests The ability to conveniently mark tests which should only be run when the CI_ENABLE_ALL_TESTS environment variable is set seems to be useful on a general level and therefore it should not be limited to the "timeouts" system test, where it is currently used. pytest documentation [1] suggests to reuse commonly used test markers by putting them all in a single Python module which then has to be imported by test files that want to use the markers defined therein. Follow that advice by creating a new bin/tests/system/pytest_custom_markers.py Python module containing the relevant marker definitions. Note that "import pytest_custom_markers" works from a test-specific subdirectory because pytest modifies sys.path so that it contains the paths to all parent directories containing a conftest.py file (and bin/tests/system/ is one). PyLint does not like that, though, so add a relevant PyLint suppression. The above changes make bin/tests/system/timeouts/conftest.py redundant, so remove it. [1] https://docs.pytest.org/en/7.0.x/how-to/skipping.html#id1 --- bin/tests/system/pytest_custom_markers.py | 20 +++++++++++++++ bin/tests/system/run.sh.in | 2 +- bin/tests/system/timeouts/conftest.py | 30 ----------------------- bin/tests/system/timeouts/tests-tcp.py | 6 +++-- 4 files changed, 25 insertions(+), 33 deletions(-) create mode 100644 bin/tests/system/pytest_custom_markers.py delete mode 100644 bin/tests/system/timeouts/conftest.py diff --git a/bin/tests/system/pytest_custom_markers.py b/bin/tests/system/pytest_custom_markers.py new file mode 100644 index 0000000000..256fb7db8d --- /dev/null +++ b/bin/tests/system/pytest_custom_markers.py @@ -0,0 +1,20 @@ +#!/usr/bin/python3 + +# Copyright (C) Internet Systems Consortium, Inc. ("ISC") +# +# SPDX-License-Identifier: MPL-2.0 +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, you can obtain one at https://mozilla.org/MPL/2.0/. +# +# See the COPYRIGHT file distributed with this work for additional +# information regarding copyright ownership. + +import os + +import pytest + + +long_test = pytest.mark.skipif(not os.environ.get('CI_ENABLE_ALL_TESTS'), + reason='CI_ENABLE_ALL_TESTS not set') diff --git a/bin/tests/system/run.sh.in b/bin/tests/system/run.sh.in index 532134cee4..87a98bc671 100644 --- a/bin/tests/system/run.sh.in +++ b/bin/tests/system/run.sh.in @@ -111,7 +111,7 @@ if [ "${srcdir}" != "${builddir}" ]; then cp -a "${srcdir}/common" "${builddir}" fi # Some tests require additional files to work for out-of-tree test runs. - for file in ckdnsrps.sh conftest.py digcomp.pl ditch.pl fromhex.pl kasp.sh packet.pl start.pl stop.pl testcrypto.sh; do + for file in ckdnsrps.sh conftest.py digcomp.pl ditch.pl fromhex.pl kasp.sh packet.pl pytest_custom_markers.py start.pl stop.pl testcrypto.sh; do if [ ! -r "${file}" ]; then cp -a "${srcdir}/${file}" "${builddir}" fi diff --git a/bin/tests/system/timeouts/conftest.py b/bin/tests/system/timeouts/conftest.py deleted file mode 100644 index 3a4375232d..0000000000 --- a/bin/tests/system/timeouts/conftest.py +++ /dev/null @@ -1,30 +0,0 @@ -# Copyright (C) Internet Systems Consortium, Inc. ("ISC") -# -# SPDX-License-Identifier: MPL-2.0 -# -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, you can obtain one at https://mozilla.org/MPL/2.0/. -# -# See the COPYRIGHT file distributed with this work for additional -# information regarding copyright ownership. - -import os -import pytest - - -def pytest_configure(config): - config.addinivalue_line( - "markers", "long: mark tests that take a long time to run" - ) - - -def pytest_collection_modifyitems(config, items): - # pylint: disable=unused-argument,unused-import,too-many-branches - # pylint: disable=import-outside-toplevel - skip_long_tests = pytest.mark.skip( - reason="need CI_ENABLE_ALL_TESTS environment variable") - if not os.environ.get("CI_ENABLE_ALL_TESTS"): - for item in items: - if "long" in item.keywords: - item.add_marker(skip_long_tests) diff --git a/bin/tests/system/timeouts/tests-tcp.py b/bin/tests/system/timeouts/tests-tcp.py index e883dddda4..74d7cb695f 100644 --- a/bin/tests/system/timeouts/tests-tcp.py +++ b/bin/tests/system/timeouts/tests-tcp.py @@ -26,6 +26,8 @@ import dns.query import dns.rdataclass import dns.rdatatype +import pytest_custom_markers # pylint: disable=import-error + TIMEOUT = 10 @@ -204,7 +206,7 @@ def test_send_timeout(named_port): raise EOFError from e -@pytest.mark.long +@pytest_custom_markers.long_test def test_max_transfer_idle_out(named_port): with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: sock.connect(("10.53.0.1", named_port)) @@ -235,7 +237,7 @@ def test_max_transfer_idle_out(named_port): assert soa is None -@pytest.mark.long +@pytest_custom_markers.long_test def test_max_transfer_time_out(named_port): with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: sock.connect(("10.53.0.1", named_port)) From 173ad9cf467f06493393148e855c6208bc99e654 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Mon, 14 Mar 2022 08:59:32 +0100 Subject: [PATCH 10/10] Tweak Automake conditionals for pytest-based tests Since pytest itself skips tests using dnspython if the latter is not available, also using Automake conditionals for silently skipping pytest-based tests requiring dnspython is redundant and hides information. Allow all pytest-based tests requiring dnspython to be run whenever pytest itself is available, in order to ensure test skipping is done in a uniform manner. Note that the above reasoning only applies to pytest-based tests, so similar adjustments were not made for shell-based tests using Python scripts that require dnspython ("chain", "cookie", "dnssec", "qmin"). --- bin/tests/system/Makefile.am | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/bin/tests/system/Makefile.am b/bin/tests/system/Makefile.am index c21ce5436f..215b42a114 100644 --- a/bin/tests/system/Makefile.am +++ b/bin/tests/system/Makefile.am @@ -214,20 +214,18 @@ endif HAVE_PERLMOD_NET_DNS if HAVE_PYTHON TESTS += kasp keymgr2kasp tcp pipelined -if HAVE_PYMOD_DNS -TESTS += checkds dispatch qmin cookie timeouts +if HAVE_PYTEST +TESTS += checkds dispatch rpzextra shutdown timeouts +endif +if HAVE_PYMOD_DNS +TESTS += qmin cookie if HAVE_PERLMOD_NET_DNS TESTS += dnssec if HAVE_PERLMOD_NET_DNS_NAMESERVER TESTS += chain endif HAVE_PERLMOD_NET_DNS_NAMESERVER endif HAVE_PERLMOD_NET_DNS - -if HAVE_PYTEST -TESTS += rpzextra shutdown -endif - endif HAVE_PYMOD_DNS endif HAVE_PYTHON