Skip to content

fix: deduplicate must-gather on shared fixture failures #626

Description

@myakove

Problem

pytest_exception_interact fires for every test method that encounters an error. When a class-scoped fixture (like prepared_plan) fails during setup, ALL test methods in the class get a setup ERROR. Each ERROR triggers a separate run_must_gather() call — e.g., 6 tests = 6 identical must-gathers for the same root cause.

Observed behavior

When running TestPlanArchivePvcCleanup against an RHV provider with an inactive NFS storage domain, prepared_plan (class-scoped) failed once during VM cloning. All 6 test methods received setup ERRORs, and run_must_gather() was called 6 times — collecting identical cluster state each time.

Root cause

pytest_exception_interact in conftest.py has no deduplication logic. It doesn't check whether the same fixture error already triggered must-gather for a sibling test in the same class.

# conftest.py — current code (no dedup)
def pytest_exception_interact(node, call, report):
    if is_dry_run(node.session.config):
        return

    if not node.session.config.getoption("skip_data_collector"):
        _data_collector_path = Path(
            f"{node.session.config.getoption('data_collector_path')}/{sanitize_test_name_for_path(node.name)}"
        )
        plan = None
        if hasattr(node, "cls") and node.cls and hasattr(node.cls, "plan_resource"):
            plan_obj = node.cls.plan_resource
            if plan_obj:
                plan = {"name": plan_obj.name, "namespace": plan_obj.namespace}

        run_must_gather(data_collector_path=_data_collector_path, plan=plan)

What already works (no fix needed)

Scenario Must-gathers Why
Test 1 call FAILED, tests 2-6 xfail (@pytest.mark.incremental) 1 xfail doesn't trigger pytest_exception_interact
Test 1 call FAILED, tests 2-6 independent (no incremental) Each gets its own Correct — different test, different cluster state

What's broken

Scenario Must-gathers Should be
Class-scoped fixture fails → N tests get setup ERROR N (e.g., 6) 1

Proposed Fix

Add deduplication in pytest_exception_interact for setup-phase errors only:

  1. Track collected fixture errors per class using a session-level set (e.g., _must_gather_collected_fixtures) stored on the session object
  2. For call.when == "setup" with class-based tests: build a dedup key from (node.cls, exception type, truncated exception message) — if already in the set, skip must-gather
  3. For call.when == "call": always collect (no change — each test failure is potentially different, and @pytest.mark.incremental already handles dedup via xfail)

Implementation sketch

def pytest_exception_interact(node, call, report):
    if is_dry_run(node.session.config):
        return

    if not node.session.config.getoption("skip_data_collector"):
        # Deduplicate must-gather for shared fixture failures.
        # When a class-scoped fixture fails, ALL test methods in the class
        # get a setup ERROR — but must-gather only needs to run once.
        if call.when == "setup" and hasattr(node, "cls") and node.cls:
            collected = getattr(node.session, "_must_gather_collected_fixtures", set())
            dedup_key = (node.cls, str(type(call.excinfo.value)), str(call.excinfo.value)[:200])
            if dedup_key in collected:
                LOGGER.info(
                    f"Skipping duplicate must-gather for {node.name} "
                    "— same fixture error already collected for this class"
                )
                return
            collected.add(dedup_key)
            node.session._must_gather_collected_fixtures = collected

        _data_collector_path = Path(
            f"{node.session.config.getoption('data_collector_path')}/{sanitize_test_name_for_path(node.name)}"
        )
        plan = None
        if hasattr(node, "cls") and node.cls and hasattr(node.cls, "plan_resource"):
            plan_obj = node.cls.plan_resource
            if plan_obj:
                plan = {"name": plan_obj.name, "namespace": plan_obj.namespace}

        run_must_gather(data_collector_path=_data_collector_path, plan=plan)

Files to change

File Change
conftest.py Add setup-error dedup in pytest_exception_interact

Effort estimate

~30 minutes

Done

  • Add setup-error dedup logic to pytest_exception_interact in conftest.py
  • Verify: class fixture failure → 1 must-gather (not N)
  • Verify: individual test call failures still collect must-gather independently
  • Verify: @pytest.mark.incremental behavior unchanged (xfail skips still don't trigger must-gather)

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions