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.

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

Add tests for new APIs for chassis and convert existing API tests to run on chassis #2985

Merged
merged 9 commits into from
Apr 1, 2021

Conversation

rawal01
Copy link
Contributor

@rawal01 rawal01 commented Feb 12, 2021

Please make sure you've read and understood our contributing guidelines;
https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md

Please provide following information to help code review process a bit easier:
-->

Description of PR

Summary:
Fixes # (issue)

Type of change

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

Approach

What is the motivation for this PR?

Add new test cases for api for test plan introduced by PR #2695 #2695 (refer to sections 2 to 6)
Need to run existing api tests against a Sonice chassis but all of the tests were using duthost and hence will always run one of duthost of chassis i.e supervisor or line card. Converted these tests to use the enum_* fixture per hwsku for dut selection. Instead of running gather facts as fixture get facts within compare function for selected duthost.

How did you do it?

Following changes made to api helper files in folder sonic-mgmt/tests/common/helpers/platform_api/
chassis.py:
add new api calls: get_module_index, get_supervisor_slot, get_my_slot, is_modular_chassis

fan_drawer.py:
add new api call: get_maximum_consumed_power

module.py:
add new api calls: get_description,	get_slot, get_type,	get_oper_status, get_midplane_ip, 
is_midplane_reachable, get_maximum_consumed_power, reboot, set_admin_state

psu.py:
add new api calls: get_maximum_supplied_power, set_status_master_led, get_status_master_led

thermal.py:
add new api calls: get_minimum_recorded, get_maximum_recorded

changes maded to api tests to support Sonic Chassis and add tests for new apis:

conftest.py:
changes to run tests on  Sonic chassis
change  plaftorm_api_conn to support multidut by using duthosts and enum_rand_one_per_hwsku_hostname to select dut at function level, 
change from getting ip from eth0 to duthost.mgp_ip to extract ip address for DUT 

change start_platform_service  to support multidut by using duthosts and enum_rand_one_per_hwsku_hostname to select dut at function level,
change from getting ip from eth0 to duthost.mgp_ip to extract ip address for DUT 

change in stop_platform_api_service: to support on all duts where the service was started

for all test modules under api these are common changes :
Remove gather_facts fixture and get facts within compare_value_with_platform_facts per duthost from test
changes changes to run tests on Sonic chassis, Replace duthost with duthosts and enum_rand_one_per_hwsku_hostname for multidut environment for all tests

other changes per module:

test_chassis.py:
add tests for new apis for chassis: get_module_index, get_supervisor_slot, get_my_slot, is_modular_chassis

test_component.py:
remove range since image_list is list for image in range(image_list) 

test_fan_drawer.py:

add test for new apis for fan_drawer: get_maximum_consumed_power

test_module.py
add tests for new apis for module_base: get_description,	get_slot, get_type,	get_oper_status, get_midplane_ip, 
is_midplane_reachable, get_maximum_consumed_power, reboot, set_admin_state

test_psu.py
changes changes to run tests on  Sonic chassis,
add tests for new apis: get_maximum_supplied_power, set_status_master_led, get_status_master_led


test_thermal.py
changes changes to run tests on  Sonic chassis, 
add tests for new apis for thermal: get_minimum_recorded, get_maximum_recorded

How did you verify/test it?

     Validated the modified tests against chassis.

Any platform specific information?

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

Documentation

@rawal01 rawal01 requested review from jleveque and a team as code owners February 12, 2021 17:33
@rawal01 rawal01 changed the title Adding new tests for api section in test plan PR #2695 and converting existing tests for pmon apis to run o… Adding new tests for apis addeed for chassis and converting existing tests for pmon apis to run o… Feb 12, 2021
@rawal01 rawal01 changed the title Adding new tests for apis addeed for chassis and converting existing tests for pmon apis to run o… Adding new tests for apis addeed for chassis and converting existing api tests to run on chassis Feb 12, 2021
@lgtm-com

This comment has been minimized.

tests/common/helpers/platform_api/chassis.py Show resolved Hide resolved
tests/common/helpers/platform_api/chassis.py Show resolved Hide resolved
tests/common/helpers/platform_api/psu.py Show resolved Hide resolved
tests/common/helpers/platform_api/psu.py Show resolved Hide resolved
tests/platform_tests/api/conftest.py Outdated Show resolved Hide resolved
tests/platform_tests/api/test_fan_drawer.py Outdated Show resolved Hide resolved
@rawal01 rawal01 force-pushed the pmon-api-tests branch 2 times, most recently from d3d5f94 to 2b294c5 Compare February 25, 2021 19:27
@lgtm-com
Copy link

lgtm-com bot commented Feb 25, 2021

This pull request introduces 1 alert when merging 2b294c53fe20fb78c88400e44221f1431bdbbc30 into 936bcac - view on LGTM.com

new alerts:

  • 1 for Wrong number of arguments in a call

Copy link
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.

Please fix conflicts.

@jleveque jleveque changed the title Adding new tests for apis addeed for chassis and converting existing api tests to run on chassis Add tests for new APIs for chassis and convert existing API tests to run on chassis Mar 16, 2021
Copy link
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.

Looks good to me. Please wait for another review, as this PR touches a lot of files.

@rawal01
Copy link
Contributor Author

rawal01 commented Mar 22, 2021

@yxieca @vdahiya12 @sujinmkang Please review this PR and merge

…n Chassis:

        tests/common/helpers/platform_api/chassis.py
        add new api calls: get_module_index, get_supervisor_slot, get_my_slot, is_modular_chassis

        tests/common/helpers/platform_api/fan_drawer.py
        add new api call: get_maximum_consumed_power

        tests/common/helpers/platform_api/module.py
        add new api calls: get_description,     get_slot, get_type,     get_oper_status, get_midplane_ip,
        is_midplane_reachable, get_maximum_consumed_power, reboot, set_admin_state

        tests/common/helpers/platform_api/psu.py
        add new api calls: get_maximum_supplied_power, set_status_master_led, get_status_master_led

        tests/common/helpers/platform_api/thermal.py
        add new api calls: get_minimum_recorded, get_maximum_recorded

        tests/platform_tests/api/conftest.py:
        changes to run tests on  Sonic chassis

        tests/platform_tests/api/test_chassis.py:
        changes to run tests on  Sonic chassis
        add tests for new apis for chassis: get_module_index, get_supervisor_slot, get_my_slot, is_modular_chassis

        tests/platform_tests/api/test_chassis_fans.py:
        changes changes to run tests on  Sonic chassis

        tests/platform_tests/api/test_component.py:
        changes changes to run tests on  Sonic chassis

        tests/platform_tests/api/test_fan_drawer.py:
        changes changes to run tests on  Sonic chassis,
        add tests for new apis for fan_drawer: get_maximum_consumed_power

        tests/platform_tests/api/test_fan_drawer_fans.py
        changes changes to run tests on  Sonic chassis

        tests/platform_tests/api/test_module.py
        changes changes to run tests on  Sonic chassis,
        add tests for new apis for module_base: get_description,        get_slot, get_type,     get_oper_status, get_midplane_ip,
        is_midplane_reachable, get_maximum_consumed_power, reboot, set_admin_state

        tests/platform_tests/api/test_psu.py
        changes changes to run tests on  Sonic chassis,
        add tests for new apis: get_maximum_supplied_power, set_status_master_led, get_status_master_led

        tests/platform_tests/api/test_psu_fans.py
        changes changes to run tests on  Sonic chassis

        tests/platform_tests/api/test_sfp.py
        changes changes to run tests on  Sonic chassis

        tests/platform_tests/api/test_thermal.py
        changes changes to run tests on  Sonic chassis,
        add tests for new apis for thermal: get_minimum_recorded, get_maximum_recorded

        tests/platform_tests/api/test_watchdog.py
        changes changes to run tests on  Sonic chassis
@shubav
Copy link
Contributor

shubav commented Mar 30, 2021

@jleveque, could you help add the second reviewer, whoever you think best knows this area? We rebased and pushed the changes again. please help move this forward. thanks

@jleveque
Copy link
Contributor

@yxieca: Can you please review this PR for sonic-mgmt best practices when you have a moment?

@wangxin wangxin merged commit a10093c into sonic-net:master Apr 1, 2021
nirmalya-keysight pushed a commit to nirmalya-keysight/sonic-mgmt that referenced this pull request Apr 5, 2021
…run on chassis (sonic-net#2985)

What is the motivation for this PR?
Add new test cases for api for test plan introduced by PR sonic-net#2695 sonic-net#2695 (refer to sections 2 to 6)
Need to run existing api tests against a Sonice chassis but all of the tests were using duthost and hence will always run one of duthost of chassis i.e supervisor or line card. Converted these tests to use the enum_* fixture per hwsku for dut selection. Instead of running gather facts as fixture get facts within compare function for selected duthost.

How did you do it?
Following changes made to api helper files in folder sonic-mgmt/tests/common/helpers/platform_api/
chassis.py:
add new api calls: get_module_index, get_supervisor_slot, get_my_slot, is_modular_chassis

fan_drawer.py:
add new api call: get_maximum_consumed_power

module.py:
add new api calls: get_description,	get_slot, get_type,	get_oper_status, get_midplane_ip, 
is_midplane_reachable, get_maximum_consumed_power, reboot, set_admin_state

psu.py:
add new api calls: get_maximum_supplied_power, set_status_master_led, get_status_master_led

thermal.py:
add new api calls: get_minimum_recorded, get_maximum_recorded
changes maded to api tests to support Sonic Chassis and add tests for new apis:

conftest.py:
changes to run tests on  Sonic chassis
change  plaftorm_api_conn to support multidut by using duthosts and enum_rand_one_per_hwsku_hostname to select dut at function level, 
change from getting ip from eth0 to duthost.mgp_ip to extract ip address for DUT 

change start_platform_service  to support multidut by using duthosts and enum_rand_one_per_hwsku_hostname to select dut at function level,
change from getting ip from eth0 to duthost.mgp_ip to extract ip address for DUT 

change in stop_platform_api_service: to support on all duts where the service was started
for all test modules under api these are common changes :
Remove gather_facts fixture and get facts within compare_value_with_platform_facts per duthost from test
changes changes to run tests on Sonic chassis, Replace duthost with duthosts and enum_rand_one_per_hwsku_hostname for multidut environment for all tests

other changes per module:

test_chassis.py:
add tests for new apis for chassis: get_module_index, get_supervisor_slot, get_my_slot, is_modular_chassis

test_component.py:
remove range since image_list is list for image in range(image_list) 

test_fan_drawer.py:
add test for new apis for fan_drawer: get_maximum_consumed_power

test_module.py
add tests for new apis for module_base: get_description,	get_slot, get_type,	get_oper_status, get_midplane_ip, 
is_midplane_reachable, get_maximum_consumed_power, reboot, set_admin_state

test_psu.py
changes changes to run tests on  Sonic chassis,
add tests for new apis: get_maximum_supplied_power, set_status_master_led, get_status_master_led

test_thermal.py
changes changes to run tests on  Sonic chassis, 
add tests for new apis for thermal: get_minimum_recorded, get_maximum_recorded

How did you verify/test it?
Validated the modified tests against chassis.
saravanansv pushed a commit to saravanansv/sonic-mgmt that referenced this pull request May 6, 2021
…run on chassis (sonic-net#2985)

What is the motivation for this PR?
Add new test cases for api for test plan introduced by PR sonic-net#2695 sonic-net#2695 (refer to sections 2 to 6)
Need to run existing api tests against a Sonice chassis but all of the tests were using duthost and hence will always run one of duthost of chassis i.e supervisor or line card. Converted these tests to use the enum_* fixture per hwsku for dut selection. Instead of running gather facts as fixture get facts within compare function for selected duthost.

How did you do it?
Following changes made to api helper files in folder sonic-mgmt/tests/common/helpers/platform_api/
chassis.py:
add new api calls: get_module_index, get_supervisor_slot, get_my_slot, is_modular_chassis

fan_drawer.py:
add new api call: get_maximum_consumed_power

module.py:
add new api calls: get_description,	get_slot, get_type,	get_oper_status, get_midplane_ip, 
is_midplane_reachable, get_maximum_consumed_power, reboot, set_admin_state

psu.py:
add new api calls: get_maximum_supplied_power, set_status_master_led, get_status_master_led

thermal.py:
add new api calls: get_minimum_recorded, get_maximum_recorded
changes maded to api tests to support Sonic Chassis and add tests for new apis:

conftest.py:
changes to run tests on  Sonic chassis
change  plaftorm_api_conn to support multidut by using duthosts and enum_rand_one_per_hwsku_hostname to select dut at function level, 
change from getting ip from eth0 to duthost.mgp_ip to extract ip address for DUT 

change start_platform_service  to support multidut by using duthosts and enum_rand_one_per_hwsku_hostname to select dut at function level,
change from getting ip from eth0 to duthost.mgp_ip to extract ip address for DUT 

change in stop_platform_api_service: to support on all duts where the service was started
for all test modules under api these are common changes :
Remove gather_facts fixture and get facts within compare_value_with_platform_facts per duthost from test
changes changes to run tests on Sonic chassis, Replace duthost with duthosts and enum_rand_one_per_hwsku_hostname for multidut environment for all tests

other changes per module:

test_chassis.py:
add tests for new apis for chassis: get_module_index, get_supervisor_slot, get_my_slot, is_modular_chassis

test_component.py:
remove range since image_list is list for image in range(image_list) 

test_fan_drawer.py:
add test for new apis for fan_drawer: get_maximum_consumed_power

test_module.py
add tests for new apis for module_base: get_description,	get_slot, get_type,	get_oper_status, get_midplane_ip, 
is_midplane_reachable, get_maximum_consumed_power, reboot, set_admin_state

test_psu.py
changes changes to run tests on  Sonic chassis,
add tests for new apis: get_maximum_supplied_power, set_status_master_led, get_status_master_led

test_thermal.py
changes changes to run tests on  Sonic chassis, 
add tests for new apis for thermal: get_minimum_recorded, get_maximum_recorded

How did you verify/test it?
Validated the modified tests against chassis.
vmittal-msft pushed a commit to vmittal-msft/sonic-mgmt that referenced this pull request Sep 28, 2021
…run on chassis (sonic-net#2985)

What is the motivation for this PR?
Add new test cases for api for test plan introduced by PR sonic-net#2695 sonic-net#2695 (refer to sections 2 to 6)
Need to run existing api tests against a Sonice chassis but all of the tests were using duthost and hence will always run one of duthost of chassis i.e supervisor or line card. Converted these tests to use the enum_* fixture per hwsku for dut selection. Instead of running gather facts as fixture get facts within compare function for selected duthost.

How did you do it?
Following changes made to api helper files in folder sonic-mgmt/tests/common/helpers/platform_api/
chassis.py:
add new api calls: get_module_index, get_supervisor_slot, get_my_slot, is_modular_chassis

fan_drawer.py:
add new api call: get_maximum_consumed_power

module.py:
add new api calls: get_description,	get_slot, get_type,	get_oper_status, get_midplane_ip, 
is_midplane_reachable, get_maximum_consumed_power, reboot, set_admin_state

psu.py:
add new api calls: get_maximum_supplied_power, set_status_master_led, get_status_master_led

thermal.py:
add new api calls: get_minimum_recorded, get_maximum_recorded
changes maded to api tests to support Sonic Chassis and add tests for new apis:

conftest.py:
changes to run tests on  Sonic chassis
change  plaftorm_api_conn to support multidut by using duthosts and enum_rand_one_per_hwsku_hostname to select dut at function level, 
change from getting ip from eth0 to duthost.mgp_ip to extract ip address for DUT 

change start_platform_service  to support multidut by using duthosts and enum_rand_one_per_hwsku_hostname to select dut at function level,
change from getting ip from eth0 to duthost.mgp_ip to extract ip address for DUT 

change in stop_platform_api_service: to support on all duts where the service was started
for all test modules under api these are common changes :
Remove gather_facts fixture and get facts within compare_value_with_platform_facts per duthost from test
changes changes to run tests on Sonic chassis, Replace duthost with duthosts and enum_rand_one_per_hwsku_hostname for multidut environment for all tests

other changes per module:

test_chassis.py:
add tests for new apis for chassis: get_module_index, get_supervisor_slot, get_my_slot, is_modular_chassis

test_component.py:
remove range since image_list is list for image in range(image_list) 

test_fan_drawer.py:
add test for new apis for fan_drawer: get_maximum_consumed_power

test_module.py
add tests for new apis for module_base: get_description,	get_slot, get_type,	get_oper_status, get_midplane_ip, 
is_midplane_reachable, get_maximum_consumed_power, reboot, set_admin_state

test_psu.py
changes changes to run tests on  Sonic chassis,
add tests for new apis: get_maximum_supplied_power, set_status_master_led, get_status_master_led

test_thermal.py
changes changes to run tests on  Sonic chassis, 
add tests for new apis for thermal: get_minimum_recorded, get_maximum_recorded

How did you verify/test it?
Validated the modified tests against chassis.
gechiang added a commit that referenced this pull request Aug 17, 2022
in test_get_presence() there is a new check added by PR (#2985):

                    name = module.get_name(platform_api_conn, i)
                    if name in self.skip_mod_list:
                        self.expect(presence is False, "Module {} is not present".format(i))
                    else:
                        self.expect(presence is True, "Module {} is not present".format(i))

I am seeing some issue with the above logic.
In our testbed where we share a physical chassis that uses the same Supervisor card but group different LCs for different "logical chassis" so for some "logical chassis" some LCs are added to "skip_mod_list" as they are not meant to be tested for that logical chassis.  Our expectation is that if it is marked as skipped, it is not meant for testing and no state of those skipped modules should be used for any validation logic.   We should always trust what the inventory marked as skipped and not try to look at whatever state the "skipped module" should be.
allen-xf pushed a commit to allen-xf/sonic-mgmt that referenced this pull request Oct 28, 2022
in test_get_presence() there is a new check added by PR (sonic-net#2985):

                    name = module.get_name(platform_api_conn, i)
                    if name in self.skip_mod_list:
                        self.expect(presence is False, "Module {} is not present".format(i))
                    else:
                        self.expect(presence is True, "Module {} is not present".format(i))

I am seeing some issue with the above logic.
In our testbed where we share a physical chassis that uses the same Supervisor card but group different LCs for different "logical chassis" so for some "logical chassis" some LCs are added to "skip_mod_list" as they are not meant to be tested for that logical chassis.  Our expectation is that if it is marked as skipped, it is not meant for testing and no state of those skipped modules should be used for any validation logic.   We should always trust what the inventory marked as skipped and not try to look at whatever state the "skipped module" should be.
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