Skip to content

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

Merged
mssonicbld merged 1 commit into
sonic-net:202605from
mssonicbld:cherry/202605/25930
Jul 23, 2026
Merged

[action] [PR:25930] [conditional_mark] Fix nodeid normalization so skips apply when rootdir floats above tests/#26518
mssonicbld merged 1 commit into
sonic-net:202605from
mssonicbld:cherry/202605/25930

Conversation

@mssonicbld

@mssonicbld mssonicbld commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Fixes #25929

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

  • Bug fix
  • Testbed and Framework (new/improvement)
  • New Test case
  • Test case enhancement
  • Skipped for non-supported platforms

Back port request

  • 202305
  • 202311
  • 202405
  • 202411
  • 202505
  • 202605

Tested branch

  • master
  • 202305
  • 202311
  • 202405
  • 202411
  • 202505
  • 202605
  • N/A

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

…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: mssonicbld <sonicbld@microsoft.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator Author

Original PR: #25930

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@Xichen96
Xichen96 requested a review from lguohan July 23, 2026 09:10
@mssonicbld

Copy link
Copy Markdown
Collaborator Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@mssonicbld
mssonicbld merged commit ea1cbf9 into sonic-net:202605 Jul 23, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants