From a388a689cfb31284e71f59d9fc8502af70e84309 Mon Sep 17 00:00:00 2001 From: Tom Krizek Date: Tue, 28 Mar 2023 16:52:49 +0200 Subject: [PATCH] Ensure --dist=loadscope is used when running pytest in parallel The loadscope setting is required for parallel execution of our system tests using pytest. The option ensure that all tests within a single (module) scope will be assigned to the same worker. This is neccessary because the worker sets up the nameservers for all the tests within a module scope. If tests from the same module would be assigned to different workers, then the setup could happen multiple times, causing a race condition. This happens because each module uses deterministic port numbers for the nameservers. (cherry picked from commit 8f57bce7afd963062c15bf66f4cb43bec523c0cf) --- .gitlab-ci.yml | 2 +- bin/tests/system/README | 17 +++++++---------- bin/tests/system/conftest.py | 16 +++++++++++++++- bin/tests/system/run.sh | 3 +-- 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 9ccc7c7a40..f5aafb2065 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -321,7 +321,7 @@ stages: - *find_pytest - cd bin/tests/system - > - "$PYTEST" --junit-xml="$CI_PROJECT_DIR"/junit.xml -n "$TEST_PARALLEL_JOBS" --dist loadscope | tee pytest.out.txt + "$PYTEST" --junit-xml="$CI_PROJECT_DIR"/junit.xml -n "$TEST_PARALLEL_JOBS" | tee pytest.out.txt - '( ! grep -F "grep: warning:" pytest.out.txt )' after_script: - awk '/^=+ FAILURES =+/{flag=1;next}/^=+.*=+$/{flag=0}flag' bin/tests/system/pytest.out.txt || true diff --git a/bin/tests/system/README b/bin/tests/system/README index 6a84940966..ae7354dfa8 100644 --- a/bin/tests/system/README +++ b/bin/tests/system/README @@ -97,12 +97,7 @@ Issuing plain `pytest` command without any argument will execute all tests sequenatially. To execute them in parallel, ensure you have pytest-xdist installed and run: -pytest --dist loadscope -n - -It is vital to provide the `--dist loadscope` option when running the tests in -parallel to ensure tests from a single module are executed from the same -thread. Otherwise, there's a risk of port contention and inefficient use of -resources. +pytest -n Running the System Tests Using the Legacy Runner @@ -754,11 +749,13 @@ collisions are likely to occur. Pytest-xdist is used for executing pytest test cases in parallel using the `-n N_WORKERS` option. By default, xdist will distribute any test case to any -worker, which would lead to the issue described above. Therefore, it is vital -to use the `--dist loadscope` option which ensures that test cases within the -same (module) scope will be handled by the same worker. +worker, which would lead to the issue described above. Therefore, conftest.py +enforces equivalent of `--dist loadscope` option which ensures that test cases +within the same (module) scope will be handled by the same worker. Parallelism +is automatically disabled when xdist.scheduler.loadscope library is not +available. -$ pytest -n auto --dist loadscope +$ pytest -n auto Test selection --- diff --git a/bin/tests/system/conftest.py b/bin/tests/system/conftest.py index 853a49fe4e..adee5540eb 100644 --- a/bin/tests/system/conftest.py +++ b/bin/tests/system/conftest.py @@ -130,7 +130,7 @@ if os.getenv("LEGACY_TEST_RUNNER", "0") == "0": help="don't remove the temporary test directories with artifacts", ) - def pytest_configure(): + def pytest_configure(config): # Ensure this hook only runs on the main pytest instance if xdist is # used to spawn other workers. if not XDIST_WORKER: @@ -156,6 +156,20 @@ if os.getenv("LEGACY_TEST_RUNNER", "0") == "0": raise exc logging.debug(proc.stdout) + if config.pluginmanager.has_plugin("xdist") and config.option.numprocesses: + # system tests depend on module scope for setup & teardown + # enforce use "loadscope" scheduler or disable paralelism + try: + import xdist.scheduler.loadscope # pylint: disable=unused-import + except ImportError: + logging.debug( + "xdist is too old and does not have " + "scheduler.loadscope, disabling parallelism" + ) + config.option.dist = "no" + else: + config.option.dist = "loadscope" + def pytest_ignore_collect(path): # System tests are executed in temporary directories inside # bin/tests/system. These temporary directories contain all files diff --git a/bin/tests/system/run.sh b/bin/tests/system/run.sh index 27d122857f..2f4fcf7dd5 100755 --- a/bin/tests/system/run.sh +++ b/bin/tests/system/run.sh @@ -28,7 +28,6 @@ def into_pytest_args(in_args): if in_args.expression is None: # running all tests - execute in parallel args.extend(["-n", "auto"]) - args.extend(["--dist", "loadscope"]) else: args.extend(["-k", in_args.expression]) if in_args.noclean: @@ -48,7 +47,7 @@ def main(): "Using pytest system test runner\n\n" 'Please consider invoking "pytest" directly for more control:\n' " single test: pytest -k dns64\n" - " parallel tests: pytest -n auto --dist loadscope\n\n" + " parallel tests: pytest -n auto\n\n" "Alternately, use ./legacy.run.sh for the legacy system test runner.\n" )