Skip to content

tests: show pytest xfail/xpass results in analyze.py - #23026

Open
donaldsharp wants to merge 1 commit into
FRRouting:masterfrom
donaldsharp:make_analyze_smarter
Open

tests: show pytest xfail/xpass results in analyze.py#23026
donaldsharp wants to merge 1 commit into
FRRouting:masterfrom
donaldsharp:make_analyze_smarter

Conversation

@donaldsharp

Copy link
Copy Markdown
Member

Topotests now use pytest.mark.xfail, which the pytest framework records as skips (expected fail) or failures (strict unexpected pass). Let's teach analyze.py to classify those separately, include known failures in the default listing, and print aligned output to make it more pleasing to the eye.

Old output:

analyze.py -Ar run_save
bgp_batch_clearing/test_bgp_batch_clearing.py::test_link_down_clears_routes

New output shows this:

analyze.py -Ar run_save
0: Known Failure : bgp_rfc7606_treat_as_withdraw/test_bgp_rfc7606_treat_as_withdraw.py::test_treat_as_withdraw[ibgp-aspath-as-zero]
1: Failure : bgp_batch_clearing/test_bgp_batch_clearing.py::test_link_down_clears_routes

Topotests now use pytest.mark.xfail, which the pytest framework records as skips
(expected fail) or failures (strict unexpected pass).  Let's teach analyze.py
to classify those separately, include known failures in the default
listing, and print aligned output to make it more pleasing to the eye.

Old output:

analyze.py -Ar run_save
bgp_batch_clearing/test_bgp_batch_clearing.py::test_link_down_clears_routes

New output shows this:

analyze.py -Ar run_save
0: Known Failure  : bgp_rfc7606_treat_as_withdraw/test_bgp_rfc7606_treat_as_withdraw.py::test_treat_as_withdraw[ibgp-aspath-as-zero]
1: Failure        : bgp_batch_clearing/test_bgp_batch_clearing.py::test_link_down_clears_routes

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
@frrbot frrbot Bot added the tests Topotests, make check, etc label Aug 13, 2026
@greptile-apps

greptile-apps Bot commented Aug 13, 2026

Copy link
Copy Markdown

Greptile Summary

This PR extends topotest JUnit analysis to distinguish expected failures and strict unexpected passes, includes known failures by default, and aligns status output.

  • Adds reusable testcase outcome classifiers and XML-node helpers.
  • Splits XFAIL and XPASS counts from aggregate skip and failure totals.
  • Adds XFAIL/XPASS selection letters and status labels.
  • Changes ordinary listing output to include ordinals, which breaks existing CI parsing.

Confidence Score: 4/5

This PR should not merge until the default listing preserves testcase names in the field consumed by the CI rerun workflow.

The new unconditional ordinal prefix causes existing cut -f1 -d: consumers to pass numeric ordinals rather than testcase paths to pytest.

Files Needing Attention: tests/topotests/analyze.py

Important Files Changed

Filename Overview
tests/topotests/analyze.py Adds XFAIL/XPASS classification and formatting, but the merged listing branch breaks CI consumers by prefixing default output with ordinals.
Prompt To Fix All With AI
### Issue 1
tests/topotests/analyze.py:524-529
**Ordinal prefix breaks CI reruns**

When CI invokes `analyze.py` without `--enumerate`, this merged branch places an ordinal before the first colon. The existing `cut -f1 -d:` consumers therefore pass numbers such as `0` instead of testcase paths to pytest, preventing the selected tests from being rerun.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "tests: show pytest xfail/xpass results i..." | Re-trigger Greptile

Comment on lines +524 to +529
if args.enumerate or (args.test is None and count == 0 and not args.time):
# print the selected test names with ordinal (usable with --test)
print(
"\n".join(
"{}: {}".format(i, testcase_label(name, found_files[name]))
for i, name in enumerate(found_files)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Ordinal prefix breaks CI reruns

When CI invokes analyze.py without --enumerate, this merged branch places an ordinal before the first colon. The existing cut -f1 -d: consumers therefore pass numbers such as 0 instead of testcase paths to pytest, preventing the selected tests from being rerun.

Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/topotests/analyze.py
Line: 524-529

Comment:
**Ordinal prefix breaks CI reruns**

When CI invokes `analyze.py` without `--enumerate`, this merged branch places an ordinal before the first colon. The existing `cut -f1 -d:` consumers therefore pass numbers such as `0` instead of testcase paths to pytest, preventing the selected tests from being rerun.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

@riw777 riw777 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

master size/L tests Topotests, make check, etc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants