From b64298d62f8e6a87548ca539fe19ddc3edfbe04d Mon Sep 17 00:00:00 2001 From: nhegde-microsoft Date: Wed, 8 Jul 2026 13:53:19 -0500 Subject: [PATCH] [conditional_mark] Fix nodeid normalization so skips apply when rootdir floats above tests/ (#25930) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #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 #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()/`, which does not prefix `tests/...`; the strip no-ops, the `tests/` survives, and every `nodeid.startswith()` 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 Signed-off-by: mssonicbld --- tests/common/plugins/conditional_mark/__init__.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/common/plugins/conditional_mark/__init__.py b/tests/common/plugins/conditional_mark/__init__.py index abf9d7383f8..27df84d999a 100644 --- a/tests/common/plugins/conditional_mark/__init__.py +++ b/tests/common/plugins/conditional_mark/__init__.py @@ -710,12 +710,16 @@ def pytest_collection_modifyitems(session, config, items): json.dumps(basic_facts, indent=2))) dynamic_update_skip_reason = session.config.option.dynamic_update_skip_reason basic_facts['constants'] = MARK_CONDITIONS_CONSTANTS - # Normalize nodeids: strip root directory prefix if present (pytest 9.0+ includes it) + # Normalize nodeids to match the tests/-relative condition keys. rootdir may + # float above tests/ (e.g. --inventory ../ansible/veos_vtb), so strip both + # basename(rootpath) and a leading "tests/" or all conditional skips no-op. root_prefix = os.path.basename(str(session.config.rootpath)) + "/" for item in items: nodeid = item.nodeid if nodeid.startswith(root_prefix): nodeid = nodeid[len(root_prefix):] + if nodeid.startswith("tests/"): + nodeid = nodeid[len("tests/"):] all_matches = find_all_matches(nodeid, conditions, session, dynamic_update_skip_reason, basic_facts) if all_matches: