Skip to content

[platform_tests: Enhance SFP reset test to handle ports with unplugged optics and diverse transceiver types]#19691

Open
YatishSVC wants to merge 6 commits intosonic-net:masterfrom
YatishSVC:yatishkoul/fix_test_sfp
Open

[platform_tests: Enhance SFP reset test to handle ports with unplugged optics and diverse transceiver types]#19691
YatishSVC wants to merge 6 commits intosonic-net:masterfrom
YatishSVC:yatishkoul/fix_test_sfp

Conversation

@YatishSVC
Copy link
Copy Markdown
Contributor

@YatishSVC YatishSVC commented Jul 17, 2025

Description of PR

This PR improves the test_reset method in the SFP platform API test suite to ensure compatibility across different transceiver types and testbed setups where not all ports may have optics plugged in.

  1. Skip Transceivers with No Optic Plugged In:
    Added a check for None return from sfp.get_transceiver_info(...) to gracefully skip transceivers with no optic inserted.
    This avoids false negatives in test assertions due to missing optics.

  2. Introduced a combined check:
    if "cmis_rev" in info_dict or self.is_xcvr_support_lpmode(info_dict):
    This ensured that all types of transceivers used in the test setup (including QSFP28, QSFP-DD, and CMIS-compliant optics) were flapped appropriately and recovered successfully.

Summary:
Fixes # (issue)

Type of change

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

Back port request

  • 202205
  • 202305
  • 202311
  • 202405
  • 202411
  • 202505

Approach

What is the motivation for this PR?

To fix failures in test_sfp.py

How did you do it?

By adding conditional checks

How did you verify/test it?

Ran in Msft Lab

Any platform specific information?

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

Documentation

@YatishSVC YatishSVC requested a review from prgeor as a code owner July 17, 2025 22:08
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@YatishSVC YatishSVC requested review from arista-nwolfe, arlakshm and mlok-nokia and removed request for prgeor July 17, 2025 22:08
@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

mihirpat1
mihirpat1 previously approved these changes Jul 18, 2025
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

continue
info_dict = port_index_to_info_dict[sfp_port_idx]

# flap ports if they support LPMODE, or if they’re in the known-skip list (e.g. Cloud Light)
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.

@YatishSVC Can we create a function for this so that we don't have to duplicate

# Temporarily add this logic to skip lpmode test for some transceivers with known issue
for xcvr_to_skip in self.LPMODE_SKIP_LIST:
if (xcvr_info_dict["manufacturer"].strip() == xcvr_to_skip["manufacturer"] and
xcvr_info_dict["host_electrical_interface"].strip() == xcvr_to_skip["host_electrical_interface"]):
logger.info("Temporarily skipping {} due to known issue".format(
xcvr_info_dict["manufacturer"]))
return False

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion @mihirpat1,
made the changes now accordingly.

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

tjchadaga
tjchadaga previously approved these changes Aug 1, 2025
return 0.3
return 0

def is_in_lpmode_skip_list(self, xcvr_info_dict):
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.

Suggested change
def is_in_lpmode_skip_list(self, xcvr_info_dict):
def should_skip_lpmode_check(self, xcvr_info_dict):

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done @mihirpat1,
Thank you.

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@mihirpat1
Copy link
Copy Markdown
Contributor

@yatishkoul LGTM

arlakshm pushed a commit to Azure/sonic-mgmt.msft that referenced this pull request Aug 11, 2025
…ptics (#635)

Cherry pick conflicts, so created new pull request.
Pr's: 
sonic-net/sonic-mgmt#18772
sonic-net/sonic-mgmt#20120
sonic-net/sonic-mgmt#19691

Adding a check for Cloudlight optics.
Adding multi asic support for test_reset

Co-authored-by: yatishkoul <[email protected]>
@YatishSVC YatishSVC dismissed stale reviews from tjchadaga and mihirpat1 via 7fe09bd March 9, 2026 21:04
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