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.

Sign up for GitHub

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

Jump to bottom

[Platform API] Add test cases for new API methods #2370

Merged
merged 5 commits into from
Oct 22, 2020
Merged

[Platform API] Add test cases for new API methods #2370

merged 5 commits into from
Oct 22, 2020

Conversation

Junchao-Mellanox
Copy link
Contributor

@Junchao-Mellanox Junchao-Mellanox commented Oct 20, 2020 •
edited
Loading

Description of PR

Summary:
During SONiC physical entity MIB feature development, a few new platform APIs were added and they need regression test cases to verify them.

Depends on sonic-net/sonic-platform-common#134

Type of change

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

Approach

What is the motivation for this PR?

Add test cases and test support for all newly added platform API introduced by SONiC physical entity MIB feature

How did you do it?

Add new API test support for chassis, fan, fan drawer, psu, sfp and add new test cases for them.

How did you verify/test it?

Manually run the regression

Any platform specific information?

N/A

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

N/A

Documentation

@Junchao-Mellanox Junchao-Mellanox mentioned this pull request Oct 20, 2020
Closed
3 tasks
@lgtm-com
Copy link

lgtm-com bot commented Oct 20, 2020

This pull request introduces 2 alerts and fixes 2 when merging 601477b into 7d140d5 - view on LGTM.com

new alerts:

  • 2 for Except block handles 'BaseException'

fixed alerts:

  • 2 for Unused import

@wangxin wangxin requested a review from jleveque October 20, 2020 11:10
jleveque
jleveque suggested changes Oct 20, 2020
View reviewed changes
tests/platform_tests/api/test_psu.py Outdated Show resolved Hide resolved
tests/platform_tests/api/test_sfp.py Outdated Show resolved Hide resolved
tests/platform_tests/api/test_psu.py Outdated Show resolved Hide resolved
tests/platform_tests/api/test_psu.py Outdated Show resolved Hide resolved
tests/platform_tests/api/test_chassis.py Outdated Show resolved Hide resolved
tests/platform_tests/api/test_chassis_fans.py Outdated Show resolved Hide resolved
tests/platform_tests/api/test_sfp.py Outdated Show resolved Hide resolved
tests/platform_tests/api/test_sfp.py Outdated Show resolved Hide resolved
tests/platform_tests/api/test_thermal.py Outdated Show resolved Hide resolved
tests/platform_tests/api/test_thermal.py Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Oct 21, 2020

This pull request fixes 2 alerts when merging 8c84817 into 18a1540 - view on LGTM.com

fixed alerts:

  • 2 for Unused import

vdahiya12
vdahiya12 suggested changes Oct 21, 2020
View reviewed changes
tests/platform_tests/api/test_fan_drawer.py Outdated Show resolved Hide resolved
tests/platform_tests/api/test_fan_drawer.py Outdated Show resolved Hide resolved
tests/platform_tests/api/test_psu.py Outdated Show resolved Hide resolved
tests/platform_tests/api/test_psu.py Outdated Show resolved Hide resolved
tests/platform_tests/api/test_sfp.py Outdated Show resolved Hide resolved
tests/platform_tests/api/test_sfp.py Outdated Show resolved Hide resolved
tests/platform_tests/api/test_sfp.py Outdated Show resolved Hide resolved
vdahiya12
vdahiya12 suggested changes Oct 21, 2020
View reviewed changes
tests/platform_tests/api/test_psu.py Outdated Show resolved Hide resolved
tests/platform_tests/api/test_sfp.py Outdated Show resolved Hide resolved
tests/platform_tests/api/test_thermal.py Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Oct 21, 2020

This pull request fixes 2 alerts when merging d4734bd into 18a1540 - view on LGTM.com

fixed alerts:

  • 2 for Unused import

sujinmkang
sujinmkang reviewed Oct 21, 2020
View reviewed changes
tests/platform_tests/api/test_chassis_fans.py Outdated Show resolved Hide resolved
tests/platform_tests/api/test_psu.py Outdated Show resolved Hide resolved
tests/platform_tests/api/test_sfp.py Outdated Show resolved Hide resolved
tests/platform_tests/api/test_thermal.py Outdated Show resolved Hide resolved
tests/platform_tests/api/test_fan_drawer.py Outdated Show resolved Hide resolved
tests/platform_tests/api/test_chassis.py Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Oct 21, 2020

This pull request fixes 2 alerts when merging 798f6c5 into 18a1540 - view on LGTM.com

fixed alerts:

  • 2 for Unused import

sujinmkang
sujinmkang reviewed Oct 21, 2020
View reviewed changes
tests/common/helpers/platform_api/fan.py Outdated Show resolved Hide resolved
tests/common/helpers/platform_api/fan_drawer.py Outdated Show resolved Hide resolved
tests/common/helpers/platform_api/psu.py Outdated Show resolved Hide resolved
tests/common/helpers/platform_api/thermal.py Outdated Show resolved Hide resolved
tests/common/helpers/platform_api/sfp.py Outdated Show resolved Hide resolved
tests/common/helpers/platform_api/chassis.py Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Oct 22, 2020

This pull request fixes 2 alerts when merging b02b4d3 into 4b3fa1c - view on LGTM.com

fixed alerts:

  • 2 for Unused import

@jleveque jleveque changed the title Add test cases for new platform API [Platform API] Add test cases for new API methods Oct 22, 2020
@jleveque jleveque merged commit ce66a48 into sonic-net:master Oct 22, 2020
@jleveque
Copy link
Contributor

@Junchao-Mellanox: Since these methods are inherited from DeviceBase, there are other classes which inherit these methods. Even though they may not be used by the MIB, we should test that the APIs are implemented for completeness. Do you agree? If so, can you please add similar API tests to the following files and their respective helpers?

  • test_component.py
  • test_fan_drawer_fans.py
  • test_module.py
  • test_psu_fans.py

@Junchao-Mellanox
Copy link
Contributor Author

@Junchao-Mellanox: Since these methods are inherited from DeviceBase, there are other classes which inherit these methods. Even though they may not be used by the MIB, we should test that the APIs are implemented for completeness. Do you agree? If so, can you please add similar API tests to the following files and their respective helpers?

  • test_component.py
  • test_fan_drawer_fans.py
  • test_module.py
  • test_psu_fans.py

Agree, will do it.

@Junchao-Mellanox Junchao-Mellanox deleted the phy-mibs-api branch October 23, 2020 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vdahiya12 vdahiya12 vdahiya12 approved these changes

@sujinmkang sujinmkang sujinmkang approved these changes

@jleveque jleveque jleveque approved these changes

Assignees
No one assigned
Labels
Enhancement Platform Pytest 🐍
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Junchao-Mellanox @jleveque @sujinmkang @vdahiya12