Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Platform API][SFP] Log warnings, don't fail test if newly-added xcvr info fields missing #3120

Merged
merged 1 commit into from
Mar 11, 2021
Merged

[Platform API][SFP] Log warnings, don't fail test if newly-added xcvr info fields missing #3120

merged 1 commit into from
Mar 11, 2021

Conversation

jleveque
Copy link
Contributor

@jleveque jleveque commented Mar 9, 2021

Description of PR

Summary: Log warnings, don't fail test if newly-added xcvr info fields missing in test_get_transceiver_info SFP platform API test

Fixes: #2983

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Approach

What is the motivation for this PR?

Some vendors currently use their own SFP EEPROM parsing logic instead of using common logic in the sonic-platform-common repo. Thus, when a new field is added in the common code, if a vendor does not utilize the common code but rather implements their own parsing logic, their parser will not provide this field until the vendor adds logic to do so.

We are planning to refactor the sonic_sfp code in sonic-platform-common in hopes of making it easier for vendors to utilize the common code and move away from using their own parsers. Until then, this patch prevents the test case from failing if recently-added fields are not present, and instead logs warning messages, which should alert the vendor that they need to parse the new field(s).

How did you do it?

Remove recently-added fields from list of expected keys and add to a new list of newly-added keys. If keys in the newly-added list aren't present, log warnings to alert the vendor that they need to update their parser (or preferably use the common parser).

How did you verify/test it?

Tested against an S6100. Test case passes and the following logs are seen:

23:57:33 WARNING test_sfp.py:test_get_transceiver_info:238: test_get_transceiver_info: Transceiver 0 info missing field 'type_abbrv_name'. Vendor needs to add support.
23:57:33 WARNING test_sfp.py:test_get_transceiver_info:240: test_get_transceiver_info: Transceiver 0 info value for 'application_advertisement' is 'N/A'. Vendor needs to add support.
23:57:33 WARNING test_sfp.py:test_get_transceiver_info:238: test_get_transceiver_info: Transceiver 0 info missing field 'is_replaceable'. Vendor needs to add support.
23:57:33 WARNING test_sfp.py:test_get_transceiver_info:238: test_get_transceiver_info: Transceiver 0 info missing field 'dom_capability'. Vendor needs to add support.

@jleveque jleveque self-assigned this Mar 9, 2021
Copy link
Contributor

@AntonHryshchuk AntonHryshchuk left a comment

Choose a reason for hiding this comment

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

Looks good for me.
Thank you Joe.

@jleveque jleveque requested a review from yxieca March 10, 2021 18:45
@jleveque jleveque merged commit 8a41e99 into sonic-net:master Mar 11, 2021
@jleveque jleveque deleted the sfp_test_backward_compat branch March 11, 2021 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[platform API] TestSfpApi::test_get_transceiver_info fails for platform with dynamic transceiver tuning
3 participants