Fix: Let pytest drive test output and isolate per-case file writes

[a-v-popov]

This is the second PR for pytest following #3391. After that PR manual test outputs are limited to a specific case, but they are mostly redundant.

$ pytest -q --tb=short -k topology/input/addressing-lan.yml
F                                                                                                                [100%]
======================================================= FAILURES =======================================================
_________________________________ test_xform_cases[topology/input/addressing-lan.yml] __________________________________
/home/apopov/netlab/tests/test_transformation.py:70: in test_xform_cases
    run_transformation_test(test_case)
/home/apopov/netlab/tests/test_transformation.py:64: in run_transformation_test
    assert result == expected
E   AssertionError: assert 'input:\n- to...er: libvirt\n' == 'input:\n- to...er: libvirt\n'
E     
E     Skipping 302 identical leading characters in diff, use -v to show
E       r2
E     -   - ififindex: 2
E     ?     --
E     +   - ifindex: 2
E           ifname: GigabitEthernet2...
E     
E     ...Full output truncated (1098 lines hidden), use '-vv' to show
------------------------------------------------- Captured stdout call -------------------------------------------------
Test case: topology/input/addressing-lan.yml
Test case: topology/input/addressing-lan.yml FAILED
--- expected
+++ result
@@ -13,7 +13,7 @@
     ifname: GigabitEthernet2
     ipv6: 2001:db8:1::15/64
     node: r2
-  - ififindex: 2
+  - ifindex: 2
     ifname: GigabitEthernet2
     ipv6: 2001:db8:1::2a/64
     node: r3
------------------------------------------------- Captured stderr call -------------------------------------------------
Warning in csr: Changing Ansible network_cli connection SSH type to 'paramiko'
... The installed version of ansible-pylibssh might not work with Cisco IOS devices
... Set defaults.devices.csr.warnings.paramiko to False to hide this warning
=============================================== short test summary info ================================================
FAILED tests/test_transformation.py::test_xform_cases[topology/input/addressing-lan.yml] - AssertionError: assert 'input:\n- to...er: libvirt\n' == 'input:\n- to...er: libvirt\n'
1 failed, 273 deselected in 0.90s

Information in "Captured stdout call" section already available in the standard pytest output.
This PR aggressively removes all the duplication.
Alternatives:

0. do nothing, ignore duplication
1. disable pytest assert rewrite
2. gate test prints by -v
3. switch to python logging with appropriate log level
4. store diffs as artifacts

Depending on what is the goal we might choose one of the alternatives.
Specifically if we need to capture large diffs in CI the latter is likely to be the right choice.
It is possible that I devalue output like "Writing inventory…" too much, informed opinion would be appreciated.

Another change in the PR is usage of tmp_path and tmp_path_factory to isolate test cases. These are not free, but their impact should be negligible, while general robustness of test results should improve.

Below is a more detailed (and AI driven) description of the changes proposed in this SR.

Summary

• Remove the manual print() and difflib.unified_diff scaffolding from test_transformation.py. With each YAML case as its own pytest node, pytest's -v and assertion rewriter cover the same ground and respect the standard verbosity flags.
• Switch per-case file writes to pytest's tmp_path fixture via os.chdir, so ansible_inventory's CWD-relative group_vars/ and host_vars/ writes no longer land in tests/.
• Drop the trailing rm -fr *files cleanup hack from run-tests.sh and run-coverage-tests.sh — the tests no longer write into tests/.
• Convert test_coverage_verbose_cases to @pytest.mark.skipif so its result is visibly SKIPPED outside coverage runs instead of silently PASSED, and switch its inner loop to tmp_path_factory.mktemp(...) so each iteration gets its own scratch dir (matching the parametrized suite's per-case isolation).

Why

This PR completes the cleanup by removing the duplicated framework-level work the harness was doing by hand.

Manual output handling duplicates pytest

run_transformation_test prints "Test case: %s" regardless of pytest verbosity. On success it adds "… succeeded, string length = %d". On failure it manually dumps a difflib.unified_diff to stdout before letting assert raise — so pytest's own assertion-rewriter diff appears alongside it, doubled up. run_error_case does the same with a manual Accumulated error log block.

With each YAML case now a pytest node (per the previous PR), pytest already supplies:

• node IDs — replaces the per-case "Test case: …" print
• an assertion-rewritten unified diff on failure — replaces the manual difflib.unified_diff
• -q for quiet, -vv for full diff expansion — replaces a hard-coded chatty default

Deleting the manual code makes the standard pytest flags work as users expect.

Per-case file writes leak into tests/

run_transformation_test passes tmpdir+"/extra/hosts.yml" to ansible.ansible_inventory, but ansible_inventory also writes group_vars/<g>/topology and host_vars/<h>/topology to CWD — which is tests/ after the conftest chdir. The wrappers worked around this with rm -fr *files at the end; cases polluted each other's state inside a single run.

Switching to tmp_path (via a localized os.chdir around the write block) gives each parametrized case its own scratch directory. The cleanup hack becomes unnecessary, cases stop stepping on each other, and tests/ stays clean.

test_coverage_verbose_cases was silently a no-op, and its scratch dir wasn't deterministic either

if not sys.gettrace(): return exits cleanly with status PASSED outside coverage runs — the same silent-PASS pattern the previous PR fixed elsewhere. Converting to @pytest.mark.skipif(not sys.gettrace(), reason="coverage-only test") reports SKIPPED, which is the truthful result.

While we're here: the function's inner loop reused a single tmp_path across all 109 iterations, which doesn't change the assertion (the comparison is against in-memory state, not disk) but does make coverage measurement filesystem-state-sensitive. If any output module's "create-if-missing" branch sees the previous case's group_vars/ already present, case N+1 doesn't exercise the same lines it would in a clean dir. Coverage results become invocation-order-dependent. Switching to tmp_path_factory.mktemp(...) per iteration extends the parametrized suite's per-case isolation guarantee to this path — standard pytest idiom, two-line change.

What changes

• tests/test_transformation.py
  • Delete the unconditional print() calls in run_transformation_test ("Test case: …", "Writing inventory…", "… succeeded, …", "Test case: … FAILED") and in run_error_case ("Test case: …", the Accumulated error log block).
  • Delete the manual sys.stdout.writelines(difflib.unified_diff(...)) call. Bare assert result == expected becomes assert result == expected, f"transformation mismatch for {test_case}"; pytest's assertion rewriter produces the diff.
  • run_transformation_test takes tmp_path: pathlib.Path instead of the vestigial tmpdir: str = '/tmp'. Around the file-writing block (ansible.ansible_inventory, ansible.ansible_config, ansible.dump, _TopologyOutput.write) it os.chdirs into tmp_path so all CWD-relative writes — including the group_vars/ and host_vars/ trees ansible_inventory creates internally — land inside the per-test temp dir. The topology read happens before the chdir; the expected-fixture read happens after the finally: os.chdir(cwd).
  • test_coverage_verbose_cases → @pytest.mark.skipif(not sys.gettrace(), reason="coverage-only test"), and its inner loop takes a tmp_path_factory instead of a single tmp_path so each iteration gets a fresh tmp_path_factory.mktemp("coverage") scratch dir.
  • Drop the now-unused import difflib.
• tests/run-tests.sh and tests/run-coverage-tests.sh
  • Drop the # Remove files unnecessarily created by various provider modules (until we fix that) / rm -fr *files block.

run-xform.sh and run-xerr.sh are unchanged — they never had the cleanup hack.

Anticipated questions

• "The legacy chatty output was useful for tracking progress on long runs. Why remove it?" It's still reachable — -v lists every case as it runs, and pytest -v --no-header -q is a knob users already know. The deletion is about not duplicating that information when users have not asked for it. If you want a real progress bar, pytest-sugar or pytest-progress are one-line plugin installs.
• "Does the os.chdir around the write block leak if a test errors mid-block?" It is wrapped in try / finally, so even an exception inside the inventory/output write restores the original CWD before the test moves on or the assertion runs.
• "Why not refactor ansible_inventory to take a base dir instead of using chdir?" Bigger change, affects production code paths and other callers. chdir is contained to one test helper and inherits the per-test isolation pytest already provides via tmp_path. Worth doing eventually but not in this PR.
• "Does dropping rm -fr *files risk leftover state if something regresses?" Yes — a regression that re-introduces CWD-relative writes from inside run_transformation_test would leak. We mitigate this in the test plan below; longer-term, a pytest autouse fixture that snapshots tests/ contents and fails the test on diff would close the gap. Out of scope here.
• "The -vv change subtly changes failure output." Yes — Left / Right headers in pytest's rewriter replace expected / result in the legacy difflib output. Same content, different labels. The labels follow operand order in assert result == expected (so Left = result, Right = expected); if that ordering is confusing we can flip the operands.

Test plan

• cd tests && python3 -m pytest -q -k 'xform_ or error_cases' — expect 219 passed, output is dots + summary only, no Test case: / succeeded lines.
• cd tests && python3 -m pytest -v -k xform_ — expect node IDs and PASSED lines, no leftover Test case: prints.
• Corrupt one expected fixture (e.g. tests/topology/expected/6pe.yml), run cd tests && python3 -m pytest -vv -k xform_. Confirm the failure section shows pytest's unified-diff-style assertion expansion, with no duplicate manual diff in stdout. Confirm summary shows "1 failed, 108 passed". Revert.
• After any test run, ls tests/group_vars tests/host_vars tests/*files 2>/dev/null returns no results — tests/ stays clean without the wrapper cleanup.
• cd tests && ./run-tests.sh — expect 219 passed, exits cleanly even with the rm -fr *files line removed.
• cd tests && ./run-coverage-tests.sh — expect coverage tests pass, tests/ clean afterwards.
• cd tests && python3 -m pytest -v -k coverage_verbose — expect SKIPPED (coverage-only test), not silent PASSED.
• Under a coverage run (e.g. coverage run -m pytest -k coverage_verbose), confirm test_coverage_verbose_cases is collected and runs.