[conditional_mark] Skip decap/test_decap.py on Arista-720DT and remove stale parametrize-keyed entries#24626
Merged
Xichen96 merged 1 commit intoMay 15, 2026
Conversation
…tale parametrize-keyed entries Add `'Arista-720DT' in hwsku` to the top-level `decap/test_decap.py:` skip block, and remove eight `decap/test_decap.py::test_decap[ttl=*, dscp=*, vxlan=*]:` entries that have been dead since PR sonic-net#20304. Why --- 1. Arista-720DT (TD3-X2 / BCM56873) does not honor `SAI_TUNNEL_DSCP_MODE_UNIFORM_MODEL`. SONiC programs CONFIG_DB, APPL_DB, ASIC_DB, and sairedis with `dscp_mode=uniform` correctly, but the chip clears the inner DSCP regardless of mode. Arista has acknowledged this as a platform-level limitation and granted a concession in `aristanetworks/sonic-qual.msft#1176`. 2. Post-PR sonic-net#20304 `tests/decap/test_decap.py::test_decap` is no longer parametrized at collection time. The `supported_ttl_dscp_params` fixture returns a single `{ttl, dscp, vxlan}` dict at runtime based on the DUT's APP_DB / ASIC heuristics. As a result, pytest collects exactly one item per session (`decap/test_decap.py::test_decap`, with no parametrize suffix). All `decap/test_decap.py::test_decap[ttl=*, dscp=*, vxlan=*]:` entries in `tests_mark_conditions.yaml` match zero items and have been doing so since 2025-08-28. The skip belongs on the top-level `decap/test_decap.py:` rule, which is the only rule that pytest's conditional_mark plugin still matches against the single collected `test_decap` item. Verification ------------ Empirically verified on `testbed-bjw2-can-720dt-6` (m0, internal-202511): * Baseline (no patch): pytest reports `1 error in 8.56s` (`test_decap` proceeds past conditional_mark and errors at the `duthosts` fixture). * With this patch: pytest reports `1 skipped, 9 warnings in 2.98s` -- `test_decap` is marked SKIPPED by conditional_mark before any fixture fires. * Removing the eight stale parametrize-keyed entries did not change the behaviour vs adding the top-level skip alone (they were already matching nothing). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Xichen Lin <lukelin0907@gmail.com>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
github-actions
Bot
requested review from
BYGX-wcr,
xwjiang-ms and
yutongzhang-microsoft
Xichen96
added
the
Request for 202511 branch
ZhaohuiS
approved these changes
May 15, 2026
Contributor
ZhaohuiS
left a comment
ZhaohuiS
left a comment
There was a problem hiding this comment.
Well-documented change. The conditions_logical_operator: or is correctly added for the two independent skip triggers (topo vs hwsku), and the 8 stale parametrize-keyed entries have been dead since #20304 refactored test_decap into a single non-parametrized item. The three-way verification (baseline / full PR / skip-only sub-patch) in the description confirms the stale-entry removal is purely cosmetic.
Collaborator
|
@Xichen96 PR conflicts with 202511 branch |
Blueve
pushed a commit
that referenced
this pull request
May 15, 2026
## Description of PR Remove the four orphaned pytest CLI options registered in `tests/decap/__init__.py`: - `--ttl_uniform` - `--dscp_uniform` - `--no_ttl_uniform` - `--no_dscp_uniform` ## Why PR #20304 refactored `tests/decap/conftest.py`. It removed `pytest_generate_tests` and `build_ttl_dscp_params`, and replaced them with the `supported_ttl_dscp_params` fixture, which returns a single `{ttl, dscp, vxlan}` dict at runtime based on the DUT's APP_DB / ASIC heuristics. That refactor left the four `--*_uniform` options registered with no consumer. The companion conditional_mark cleanup PR (#24626) removes the parametrize-keyed entries these options used to drive. A grep across `origin/master` and `origin/202511` confirms that no file in the public sonic-mgmt repo references `--ttl_uniform`, `--dscp_uniform`, `--no_ttl_uniform`, or `--no_dscp_uniform` outside the `addoption` block itself. Removing the dead options keeps the test surface honest. The `--outer_ipv4` / `--outer_ipv6` / `--inner_ipv4` / `--inner_ipv6` options are still in use and remain. ## Approach Pure deletion in `tests/decap/__init__.py` — −25 lines, no additions. ## How to verify it `pytest --collect-only tests/decap/test_decap.py` still collects exactly **one** item (`decap/test_decap.py::test_decap`), identical to before. After this change, passing `--no_dscp_uniform` will be rejected by pytest as `unrecognized arguments`, surfacing dead invocations rather than silently doing nothing. ## Related - Refactor that orphaned the flags: **#20304** - Companion conditional_mark cleanup (skip on Arista-720DT + remove stale parametrize entries): **#24626** - Tracking issue: **aristanetworks/sonic-qual.msft#1176** ## Backports Backports handled by automation after master merges. Signed-off-by: Xichen Lin <lukelin0907@gmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of PR
Two related changes to
tests/common/plugins/conditional_mark/tests_mark_conditions.yamlfordecap/test_decap.py:'Arista-720DT' in hwskuto the top-leveldecap/test_decap.py:skip block.decap/test_decap.py::test_decap[ttl=*, dscp=*, vxlan=*]:.Result: +3 / -63 lines, no behavioural changes for any platform other than
Arista-720DT(which gets skipped).Why (1) — skip on Arista-720DT
Arista-720DT (TD3-X2 / BCM56873) does not honor
SAI_TUNNEL_DSCP_MODE_UNIFORM_MODEL. SONiC programs the entire stack correctly (CONFIG_DB → APPL_DB → ASIC_DB → sairedis.rec all showdscp_mode=uniform), but the chip clears the inner DSCP regardless of the configured mode. Arista has acknowledged this as a platform-level limitation and granted a concession in aristanetworks/sonic-qual.msft#1176.Why (2) — remove stale entries
PR #20304 (2025-08-28) refactored
tests/decap/conftest.py: it removedpytest_generate_tests+build_ttl_dscp_paramsand replaced them with thesupported_ttl_dscp_paramsfixture, which returns a single{ttl, dscp, vxlan}dict per session based on the DUT's APP_DB / ASIC heuristics.Since that refactor,
pytest --collect-only decap/test_decap.pycollects exactly one item:decap/test_decap.py::test_decap— no parametrize suffix. The eight[ttl=*, dscp=*, vxlan=*]entries intests_mark_conditions.yamlhave therefore been matching zero items for the last ~9 months. Removing them is a pure cleanup; git history preserves them.The only rule that pytest's conditional_mark plugin can match against the single collected
test_decapitem is the top-leveldecap/test_decap.py:rule, which is why the new skip belongs there.How to verify it
Empirically verified on
testbed-bjw2-can-720dt-6(m0, internal-202511 mgmt +SONiC.20251110.26):1 error in 8.56s(test_decapproceeds past conditional_mark and errors atduthostsfixture — nothing matched)1 skipped, 9 warnings in 2.98s—test_decapis marked SKIPPED before any fixture fires1 skipped, 9 warnings in 2.98s— confirms removing the stale entries has zero behavioural effectRelated
Backports
Backports handled by automation after master merges.