[conditional_mark] Fix nodeid normalization so skips apply when rootdir floats above tests/ by nhegde-microsoft · Pull Request #25930 · sonic-net/sonic-mgmt

nhegde-microsoft · 2026-07-06T21:34:40Z

ADO: https://msazure.visualstudio.com/One/_workitems/edit/38913902

Description of PR

conditional_mark matches each test's item.nodeid against the tests-mark-conditions files, whose keys are relative to the tests/ directory (e.g. platform_tests/api). Before matching, pytest_collection_modifyitems normalizes the nodeid by stripping basename(rootpath) + "/" (added for pytest 9.0+ in #22944).

That normalization breaks when pytest's rootdir is not the tests/ directory. When the harness is invoked with a path argument outside tests/ (e.g. run_tests.sh passes --inventory ../ansible/veos_vtb), pytest 9.0 floats rootdir up to the sonic-mgmt checkout root, so item.nodeid becomes tests/platform_tests/api/.... root_prefix is then basename(<checkout>)/, which does not prefix tests/...; the strip no-ops, the tests/ survives, and every nodeid.startswith(<key>) fails. Result: no conditional mark is applied at all (Found match: 0, skipped: 0).

Practically, on a virtual-switch (asic_type: vs) DUT the platform_tests/api: skip if asic_type in ['vs'] rule stops firing, so those tests run and error with No module named 'sonic_platform' instead of skipping with "Unsupported platform API".

This PR adds a second normalization step: after stripping basename(rootpath), also strip a leading tests/ so nodeids match the tests/-relative rule keys regardless of where pytest resolved rootdir.

Type of change

Back port request

Tested branch

Test result

master: on a virtual-switch (asic_type: vs) testbed through run_tests.sh, the platform API cases changed from errors to the expected conditional skips/xfails.
202605: testbed-bjw-can-720dt-3 (m0, Arista-720DT-G48S4) on SONiC.20260510.06; with this node-ID normalization change and the [route]: Skip warm reboot tests on m0/m1/mx #26340 conditional marks applied to the 202605 test branch, targeted tests/run_tests.sh completed with 3 skipped, 0 failures, exit 0 in 43.88 seconds. This standard-runner path previously retained the leading tests/ and bypassed the marks.

Approach

What is the motivation for this PR?

Conditional skips/xfails are silently ignored whenever pytest's rootdir resolves above tests/ (which happens with the standard run_tests.sh invocation that passes ../-relative paths). This causes false ERRORs for tests that are supposed to skip — most visibly the platform_tests/api suite on virtual-switch DUTs, but it affects every conditional rule in that run mode.

How did you do it?

In pytest_collection_modifyitems, after the existing basename(rootpath) strip, also strip a leading tests/ from the nodeid before matching against the condition keys.

How did you verify/test it?

On a virtual-switch (asic_type: vs) testbed via run_tests.sh (which passes --inventory ../ansible/veos_vtb, i.e. the rootdir-floats-up case):

Test	Before	After
`platform_tests/api/test_watchdog.py`	8 errors (`No module named 'sonic_platform'`)	8 skipped ("Unsupported platform API")
`platform_tests/api/test_psu.py`	errors	14 skipped ("Unsupported platform API")
`platform_tests/sfp/test_sfputil.py`	errors	`asic_type in ['vs']` xfail rule fires (4 xfailed)

The sfp case exercises a different conditions file and an xfail (not skip) rule, confirming the fix restores conditional-mark matching globally rather than only for platform_tests/api.

flake8 --max-line-length=120 clean.

The 202605 target-branch validation used the standard test runner with the backported normalization change and confirmed that all three #26340 warm-reboot conditions matched and skipped before entering the reboot path.

Documentation

Yes
No

mssonicbld · 2026-07-06T21:34:48Z

/azp run

azure-pipelines · 2026-07-06T21:35:03Z

Azure Pipelines successfully started running 1 pipeline(s).

…ir floats up conditional_mark's rule keys are relative to the tests/ directory (e.g. "platform_tests/api"). To match them, pytest_collection_modifyitems normalizes each item.nodeid by stripping basename(rootpath)+"/" (added for pytest 9.0+ in PR sonic-net#22944). That normalization is insufficient when pytest's rootdir is not the tests/ directory. When the test harness is invoked with a path argument that lives outside tests/ (for example `--inventory ../ansible/veos_vtb`, as run_tests.sh does), pytest 9.0 floats rootdir up to the sonic-mgmt checkout root. item.nodeid then carries a leading "tests/" segment ("tests/platform_tests/api/..."), while root_prefix is basename(checkout)+"/", which does not prefix it. The strip no-ops, the "tests/" survives, and every `nodeid.startswith(<key>)` comparison fails -> no conditional mark is ever applied (Found match: 0, skipped: 0). The practical effect is that all conditional skips/xfails silently stop firing in that invocation mode. For example on a virtual-switch (asic_type: vs) DUT the `platform_tests/api: skip if asic_type in ['vs']` rule no longer applies, so the platform-API tests run and error with "No module named 'sonic_platform'" instead of skipping ("Unsupported platform API"). Fix: after the existing basename(rootpath) strip, also strip a leading "tests/" so nodeids match the tests/-relative rule keys regardless of where pytest resolved rootdir. Verified on a virtual-switch testbed: platform_tests/api/test_watchdog.py goes from 8 errors to 8 skipped, platform_tests/api/test_psu.py to 14 skipped (all "Unsupported platform API"), and platform_tests/sfp/test_sfputil.py's `asic_type in ['vs']` xfail rule fires as expected. Signed-off-by: Nikhil Hegde <nikhilhegde@microsoft.com>

mssonicbld · 2026-07-06T22:52:00Z

/azp run

azure-pipelines · 2026-07-06T22:52:15Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Nikhil Hegde <nikhilhegde@microsoft.com>

mssonicbld · 2026-07-06T23:51:15Z

/azp run

azure-pipelines · 2026-07-06T23:51:30Z

Azure Pipelines successfully started running 1 pipeline(s).

nhegde-microsoft · 2026-07-07T21:21:58Z

/azp run

azure-pipelines · 2026-07-07T21:22:14Z

Azure Pipelines successfully started running 1 pipeline(s).

…ir floats above tests/ (sonic-net#25930) Fixes sonic-net#25929 ### Description of PR `conditional_mark` matches each test's `item.nodeid` against the tests-mark-conditions files, whose keys are relative to the `tests/` directory (e.g. `platform_tests/api`). Before matching, `pytest_collection_modifyitems` normalizes the nodeid by stripping `basename(rootpath) + "/"` (added for pytest 9.0+ in sonic-net#22944). That normalization breaks when pytest's `rootdir` is not the `tests/` directory. When the harness is invoked with a path argument outside `tests/` (e.g. `run_tests.sh` passes `--inventory ../ansible/veos_vtb`), pytest 9.0 floats `rootdir` up to the sonic-mgmt checkout root, so `item.nodeid` becomes `tests/platform_tests/api/...`. `root_prefix` is then `basename(<checkout>)/`, which does not prefix `tests/...`; the strip no-ops, the `tests/` survives, and every `nodeid.startswith(<key>)` fails. Result: **no conditional mark is applied at all** (`Found match: 0`, `skipped: 0`). Practically, on a virtual-switch (`asic_type: vs`) DUT the `platform_tests/api: skip if asic_type in ['vs']` rule stops firing, so those tests run and error with `No module named 'sonic_platform'` instead of skipping with "Unsupported platform API". This PR adds a second normalization step: after stripping `basename(rootpath)`, also strip a leading `tests/` so nodeids match the `tests/`-relative rule keys regardless of where pytest resolved `rootdir`. #### Type of change - [ ] Bug fix - [ ] Testbed and Framework (new/improvement) - [ ] New Test case - [x] Test case enhancement - [ ] Skipped for non-supported platforms ### Back port request - [ ] 202305 - [ ] 202311 - [ ] 202405 - [ ] 202411 - [ ] 202505 ### Approach #### What is the motivation for this PR? Conditional skips/xfails are silently ignored whenever pytest's `rootdir` resolves above `tests/` (which happens with the standard `run_tests.sh` invocation that passes `../`-relative paths). This causes false ERRORs for tests that are supposed to skip — most visibly the `platform_tests/api` suite on virtual-switch DUTs, but it affects every conditional rule in that run mode. #### How did you do it? In `pytest_collection_modifyitems`, after the existing `basename(rootpath)` strip, also strip a leading `tests/` from the nodeid before matching against the condition keys. #### How did you verify/test it? On a virtual-switch (`asic_type: vs`) testbed via `run_tests.sh` (which passes `--inventory ../ansible/veos_vtb`, i.e. the rootdir-floats-up case): | Test | Before | After | | --- | --- | --- | | `platform_tests/api/test_watchdog.py` | 8 errors (`No module named 'sonic_platform'`) | **8 skipped** ("Unsupported platform API") | | `platform_tests/api/test_psu.py` | errors | **14 skipped** ("Unsupported platform API") | | `platform_tests/sfp/test_sfputil.py` | errors | `asic_type in ['vs']` **xfail** rule fires (4 xfailed) | The `sfp` case exercises a different conditions file and an `xfail` (not `skip`) rule, confirming the fix restores conditional-mark matching globally rather than only for `platform_tests/api`. `flake8 --max-line-length=120` clean. ### Documentation - [ ] Yes - [x] No --------- Signed-off-by: Nikhil Hegde <nikhilhegde@microsoft.com> Signed-off-by: selldinesh <dinesh.sellappan@keysight.com>

…ir floats above tests/ (sonic-net#25930) Fixes sonic-net#25929 ### Description of PR `conditional_mark` matches each test's `item.nodeid` against the tests-mark-conditions files, whose keys are relative to the `tests/` directory (e.g. `platform_tests/api`). Before matching, `pytest_collection_modifyitems` normalizes the nodeid by stripping `basename(rootpath) + "/"` (added for pytest 9.0+ in sonic-net#22944). That normalization breaks when pytest's `rootdir` is not the `tests/` directory. When the harness is invoked with a path argument outside `tests/` (e.g. `run_tests.sh` passes `--inventory ../ansible/veos_vtb`), pytest 9.0 floats `rootdir` up to the sonic-mgmt checkout root, so `item.nodeid` becomes `tests/platform_tests/api/...`. `root_prefix` is then `basename(<checkout>)/`, which does not prefix `tests/...`; the strip no-ops, the `tests/` survives, and every `nodeid.startswith(<key>)` fails. Result: **no conditional mark is applied at all** (`Found match: 0`, `skipped: 0`). Practically, on a virtual-switch (`asic_type: vs`) DUT the `platform_tests/api: skip if asic_type in ['vs']` rule stops firing, so those tests run and error with `No module named 'sonic_platform'` instead of skipping with "Unsupported platform API". This PR adds a second normalization step: after stripping `basename(rootpath)`, also strip a leading `tests/` so nodeids match the `tests/`-relative rule keys regardless of where pytest resolved `rootdir`. #### Type of change - [ ] Bug fix - [ ] Testbed and Framework (new/improvement) - [ ] New Test case - [x] Test case enhancement - [ ] Skipped for non-supported platforms ### Back port request - [ ] 202305 - [ ] 202311 - [ ] 202405 - [ ] 202411 - [ ] 202505 ### Approach #### What is the motivation for this PR? Conditional skips/xfails are silently ignored whenever pytest's `rootdir` resolves above `tests/` (which happens with the standard `run_tests.sh` invocation that passes `../`-relative paths). This causes false ERRORs for tests that are supposed to skip — most visibly the `platform_tests/api` suite on virtual-switch DUTs, but it affects every conditional rule in that run mode. #### How did you do it? In `pytest_collection_modifyitems`, after the existing `basename(rootpath)` strip, also strip a leading `tests/` from the nodeid before matching against the condition keys. #### How did you verify/test it? On a virtual-switch (`asic_type: vs`) testbed via `run_tests.sh` (which passes `--inventory ../ansible/veos_vtb`, i.e. the rootdir-floats-up case): | Test | Before | After | | --- | --- | --- | | `platform_tests/api/test_watchdog.py` | 8 errors (`No module named 'sonic_platform'`) | **8 skipped** ("Unsupported platform API") | | `platform_tests/api/test_psu.py` | errors | **14 skipped** ("Unsupported platform API") | | `platform_tests/sfp/test_sfputil.py` | errors | `asic_type in ['vs']` **xfail** rule fires (4 xfailed) | The `sfp` case exercises a different conditions file and an `xfail` (not `skip`) rule, confirming the fix restores conditional-mark matching globally rather than only for `platform_tests/api`. `flake8 --max-line-length=120` clean. ### Documentation - [ ] Yes - [x] No --------- Signed-off-by: Nikhil Hegde <nikhilhegde@microsoft.com> Signed-off-by: ssithaia-ebay <ssithaian@ebay.com>

### Description of PR Summary: Skip `test_static_route_warmboot`, `test_static_route_ipv6_warmboot`, and `test_static_route_ecmp_warmboot` on all m0, m1, and mx topologies through centralized conditional marks. Preserve the existing dualtor exclusions. ADO: https://msazure.visualstudio.com/One/_workitems/edit/38862000 ### Type of change - [ ] Bug fix - [ ] Testbed and Framework(new/improvement) - [ ] New Test case - [ ] Skipped for non-supported platforms - [x] Test case improvement ### Back port request - [ ] 202311 - [ ] 202405 - [ ] 202411 - [ ] 202505 - [ ] 202511 - [ ] 202512 - [x] 202605 Tracking issue/work item for backport/cherry-pick request (GitHub issue or Microsoft ADO): https://msazure.visualstudio.com/One/_workitems/edit/38862000 Failure type: day-one topology applicability gap ### Tested branch - [x] master - [ ] 202311 - [ ] 202405 - [ ] 202411 - [ ] 202505 - [ ] 202511 - [ ] 202512 - [x] 202605 - [ ] N/A ### Test result - master: the conditional-mark YAML loads successfully; direct condition evaluation verifies all three cases skip for m0, m1, mx, and dualtor, while remaining runnable on t1. - 202605: `testbed-bjw-can-720dt-3` (m0, `Arista-720DT-G48S4`) on `SONiC.20260510.06`; targeted `tests/run_tests.sh` validation is being refreshed for the topology-wide condition. - 202605 prerequisite: the backport must also include conditional-mark node-ID normalization fix #25930 (`3cf07f4b6bf3`). Without it, standard `run_tests.sh` can produce `tests/...` node IDs and silently bypass centralized conditional marks. - Nightly evidence: https://elastictest.org/scheduler/testplan/6a5b5cb0b2ac8cdc8af95b7d reported the three warm-reboot cases failing across all six 720DT mx testbeds. ### Approach #### What is the motivation for this PR? Recent m0/mx nightlies reported failures for the three static-route warm-reboot tests. Warm-reboot coverage is not applicable to m0, m1, or mx topologies, so these cases should not enter the reboot path on those topologies. #### How did you do it? Extended each existing dualtor conditional skip with an OR condition matching `topo_type in ['m0', 'm1', 'mx']`. #### How did you verify/test it? Reviewed the recent 720DT m0/mx failures and confirmed these are the three logical cases that invoke warm reboot. Validated the YAML and evaluated the conditions for m0, m1, mx, dualtor, and t1. The 202605 physical validation uses the standard test runner with the required #25930 prerequisite. #### Any platform specific information? The condition is topology-based and applies to every platform running m0, m1, or mx. #### Supported testbed topology if it's a new test case? Not a new test case. The new exclusions are m0, m1, and mx; the existing dualtor exclusions remain. ### Documentation Not applicable. --------- Signed-off-by: Xichen96 <lukelin0907@gmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

mssonicbld · 2026-07-23T03:09:56Z

Cherry-pick PR to msft-202608: Azure/sonic-mgmt.msft#1328

mssonicbld · 2026-07-23T06:18:40Z

This PR has backport request label(s) for branch(es): msft-202608, but is missing required test information. Please make sure you tick the tested branch(es) in the Tested branch section and provide test evidence (e.g., 202608: <test result>) in the Test result section as well in your PR description.

_{---Powered by SONiC BuildBot}

mssonicbld · 2026-07-23T09:09:10Z

Cherry-pick PR to 202605: #26518

github-actions Bot requested review from BYGX-wcr, xwjiang-ms and yutongzhang-microsoft July 6, 2026 21:35

nhegde-microsoft requested a review from lguohan July 6, 2026 21:38

nhegde-microsoft added bug csonic Issues related to cSONiC (docker-sonic-vs) neighbors in KVM testbed labels Jul 6, 2026

nhegde-microsoft force-pushed the csonic-fix/conditional-mark-nodeid-tests-prefix branch from 0d9f535 to 404f943 Compare July 6, 2026 22:51

nhegde-microsoft marked this pull request as ready for review July 6, 2026 23:39

ci: re-trigger pipeline (transient apt/dpkg lock flake dropped jq)

68d1911

Signed-off-by: Nikhil Hegde <nikhilhegde@microsoft.com>

lguohan approved these changes Jul 8, 2026

View reviewed changes

lguohan merged commit 3cf07f4 into sonic-net:master Jul 8, 2026
26 checks passed

congh-nvidia mentioned this pull request Jul 17, 2026

[SmartSwitch HA] Correct ha skip file paths and move them to the ha section #26292

Open

21 tasks

Xichen96 mentioned this pull request Jul 20, 2026

[route]: Skip warm reboot tests on m0/m1/mx #26340

Merged

21 tasks

mssonicbld mentioned this pull request Jul 21, 2026

[action] [PR:26340] [route]: Skip warm reboot tests on m0/m1/mx #26417

Open

21 tasks

Xichen96 mentioned this pull request Jul 22, 2026

[202605][conditional_mark] Backport nodeid normalization for centralized marks #26452

Closed

21 tasks

ediwibowo-msft added the Request for msft-202608 branch label Jul 23, 2026

yejianquan added the Approved for msft-202608 Branch label Jul 23, 2026

mssonicbld mentioned this pull request Jul 23, 2026

[action] [PR:25930] [conditional_mark] Fix nodeid normalization so skips apply when rootdir floats above tests/ Azure/sonic-mgmt.msft#1328

Merged

12 tasks

mssonicbld added the Created PR to msft-202608 branch label Jul 23, 2026

mssonicbld added Included in msft-202608 branch and removed Created PR to msft-202608 branch labels Jul 23, 2026

Xichen96 added Request for 202605 branch Tested for 202605 branch Tested for 202605 branch Approved for 202605 branch labels Jul 23, 2026

mssonicbld added the Created PR to 202605 branch label Jul 23, 2026

mssonicbld mentioned this pull request Jul 23, 2026

[action] [PR:25930] [conditional_mark] Fix nodeid normalization so skips apply when rootdir floats above tests/ #26518

Merged

21 tasks

mssonicbld added Included in 202605 branch and removed Created PR to 202605 branch labels Jul 23, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[conditional_mark] Fix nodeid normalization so skips apply when rootdir floats above tests/#25930

[conditional_mark] Fix nodeid normalization so skips apply when rootdir floats above tests/#25930
lguohan merged 2 commits into
sonic-net:masterfrom
nhegde-microsoft:csonic-fix/conditional-mark-nodeid-tests-prefix

nhegde-microsoft commented Jul 6, 2026 •

edited by Xichen96

Loading

Uh oh!

mssonicbld commented Jul 6, 2026

Uh oh!

azure-pipelines Bot commented Jul 6, 2026

Uh oh!

mssonicbld commented Jul 6, 2026

Uh oh!

azure-pipelines Bot commented Jul 6, 2026

Uh oh!

mssonicbld commented Jul 6, 2026

Uh oh!

azure-pipelines Bot commented Jul 6, 2026

Uh oh!

nhegde-microsoft commented Jul 7, 2026

Uh oh!

azure-pipelines Bot commented Jul 7, 2026

Uh oh!

Uh oh!

mssonicbld commented Jul 23, 2026

Uh oh!

mssonicbld commented Jul 23, 2026

Uh oh!

mssonicbld commented Jul 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

nhegde-microsoft commented Jul 6, 2026 • edited by Xichen96 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of PR

Type of change

Back port request

Tested branch

Test result

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Documentation

Uh oh!

mssonicbld commented Jul 6, 2026

Uh oh!

azure-pipelines Bot commented Jul 6, 2026

Uh oh!

mssonicbld commented Jul 6, 2026

Uh oh!

azure-pipelines Bot commented Jul 6, 2026

Uh oh!

mssonicbld commented Jul 6, 2026

Uh oh!

azure-pipelines Bot commented Jul 6, 2026

Uh oh!

nhegde-microsoft commented Jul 7, 2026

Uh oh!

azure-pipelines Bot commented Jul 7, 2026

Uh oh!

Uh oh!

mssonicbld commented Jul 23, 2026

Uh oh!

mssonicbld commented Jul 23, 2026

Uh oh!

mssonicbld commented Jul 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

nhegde-microsoft commented Jul 6, 2026 •

edited by Xichen96

Loading