Skip to content

[conditional_mark]: Skip unsupported FEC stats test on Tomahawk - #28295

Merged
yxieca merged 1 commit into
sonic-net:masterfrom
az-pz:ariz/fix-arista-test-failure
Sep 30, 2026
Merged

yxieca merged 1 commit into
sonic-net:masterfrom
az-pz:ariz/fix-arista-test-failure

Conversation

@az-pz

@az-pz az-pz commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Description of PR

Summary:
Add a conditional skip for layer1/test_fec_error.py::test_verify_fec_stats_counters on Arista-7060CX-32S-D48C8, where Broadcom legacy Tomahawk SAI does not support the FEC_SYMBOL_ERR counter on 50G links.

This restores coverage of the platform limitation documented in #23936 for the test's layer1/ node ID; that PR added the equivalent condition for platform_tests/test_intf_fec.py.

Fixes # (issue): N/A

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): N/A
Failure type: day-one platform limitation

Tested branch

  • master
  • 202311
  • 202405
  • 202411
  • 202505
  • 202511
  • 202512
  • 202605
  • N/A

Test result

master: Ran the layer1/test_fec_error.py test against a device with Arista-7060CX-32S-D48C8 HwSku. The test gets skipped.

=============================================== short test summary info ================================================
SKIPPED [1] layer1/test_fec_error.py: Broadcom TH does not support FEC_SYMBOL_ERR at 50G
=========================================== 1 skipped, 2 warnings in 44.27s ============================================

202605: The test gets skipped as expected.

=============================== short test summary info ================================
SKIPPED [1] layer1/test_fec_error.py: Broadcom TH does not support FEC_SYMBOL_ERR at 50G
=========================== 1 skipped, 2 warnings in 56.84s ============================

Approach

What is the motivation for this PR?

The FEC statistics test fails on Arista-7060CX-32S-D48C8 because Broadcom legacy Tomahawk SAI does not provide FEC_SYMBOL_ERR for 50G links. The existing conditional mark from #23936 only targets the former platform_tests/test_intf_fec.py node ID and therefore does not cover the test under layer1/test_fec_error.py.

How did you do it?

Added the equivalent conditional skip for layer1/test_fec_error.py::test_verify_fec_stats_counters, scoped to the affected HWSKU.

How did you verify/test it?

Ran the layer1/test_fec_error.py test against a device with Arista-7060CX-32S-D48C8 HwSku. The test gets skipped.

Any platform specific information?

This applies to Broadcom legacy Tomahawk on Arista-7060CX-32S-D48C8 at 50G.

Supported testbed topology if it's a new test case?

N/A; this is not a new test case.

Documentation

N/A; this change only updates conditional test metadata.

MSFT ADO: 39182694

Signed-off-by: Ariz Zubair <arizzubair@microsoft.com>
@azure-pipelines

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

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp 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.

Copilot review overview

🟢 Approval recommended

The condition matches the established platform-test exception and correctly targets the relocated test node.

Review effort: Balanced
Findings: None

What changed in this PR

Extends the existing Tomahawk FEC limitation handling from platform_tests to the equivalent layer1 test.

Changes:

  • Skips the FEC statistics test on Arista-7060CX-32S-D48C8.
  • Adds a section header for Layer 1 conditional marks.
File Description
tests/​common/​plugins/​conditional_mark/​tests_mark_conditions.yaml Adds the platform-specific conditional skip for the Layer 1 FEC test.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@mssonicbld

Copy link
Copy Markdown
Collaborator

This PR has backport request label(s) for branch(es): 202605, 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., 202605: <test result>) in the Test result section as well in your PR description.

---Powered by SONiC BuildBot

@yxieca
yxieca added this pull request to the merge queue Sep 30, 2026
Merged via the queue into sonic-net:master with commit 79aa648 Sep 30, 2026
33 checks passed
@mssonicbld mssonicbld added the Tested for 202605 branch Tested for 202605 branch label Oct 1, 2026
@mssonicbld

Copy link
Copy Markdown
Collaborator

The Tested branch section has been ticked and Test result is provided for branch(es): 202605. Added label(s): Tested for 202605 Branch.

---Powered by SONiC BuildBot

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.

5 participants