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] 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")