Skip to content

Commit

Permalink
[Platform API][SFP] Log warnings, don't fail test if newly-added xcvr…
Browse files Browse the repository at this point in the history
… info fields missing (#3120)

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).
  • Loading branch information
jleveque authored Mar 11, 2021
1 parent 11c7344 commit 8a41e99
Showing 1 changed file with 22 additions and 3 deletions.
25 changes: 22 additions & 3 deletions tests/platform_tests/api/test_sfp.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ class TestSfpApi(PlatformApiTestBase):

EXPECTED_XCVR_INFO_KEYS = [
'type',
'type_abbrv_name',
'manufacturer',
'model',
'hardware_rev',
Expand All @@ -61,7 +60,20 @@ class TestSfpApi(PlatformApiTestBase):
'cable_length',
'specification_compliance',
'nominal_bit_rate',
'application_advertisement'
]

# These are fields which have been added in the common parsers
# in sonic-platform-common/sonic_sfp, but since some vendors are
# using their own custom parsers, they do not yet provide these
# fields. So we treat them differently. Rather than failing the test
# if these fields are not present or 'N/A', we will simply log warnings
# until all vendors utilize the common parsers. At that point, we should
# add these into EXPECTED_XCVR_INFO_KEYS.
NEWLY_ADDED_XCVR_INFO_KEYS = [
'type_abbrv_name',
'application_advertisement',
'is_replaceable',
'dom_capability'
]

EXPECTED_XCVR_BULK_STATUS_KEYS = [
Expand Down Expand Up @@ -220,7 +232,14 @@ def test_get_transceiver_info(self, duthost, localhost, platform_api_conn):
for key in missing_keys:
self.expect(False, "Transceiver {} info does not contain field: '{}'".format(i, key))

unexpected_keys = set(actual_keys) - set(self.EXPECTED_XCVR_INFO_KEYS)
# TODO: Remove this once we can include these keys in EXPECTED_XCVR_INFO_KEYS
for key in self.NEWLY_ADDED_XCVR_INFO_KEYS:
if key not in actual_keys:
logger.warning("test_get_transceiver_info: Transceiver {} info missing field '{}'. Vendor needs to add support.".format(i, key))
elif info_dict[key] == "N/A":
logger.warning("test_get_transceiver_info: Transceiver {} info value for '{}' is 'N/A'. Vendor needs to add support.".format(i, key))

unexpected_keys = set(actual_keys) - set(self.EXPECTED_XCVR_INFO_KEYS + self.NEWLY_ADDED_XCVR_INFO_KEYS)
for key in unexpected_keys:
self.expect(False, "Transceiver {} info contains unexpected field '{}'".format(i, key))
self.assert_expectations()
Expand Down

0 comments on commit 8a41e99

Please sign in to comment.