From 4c3255fd73dee022885d30c41695313a9df7f05f Mon Sep 17 00:00:00 2001 From: Aleksey Popov Date: Thu, 14 May 2026 13:42:16 +0000 Subject: [PATCH] Fix: Parametrize transformation tests and anchor pytest environment The transformation suite had three usability defects that all surface as "the tests silently lie about what they're testing": 1. From any cwd other than tests/, glob.glob('topology/input/*yml') returned [], the for-loop ran zero iterations, and pytest reported PASSED. A test file globbing relative paths cannot be safely invoked from anywhere except tests/, but AGENTS.md showed exactly that. 2. From a git worktree, 'cd tests && pytest' silently imported the netsim package from the venv egg-link (the main checkout) instead of the worktree's own code, because sys.path[0]='' resolves to cwd at interpreter startup and tests/ has no netsim/. The wrapper scripts papered over this with PYTHONPATH=../ but bare pytest did not. 3. All 109 transform cases (and 110 error cases, and the two coverage suites) ran inside a single for-loop with one assert. The first failing case aborted the rest -- CI and developers saw "1 failed" without knowing whether the regression touched 1 case or 50, and -k couldn't target an individual case because they weren't pytest nodes. Changes: - New tests/conftest.py: prepends the worktree root to sys.path so imports always resolve against this checkout's netsim/, and chdir's to tests/ in pytest_configure so the existing relative globs work from any invocation cwd. - tests/test_transformation.py: @pytest.mark.parametrize over the four glob.glob() lists so each YAML case becomes its own pytest node (test_xform_cases[topology/input/foo.yml], etc.). -k can now target one case; one failure no longer hides others. test_coverage_verbose_cases iterates the glob directly since it's no longer callable as a function after parametrize. The unreachable __main__ block (broken signature, no callers) was removed. The run-tests.sh / run-xform.sh / run-xerr.sh / run-coverage-tests.sh wrappers continue to work unchanged; their -k selectors keep matching because the function names are preserved. Co-Authored-By: Claude Opus 4.7 --- tests/conftest.py | 45 ++++++++++++++++++++++++++---------- tests/test_transformation.py | 36 ++++++++++++----------------- 2 files changed, 48 insertions(+), 33 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 2118d241a7..203efeb986 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,19 +1,40 @@ # -# Pytest hooks shared across the netlab test tree +# Shared pytest configuration for the netlab test suite. +# +# 1. Prepend the repository root (the directory containing the 'netsim' +# package) to sys.path so 'cd tests && pytest' inside a git worktree +# imports this checkout's netsim/, not the venv egg-link's (which +# usually points at the main checkout). +# +# 2. Anchor the working directory to tests/ so the test bodies' relative +# globs (e.g. glob.glob('topology/input/*yml')) resolve no matter +# where pytest is invoked from. +# +# 3. Surface a UserWarning when ruamel.yaml is installed -- the +# transformation tests will be slower and create-error-tests.sh is +# unsupported (see https://github.com/ipspace/netlab/issues/3345). # +import os +import pathlib +import sys + import pytest -from utils import HAS_RUAMEL + +_HERE = pathlib.Path(__file__).resolve().parent +sys.path.insert(0, str(_HERE.parent)) + +from utils import HAS_RUAMEL # noqa: E402 -- requires sys.path tweak above def pytest_configure(config: pytest.Config) -> None: - if not HAS_RUAMEL: - return - config.issue_config_time_warning( - UserWarning( - "ruamel.yaml is installed; transformation tests will be slower and " - "`create-error-tests.sh` is unsupported " - "(see https://github.com/ipspace/netlab/issues/3345)." - ), - stacklevel=2, - ) + os.chdir(_HERE) + if HAS_RUAMEL: + config.issue_config_time_warning( + UserWarning( + "ruamel.yaml is installed; transformation tests will be slower and " + "`create-error-tests.sh` is unsupported " + "(see https://github.com/ipspace/netlab/issues/3345)." + ), + stacklevel=2, + ) diff --git a/tests/test_transformation.py b/tests/test_transformation.py index 08037d75ad..0a37c5d8c0 100755 --- a/tests/test_transformation.py +++ b/tests/test_transformation.py @@ -63,12 +63,11 @@ def run_transformation_test(test_case: str, tmpdir: str = '/tmp') -> None: assert result == expected print("... succeeded, string length = %d" % len(result)) - + @pytest.mark.filterwarnings("ignore::PendingDeprecationWarning") -def test_xform_cases(tmpdir: str) -> None: - print("Starting transformation test cases") - for test_case in list(glob.glob('topology/input/*yml')): - run_transformation_test(test_case) +@pytest.mark.parametrize('test_case',sorted(glob.glob('topology/input/*yml'))) +def test_xform_cases(test_case: str, tmpdir: str) -> None: + run_transformation_test(test_case) # Verbose test cases are executed only when we're doing a coverage report # @@ -76,7 +75,8 @@ def test_coverage_verbose_cases(tmpdir: str) -> None: if not sys.gettrace(): return log.set_verbose() - test_xform_cases(tmpdir) + for test_case in sorted(glob.glob('topology/input/*yml')): + run_transformation_test(test_case) def run_error_case(test_case: str) -> None: log.set_flag(raise_error = True) @@ -97,22 +97,16 @@ def run_error_case(test_case: str) -> None: assert error_log == log_lines @pytest.mark.filterwarnings("ignore::PendingDeprecationWarning") -def test_error_cases() -> None: - print("Starting error test cases") - for test_case in list(glob.glob('errors/*yml')): - run_error_case(test_case) +@pytest.mark.parametrize('test_case',sorted(glob.glob('errors/*yml'))) +def test_error_cases(test_case: str) -> None: + run_error_case(test_case) @pytest.mark.filterwarnings("ignore::PendingDeprecationWarning") -def test_coverage_xf_cases() -> None: - for test_case in list(glob.glob('coverage/input/*yml')): - run_transformation_test(test_case) +@pytest.mark.parametrize('test_case',sorted(glob.glob('coverage/input/*yml'))) +def test_coverage_xf_cases(test_case: str) -> None: + run_transformation_test(test_case) @pytest.mark.filterwarnings("ignore::PendingDeprecationWarning") -def test_coverage_errors() -> None: - for test_case in list(glob.glob('coverage/errors/*yml')): - run_error_case(test_case) - -if __name__ == "__main__": - test_xform_cases("/tmp") -# test_error_cases() -# test_minimal_cases() +@pytest.mark.parametrize('test_case',sorted(glob.glob('coverage/errors/*yml'))) +def test_coverage_errors(test_case: str) -> None: + run_error_case(test_case)