SFPS: fix to fetch right value from sfp_list to compare in test_sfps test in 'platform_tests/api/test_chassis.py' #3985
Conversation
|
look at the history. it was change to sfp_index explicitly, I am not sure if this pr is correct or not. @sujinmkang , have you confirmed? @rawal01 , can you check? |
|
@lguohan
so sfp for index 1 will get sfp1 the first sfp in DUT by chassis.get_sfp(platform_api_conn, i)
it will always compare value returned by first index from list of indexes and first item in list from get_all_sfps After this change it would work for both 0 based and 1 based or even if it is non continuous indexes. |
There was a problem hiding this comment.
@lguohan
this is correct as in case of platforms not starting from 0, you will get list of indexes 1...x say,
list_sfps = [1, 2 , ..]
when do chassis.get_all_sfps it returns a list of sfps from DUT for e.g. [sfp1, sfp2 , ...]
current code was doing :for i in list_sfps: sfp = chassis.get_sfp(platform_api_conn, i) self.expect(sfp and sfp == sfp_list[i], "SFP {} is incorrect".format(i))so sfp for index 1 will get sfp1 the first sfp in DUT by chassis.get_sfp(platform_api_conn, i)
but sfp_list[1] will get second item in list sfp2 instead of sfp1 , as lists are 0 index based (python list), so to correct this we introduced change to fix that.for i in range(num_sfps): index = list_sfps[i] sfp = chassis.get_sfp(platform_api_conn, index) self.expect(sfp and sfp == sfp_list[i], "SFP {} is incorrect".format(i))it will always compare value returned by first index from list of indexes and first item in list from get_all_sfps
After this change it would work for both 0 based and 1 based or even if it is non continuous indexes.
Hope this makes it more clear.
@lguohan
this is correct as in case of platforms not starting from 0, you will get list of indexes 1...x say,
list_sfps = [1, 2 , ..]
when do chassis.get_all_sfps it returns a list of sfps from DUT for e.g. [sfp1, sfp2 , ...]
current code was doing :for i in list_sfps: sfp = chassis.get_sfp(platform_api_conn, i) self.expect(sfp and sfp == sfp_list[i], "SFP {} is incorrect".format(i))so sfp for index 1 will get sfp1 the first sfp in DUT by chassis.get_sfp(platform_api_conn, i)
but sfp_list[1] will get second item in list sfp2 instead of sfp1 , as lists are 0 index based (python list), so to correct this we introduced change to fix that.for i in range(num_sfps): index = list_sfps[i] sfp = chassis.get_sfp(platform_api_conn, index) self.expect(sfp and sfp == sfp_list[i], "SFP {} is incorrect".format(i))it will always compare value returned by first index from list of indexes and first item in list from get_all_sfps
After this change it would work for both 0 based and 1 based or even if it is non continuous indexes.
Hope this makes it more clear.
The SFP list will always start from index "0". See Ref:- https://github.com/Azure/sonic-platform-common/blob/master/sonic_platform_base/chassis_base.py#L473
chassis.get_sfp(0) should return SFP object corresponding to FIRST physical port, chassis.get_sfp(1) should return the SECOND physical port and so on. The abstract class doesn't talk about any mapping because the chassis object corresponding to a platform should take care of the internal mapping between SFP index and the physical port numbering. The common test relies upon the abstract class definition described and agreed upon by all platforms.
|
I believe @rawal01's quoting is correct. We do want to have index match the front panel numbering, therefore index can start from either 0 or 1. |
|
The test_sfps() test case in test_chassis.py is going through a list of SFP objects but in reality, chassis.get_all_sfps() and chassis.get_sfp() is actually not returning objects (see the test logs below) 05:49:35 chassis.chassis_api L0016 INFO | Executing chassis API: "get_sfp", arguments: "[0]", result: "{u'module': u'arista.utils.sonic_platform.sfp', u'class': u'Sfp'}" All the results are same dictionary so there is no point in going through each index. Object serialization is failing here:- https://github.com/Azure/sonic-mgmt/blob/master/tests/common/helpers/platform_api/scripts/platform_api_server.py#L96 Aug 21 05:49:35.557043 str2-7050cx3-acs-09 WARNING pmon#platform_api_server.py: Unserializable object: arista.utils.sonic_platform.sfp.Sfp |
|
@rawal01 This PR assumes that the ChassisBase._sfp_list[] has all valid SFPs but this assumption doesn't go well with get_sfp() where index starts from '1'. I have fixed this here sonic-net/sonic-platform-common#214. Can you review? |
…test in 'platform_tests/api/test_chassis.py' (sonic-net#3985) * get_all_sfps returns list which should be traversed from 0 index irrespective of sfp index * Update test_chassis.py
Description of PR
Fixed the issue: - 'get_all_sfps' returns list which should be traversed from 0 index irrespective of sfp index
Summary:
Fixes # (issue)
Type of change
Back port request
Approach
What is the motivation for this PR?
How did you do it?
How did you verify/test it?
Any platform specific information?
Supported testbed topology if it's a new test case?
Documentation