diff --git a/.github/workflows/t-pull.yml b/.github/workflows/t-pull.yml index c26ccb830f..60c7d312de 100644 --- a/.github/workflows/t-pull.yml +++ b/.github/workflows/t-pull.yml @@ -37,11 +37,31 @@ jobs: for file in netsim/extra/*/plugin.py; do python3 -m mypy $file done + - name: Compute pytest tmp path + if: ${{ github.event.pull_request.head.repo.full_name != 'ipspace/netlab' }} + run: echo "PYTEST_TMP=$RUNNER_TEMP/pytest-tmp" >> "$GITHUB_ENV" - name: Run transformation tests + id: pytest if: ${{ github.event.pull_request.head.repo.full_name != 'ipspace/netlab' }} run: | cd tests - PYTHONPATH="../" pytest + PYTHONPATH="../" pytest --basetemp="$PYTEST_TMP" --junit-xml="$PYTEST_TMP/junit.xml" + - name: Prune empty case dirs + if: >- + ${{ !cancelled() && steps.pytest.outcome == 'failure' + && github.event.pull_request.head.repo.full_name != 'ipspace/netlab' }} + run: | + find "$PYTEST_TMP" -type d -empty -delete + find "$PYTEST_TMP" -xtype l -delete + - name: Upload pytest artifacts + if: >- + ${{ !cancelled() && steps.pytest.outcome == 'failure' + && github.event.pull_request.head.repo.full_name != 'ipspace/netlab' }} + uses: actions/upload-artifact@v7 + with: + name: pytest-tmp-py${{ matrix.python-version }} + path: ${{ env.PYTEST_TMP }} + retention-days: 7 - name: Check integration tests run: | cd tests diff --git a/.github/workflows/t-push.yml b/.github/workflows/t-push.yml index c97d1d24b3..cb63379be9 100644 --- a/.github/workflows/t-push.yml +++ b/.github/workflows/t-push.yml @@ -31,7 +31,22 @@ jobs: for file in netsim/extra/*/plugin.py; do python3 -m mypy $file done + - name: Compute pytest tmp path + run: echo "PYTEST_TMP=$RUNNER_TEMP/pytest-tmp" >> "$GITHUB_ENV" - name: Run transformation tests + id: pytest run: | cd tests - PYTHONPATH="../" pytest + PYTHONPATH="../" pytest --basetemp="$PYTEST_TMP" --junit-xml="$PYTEST_TMP/junit.xml" + - name: Prune empty case dirs + if: ${{ !cancelled() && steps.pytest.outcome == 'failure' }} + run: | + find "$PYTEST_TMP" -type d -empty -delete + find "$PYTEST_TMP" -xtype l -delete + - name: Upload pytest artifacts + if: ${{ !cancelled() && steps.pytest.outcome == 'failure' }} + uses: actions/upload-artifact@v7 + with: + name: pytest-tmp-py${{ matrix.python-version }} + path: ${{ env.PYTEST_TMP }} + retention-days: 7 diff --git a/tests/conftest.py b/tests/conftest.py index 203efeb986..90964810eb 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -14,10 +14,22 @@ # transformation tests will be slower and create-error-tests.sh is # unsupported (see https://github.com/ipspace/netlab/issues/3345). # +# 4. Normalize the failure-report "reprcrash" message to its first line so +# pytest's `short test summary info` does not duplicate the diff body +# that `_report_mismatch` puts in the `pytest.fail(...)` message. The +# rich multi-line message stays in `longrepr`, so the `FAILURES` +# section is unaffected. Without this, pytest 9.x prints the entire +# failure message verbatim in the short summary whenever it detects +# CI (`CI`/`BUILD_NUMBER` env vars) or runs at `-vv`+ -- see +# `_pytest/terminal.py::_get_line_with_reprcrash_message`. Pytest's +# own `ReprFileLocation.toterminal` already trims to the first line; +# we just enforce the same invariant globally. +# import os import pathlib import sys +import typing import pytest @@ -38,3 +50,12 @@ def pytest_configure(config: pytest.Config) -> None: ), stacklevel=2, ) + + +@pytest.hookimpl(hookwrapper=True) +def pytest_runtest_makereport(item: pytest.Item, call: pytest.CallInfo[None]) -> typing.Generator[None, None, None]: + outcome = yield + report = outcome.get_result() # type: ignore[attr-defined] + crash = getattr(getattr(report, "longrepr", None), "reprcrash", None) + if crash is not None and "\n" in crash.message: + crash.message = crash.message.split("\n", 1)[0] diff --git a/tests/test_transformation.py b/tests/test_transformation.py index e6e2c7d8ba..661a5afec5 100755 --- a/tests/test_transformation.py +++ b/tests/test_transformation.py @@ -4,6 +4,7 @@ # topology file # +import difflib import glob import os import pathlib @@ -20,6 +21,8 @@ from netsim.utils import log from netsim.utils import read as _read +MAX_PREVIEW_LINES = 20 +MAX_PREVIEW_LINES_VERBOSE = 2 * MAX_PREVIEW_LINES def run_test(fname: str) -> Box: log.init_log_system(header = False) @@ -64,14 +67,62 @@ def transformation_results(test_case: str, tmp_path: pathlib.Path) -> typing.Tup return (result,expected) -def run_transformation_test(test_case: str, tmp_path: pathlib.Path) -> None: +# On mismatch, drop the rendered output and a unified diff into tmp_path +# so CI's upload-artifact step can surface them, then raise pytest.fail +# with a diff preview in the message. Constructing the failure message +# ourselves (rather than letting `assert actual == expected` go through) +# skips pytest's assertion rewriter, which would otherwise emit an ndiff +# truncated to ~8 lines of context at default verbosity and flooded with +# matching lines at -vv. pytest.fail is preferred over raise +# AssertionError for its pytrace= argument: a fixture mismatch needs no +# helper-frame traceback at default verbosity. +# +# Preview length and pytrace are tied to pytest's -v count: +# verbose<=0 -> cap at MAX_PREVIEW_LINES (CI / default pytest) +# verbose==1 -> cap at MAX_PREVIEW_LINES_VERBOSE (run-tests.sh, -v) +# verbose>=2 -> no cap (targeted local run) +# verbose>=3 -> no cap, pytrace on (debugging this harness) +# The full diff is written to disk regardless, so CI artifact upload is +# unaffected by the cap. +def _report_mismatch( + test_case: str, + label: str, + actual: str, + expected: str, + tmp_path: pathlib.Path, + actual_name: str, + verbose: int, + ) -> None: + if actual == expected: + return + + diff_lines = list(difflib.unified_diff( + expected.splitlines(keepends=True), + actual.splitlines(keepends=True), + fromfile='expected',tofile='actual')) + if verbose >= 2: + cap: typing.Optional[int] = None + elif verbose >= 1: + cap = MAX_PREVIEW_LINES_VERBOSE + else: + cap = MAX_PREVIEW_LINES + preview = "".join(diff_lines if cap is None else diff_lines[:cap]) + if cap is not None and len(diff_lines) > cap: + preview += "\n... diff truncated ..." + actual_path = tmp_path / actual_name + actual_path.write_text(actual) + diff_path = actual_path.with_suffix(".diff") + diff_path.write_text("".join(diff_lines)) + pytest.fail(f"{label} mismatch for {test_case}\n{preview}\n\nFull diff: {diff_path}",pytrace=verbose >= 3) + +def run_transformation_test(test_case: str, tmp_path: pathlib.Path, verbose: int) -> None: (result,expected) = transformation_results(test_case,tmp_path) - assert result == expected, f"transformation mismatch for {test_case}" + _report_mismatch(test_case,"transformation",result,expected,tmp_path,"result.yml",verbose) @pytest.mark.filterwarnings("ignore::PendingDeprecationWarning") @pytest.mark.parametrize('test_case',sorted(glob.glob('topology/input/*yml'))) -def test_xform_cases(test_case: str, tmp_path: pathlib.Path) -> None: - run_transformation_test(test_case,tmp_path) +def test_xform_cases(test_case: str, tmp_path: pathlib.Path, pytestconfig: pytest.Config) -> None: + run_transformation_test(test_case,tmp_path,pytestconfig.getoption("verbose")) # Verbose test cases are executed only when we're running under coverage # (sys.gettrace() returns the tracer); skipped otherwise so the result is @@ -83,10 +134,14 @@ def test_xform_cases(test_case: str, tmp_path: pathlib.Path) -> None: # exercise different code paths depending on iteration order. # @pytest.mark.skipif(not sys.gettrace(),reason="coverage-only test") -def test_coverage_verbose_cases(tmp_path_factory: pytest.TempPathFactory) -> None: +def test_coverage_verbose_cases( + tmp_path_factory: pytest.TempPathFactory, + pytestconfig: pytest.Config, + ) -> None: log.set_verbose() + verbose = pytestconfig.getoption("verbose") for test_case in sorted(glob.glob('topology/input/*yml')): - run_transformation_test(test_case,tmp_path_factory.mktemp("coverage")) + run_transformation_test(test_case,tmp_path_factory.mktemp("coverage"),verbose) def error_results(test_case: str) -> typing.Tuple[str, str]: log.set_flag(raise_error = True) @@ -97,22 +152,22 @@ def error_results(test_case: str) -> typing.Tuple[str, str]: log_file = pathlib.Path(test_case.replace('.yml','.log')) expected_log = log_file.read_text().strip('\n') if log_file.exists() else "" return (error_log,expected_log) - -def run_error_case(test_case: str) -> None: + +def run_error_case(test_case: str, tmp_path: pathlib.Path, verbose: int) -> None: (error_log,expected_log) = error_results(test_case) - assert error_log == expected_log, f"error-log mismatch for {test_case}" + _report_mismatch(test_case,"error-log",error_log,expected_log,tmp_path,"error_log.actual",verbose) @pytest.mark.filterwarnings("ignore::PendingDeprecationWarning") @pytest.mark.parametrize('test_case',sorted(glob.glob('errors/*yml'))) -def test_error_cases(test_case: str) -> None: - run_error_case(test_case) +def test_error_cases(test_case: str, tmp_path: pathlib.Path, pytestconfig: pytest.Config) -> None: + run_error_case(test_case,tmp_path,pytestconfig.getoption("verbose")) @pytest.mark.filterwarnings("ignore::PendingDeprecationWarning") @pytest.mark.parametrize('test_case',sorted(glob.glob('coverage/input/*yml'))) -def test_coverage_xf_cases(test_case: str, tmp_path: pathlib.Path) -> None: - run_transformation_test(test_case,tmp_path) +def test_coverage_xf_cases(test_case: str, tmp_path: pathlib.Path, pytestconfig: pytest.Config) -> None: + run_transformation_test(test_case,tmp_path,pytestconfig.getoption("verbose")) @pytest.mark.filterwarnings("ignore::PendingDeprecationWarning") @pytest.mark.parametrize('test_case',sorted(glob.glob('coverage/errors/*yml'))) -def test_coverage_errors(test_case: str) -> None: - run_error_case(test_case) +def test_coverage_errors(test_case: str, tmp_path: pathlib.Path, pytestconfig: pytest.Config) -> None: + run_error_case(test_case,tmp_path,pytestconfig.getoption("verbose"))