From 8a41e995c4f41fd62c007262a3fa74fcf8ffaed7 Mon Sep 17 00:00:00 2001
From: Joe LeVeque <jleveque@users.noreply.github.com>
Date: Thu, 11 Mar 2021 10:24:34 -0800
Subject: [PATCH] [Platform API][SFP] Log warnings, don't fail test if
 newly-added xcvr 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).
---
 tests/platform_tests/api/test_sfp.py | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/tests/platform_tests/api/test_sfp.py b/tests/platform_tests/api/test_sfp.py
index e4111b1c4c..73cbba8ddc 100644
--- a/tests/platform_tests/api/test_sfp.py
+++ b/tests/platform_tests/api/test_sfp.py
@@ -46,7 +46,6 @@ class TestSfpApi(PlatformApiTestBase):
 
     EXPECTED_XCVR_INFO_KEYS = [
         'type',
-        'type_abbrv_name',
         'manufacturer',
         'model',
         'hardware_rev',
@@ -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 = [
@@ -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()