Skip to content

[Platform_api test] Add a "--skip-absent-sfp" option to rule out absent Transceivers from SFP Platform API tests#2550

Merged
jleveque merged 3 commits intosonic-net:masterfrom
keboliu:fix-sfp-api-presence
Nov 24, 2020
Merged

[Platform_api test] Add a "--skip-absent-sfp" option to rule out absent Transceivers from SFP Platform API tests#2550
jleveque merged 3 commits intosonic-net:masterfrom
keboliu:fix-sfp-api-presence

Conversation

@keboliu
Copy link
Copy Markdown
Contributor

@keboliu keboliu commented Nov 18, 2020

Description of PR

Summary:
Fixes # (#2539)

Type of change

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

Approach

What is the motivation for this PR?

In the SFP platform API tests, all test cases assuming that all the SFP are present, but this is not always true.
Some testbed may only have SFP plugged on partial interfaces. Add a "--skip_absent_sfp" option to tell the test cases to skip the absent SFP.

How did you do it?

If the option "--skip_absent_sfp" set to True, during the setup phase, check the SFP presence status and rule out the non-present SFP index from the list.

How did you verify/test it?

Run platform SFP API tests on the testbed

Any platform specific information?

No.

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

N/A

Documentation

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Nov 18, 2020

This pull request introduces 1 alert when merging 9403598 into bbaaf00 - view on LGTM.com

new alerts:

  • 1 for Except block handles 'BaseException'

Copy link
Copy Markdown
Contributor

@jleveque jleveque left a comment

Choose a reason for hiding this comment

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

I think we need a way to tell the test whether to expect all transceivers are present or to ignore absent transceivers. If not, there could be an issue where a transceiver is present but the API call is broken and returns non-present and the test would not catch this case.

@liat-grozovik
Copy link
Copy Markdown
Collaborator

liat-grozovik commented Nov 19, 2020 via email

@jleveque
Copy link
Copy Markdown
Contributor

I feel like the test itself should be told whether to expect all transceivers present or to ignore transceivers which are reporting as absent. I don't know of a good method to pass this information to Pytest. Maybe we could check for an environment variable? By default, the test would expect all transceivers present, but if the environment variable is set it will ignore all absent transceivers?

@keboliu
Copy link
Copy Markdown
Contributor Author

keboliu commented Nov 20, 2020

@liat-grozovik @jleveque although we are able to get the interface "admin up" status from sonic-mgmt, but still it's missing the knowledge about the mapping between the interface and the SFP object index fetched from platform API, it would be too complicated to add this to sonic-mgmt. I would suggest adding an option to these test cases, by default it will assume all SFP exist, with this option it will only check the present SFPs.

@keboliu keboliu changed the title [Platform_api test] Rule out non-present Transceivers from SFP Platform API tests [Platform_api test] Add a "--skip_absent_sfp" option to rule out absent Transceivers from SFP Platform API tests Nov 20, 2020
@keboliu keboliu changed the title [Platform_api test] Add a "--skip_absent_sfp" option to rule out absent Transceivers from SFP Platform API tests [Platform_api test] Add a "--skip-absent-sfp" option to rule out absent Transceivers from SFP Platform API tests Nov 23, 2020
@liat-grozovik
Copy link
Copy Markdown
Collaborator

@jleveque and @wangxin any further comments?

@jleveque
Copy link
Copy Markdown
Contributor

LGTM. @wangxin to review, also.

@wangxin
Copy link
Copy Markdown
Collaborator

wangxin commented Nov 24, 2020

LGTM

@jleveque jleveque merged commit a1ad771 into sonic-net:master Nov 24, 2020
@keboliu keboliu deleted the fix-sfp-api-presence branch January 2, 2021 01:21
kazinator-arista pushed a commit to kazinator-arista/sonic-mgmt that referenced this pull request Mar 4, 2026
…#12974)

utilities:
* 4b51e41 2022-12-06 | [config] Add check in config interface ip command to block if the interface is portchannel member (sonic-net#2539) (HEAD -> 202205) [Sudharsan Dhamal Gopalarathnam]
* e53b32e 2022-12-06 | [generate_dump] [Mellanox] Fix the duplicate dfw dump collection problem by adding symlinks (sonic-net#2536) [Vivek]
* 0391221 2022-12-02 | [GCU] Add RemoveCreateOnlyDependency Validator/Generator (sonic-net#2500) [jingwenxie]
* e3658e9 2022-04-13 | [scripts/fast-reboot] Shutdown remaining containers through systemd (sonic-net#2133) [Stepan Blyshchak]

swss:
* 1a4a5d9 2022-12-02 | [ACL] Support ACTION_COUNTER action in custom ACL table type (sonic-net#2550) [bingwang-ms]
* 33b0a9e 2022-12-05 | [muxorch] Adding case for maintaining current state (sonic-net#2280) [Nikola Dancejic]

sairedis:
* b29bb45 2022-12-02 | enable cisco8000 SAI bulk API feature (sonic-net#1153) (HEAD -> 202205) [Keith Lu]

Signed-off-by: Ying Xie <ying.xie@microsoft.com>

Signed-off-by: Ying Xie <ying.xie@microsoft.com>
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.

4 participants