Skip to content

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

Merged
StormLiangMS merged 2 commits into
sonic-net:masterfrom
Xichen96:dev/xichenlin/skip-720dt-warm-reboot
Jul 21, 2026
Merged

[route]: Skip warm reboot tests on m0/m1/mx#26340
StormLiangMS merged 2 commits into
sonic-net:masterfrom
Xichen96:dev/xichenlin/skip-720dt-warm-reboot

Conversation

@Xichen96

@Xichen96 Xichen96 commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

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
  • Test case improvement

Back port request

  • 202311
  • 202405
  • 202411
  • 202505
  • 202511
  • 202512
  • 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

  • master
  • 202311
  • 202405
  • 202411
  • 202505
  • 202511
  • 202512
  • 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 completed with 3 skipped, 0 failures, exit 0 in 43.88 seconds. Each case reported Warm reboot is not supported on dualtor, m0, m1, or mx topologies.
  • 202605 prerequisite: the backport must also include conditional-mark node-ID normalization fix [conditional_mark] Fix nodeid normalization so skips apply when rootdir floats above tests/ #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. On 202605, ran the three cases through tests/run_tests.sh with the required #25930 prerequisite; all three were conditionally skipped.

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.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b8fb8259-87f3-4de3-b1a4-59fe52e8be86
Signed-off-by: Xichen96 <lukelin0907@gmail.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

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

@github-actions

Copy link
Copy Markdown

@StormLiangMS @wangxin @yxieca A user wants to merge changes to the conditional mark files into master. Please review.

@azure-pipelines

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the conditional-mark rules to skip static-route warm reboot tests on Arista-720DT platforms, aligning test applicability with the platform’s lack of warm reboot support.

Changes:

  • Extend the existing skip conditions for three *_warmboot static-route tests to also match HWSKUs containing Arista-720DT.
  • Set conditions_logical_operator: OR so the skip applies to either dualtor topologies or Arista-720DT hardware.

@mssonicbld

Copy link
Copy Markdown
Collaborator

This PR has backport request for branch(es): 202605.
Added label(s) for branch(es) 202605.

---Powered by SONiC BuildBot

@StormLiangMS StormLiangMS left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@Xichen96 — the skip change is correctly scoped.

The existing dual-ToR exclusion is preserved with OR, and only the three warm-reboot static-route cases are additionally skipped when the HWSKU contains Arista-720DT; non-warm-reboot coverage remains enabled.

The failing t1-lag-vpp check is unrelated: it failed in arp/test_neighbor_mac.py on the VPP platform, where the new 720DT condition is false. No code blocker found; the unrelated CI failure still needs a rerun or waiver before merge.

@StormLiangMS

Copy link
Copy Markdown
Collaborator

hi @Xichen96 I would assume warm reboot should be skipped on m0/m1/mx devices other than specific HWSKU?

Broaden the three static-route warm-reboot conditional skips from Arista-720DT to every m0, m1, and mx topology while retaining the existing dualtor exclusions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b8fb8259-87f3-4de3-b1a4-59fe52e8be86
Signed-off-by: Xichen96 <lukelin0907@gmail.com>
Copilot AI review requested due to automatic review settings July 21, 2026 10:32
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@Xichen96 Xichen96 changed the title [route]: Skip warm reboot tests on 720dt [route]: Skip warm reboot tests on m0/m1/mx Jul 21, 2026
@azure-pipelines

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@StormLiangMS
StormLiangMS merged commit 2d312af into sonic-net:master Jul 21, 2026
26 checks passed
@mssonicbld

Copy link
Copy Markdown
Collaborator

Cherry-pick PR to 202605: #26417

mssonicbld added a commit that referenced this pull request Jul 23, 2026
…ips apply when rootdir floats above tests/ (#26518)

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
- [x] Test case enhancement
- [ ] Skipped for non-supported platforms

### Back port request

- [ ] 202305
- [ ] 202311
- [ ] 202405
- [ ] 202411
- [ ] 202505
- [x] 202605

### Tested branch

- [x] master
- [ ] 202305
- [ ] 202311
- [ ] 202405
- [ ] 202411
- [ ] 202505
- [x] 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
#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
- [x] No

Signed-off-by: Nikhil Hegde <nikhilhegde@microsoft.com>
Signed-off-by: mssonicbld <sonicbld@microsoft.com>
Co-authored-by: nhegde-microsoft <nikhilhegde@microsoft.com>
@Xichen96
Xichen96 deleted the dev/xichenlin/skip-720dt-warm-reboot branch July 23, 2026 17:15
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.

4 participants