tests: fix grpc topotest xdist collection mismatch in CI

[Jafaral]

Fix intermittent CI topotest failures where pytest-xdist aborts with Different tests were collected between gw0 and gw5 because grpc_basic was collected on some workers but module-skipped on others.

Example CI failure here

[greptile-apps]

Greptile Summary

This PR fixes intermittent pytest-xdist CI failures where workers collected grpc_basic differently because the old skip guard ran grpc-query.py --check as a subprocess — a non-deterministic operation that could succeed on some workers and fail on others. The fix replaces that subprocess call with a deterministic filesystem check for grpc.so, and separately filters xdist worker-id entries from analyze.py XML parsing.

• test_basic_grpc.py: Adds _frr_grpc_module_available() which globs standard system paths and an optional FRR_BUILD_DIR for grpc.so; the commander.cmd_raises skip guard is replaced with this file-existence check, eliminating per-worker variance during collection.
• analyze.py: Introduces XDIST_WORKER_RE = re.compile(r"^gw\d+$") and skips any testcase whose @name matches a worker ID, preventing downstream name-processing errors on xdist-injected error entries in JUnit XML output.

Confidence Score: 5/5

Safe to merge — both changes are narrowly scoped to CI test infrastructure and do not touch any FRR daemon or production code.

The subprocess-to-filesystem swap removes the sole source of non-determinism that caused xdist mismatches; file existence is evaluated identically on every worker. The analyze.py regex guard is correct and only skips entries that were previously causing downstream name-processing errors. No functional test logic was altered.

No files require special attention.

Important Files Changed

Filename	Overview
tests/topotests/grpc_basic/test_basic_grpc.py	Replaces non-deterministic subprocess-based skip check with a deterministic filesystem glob search for grpc.so, fixing xdist collection mismatch; also removes unused commander import.
tests/topotests/analyze.py	Adds XDIST_WORKER_RE regex to filter out xdist worker-id entries (e.g. gw5) that appear as @name in JUnit XML when collection errors occur, preventing analysis failures on those entries.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[pytest-xdist worker: collection phase] --> B{grpc / grpc_tools\nPython imports OK?}
    B -- No --> C[pytest.skip: gRPC modules not installed\nallow_module_level=True]
    B -- Yes --> D{_frr_grpc_module_available?\nglob grpc.so in system paths\n+ FRR_BUILD_DIR}
    D -- No --> E[pytest.skip: FRR gRPC northbound\nmodule not installed\nallow_module_level=True]
    D -- Yes --> F[Collect & run grpc_basic tests]

    style C fill:#f88,stroke:#c00
    style E fill:#f88,stroke:#c00
    style F fill:#8f8,stroke:#080

Loading

_{Reviews (3): Last reviewed commit: "tests: skip grpc_basic when FRR gRPC mod..." | Re-trigger Greptile}

[Jafaral]

@greptile

Removing the --check guard in test_basic_grpc.py trades a clean module-level skip for a runtime error if proto generation ever fails in an environment where grpc/grpc_tools are importable but frr-northbound.proto is missing — those cases would now surface as test failures rather than skips.

The runtime-vs-skip tradeoff is acceptable: CI Docker always builds with --enable-grpc, and a proto/protoc problem there should fail as a normal test error, not a per-worker collection skip that aborts the whole xdist run.

For local runs without --enable-grpc: if grpcio/grpcio-tools aren’t installed, the module still skips via the import guard (unchanged since ea85c7c). The removed --check never verified the FRR build anyway — only Python proto generation. A deterministic skip based on GRPC_TRUE / grpc.so would be a nice follow-up for devs who have pip grpc but don’t enable gRPC in the build.

If you have better ideas or code suggestions, let me know.

[greptile-apps]

The rationale is sound. The key insight is that --check runs protoc to regenerate the proto modules, and whether that succeeds is environment-dependent (proto file path, protoc binary availability) — making it a flaky, worker-local check that's the root cause of the xdist collection mismatch.

Your analysis of the tradeoffs is correct:

1. CI (Docker, --enable-grpc): proto generation happens at build time; if it fails there, a test error is the right signal.
2. Local without --enable-grpc: the grpcio/grpcio-tools import guard still fires deterministically before collection, so the module skips cleanly on all workers.

The one gap you identified — a dev with pip-installed grpc but a non-grpc FRR build — is real but narrow. A clean follow-up would be to check for grpc.so or a build flag at the session level rather than per-module. Something like a conftest.py hook:

# tests/topotests/conftest.py
def pytest_ignore_collect(collection_path, config):
    if "grpc_basic" in str(collection_path) and not _frr_built_with_grpc():
        return True

where _frr_built_with_grpc() checks for grpc.so or runs frr-config --grpc. Since pytest_ignore_collect is called on the collection master before workers fork, the decision is made once and is consistent across all xdist workers — safe by construction.

But that's genuinely a follow-up. The current fix correctly removes the per-worker non-determinism, and the tradeoff is well-reasoned.

[riw777]

looks good

[Jafaral]

@greptile review

[Jafaral]

@greptile review

[Jafaral]

@Mergifyio backport stable/10.6

[mergify]

backport stable/10.6

✅ Backports have been created

Details

• #22193 tests: fix grpc topotest xdist collection mismatch in CI (backport #22048) has been created for branch stable/10.6 but encountered conflicts

Cherry-pick of 54429f5 has failed:

On branch mergify/bp/stable/10.6/pr-22048
Your branch is up to date with 'origin/stable/10.6'.

You are currently cherry-picking commit 54429f53c.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   tests/topotests/grpc_basic/test_basic_grpc.py

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally