Skip to content

[Mellanox] Update tests related to thermal control#9251

Merged
liat-grozovik merged 1 commit intosonic-net:masterfrom
JibinBao:update_tc_case
Aug 27, 2023
Merged

[Mellanox] Update tests related to thermal control#9251
liat-grozovik merged 1 commit intosonic-net:masterfrom
JibinBao:update_tc_case

Conversation

@JibinBao
Copy link
Contributor

@JibinBao JibinBao commented Aug 3, 2023

Description of PR

Update tests related to thermal control due to the thermal control mechanism being changed for Nvidia device.

  1. Update test_get_fans_target_speed, test_show_platform_fanstatus_mocked and test_show_platform_temperature_mocked
  2. Remove tests/platform_tests/mellanox/test_thermal_control.py due to the case is not suitable for the new thermal control mechanism

Summary:
Fixes # (issue)

Type of change

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

Back port request

  • 201911
  • 202012
  • 202205
  • 202305

Approach

What is the motivation for this PR?

Update tests related to thermal control due to the thermal control mechanism being changed.

How did you do it?

Update tests related to thermal control

How did you verify/test it?

run tests related to thermal control on the image supporting new thermal control mechanism

Any platform specific information?

Any

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

Any

Documentation

@JibinBao JibinBao requested a review from prgeor as a code owner August 3, 2023 05:26
@JibinBao JibinBao changed the title update tests related to thermal control [Nvidia] Update tests related to thermal control Aug 3, 2023
@liat-grozovik liat-grozovik merged commit 3811931 into sonic-net:master Aug 27, 2023
@liat-grozovik liat-grozovik changed the title [Nvidia] Update tests related to thermal control [Mellanox] Update tests related to thermal control Aug 27, 2023
mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Aug 28, 2023
Update tests related to thermal control due to the thermal control mechanism being changed for Nvidia device.

Update test_get_fans_target_speed, test_show_platform_fanstatus_mocked and test_show_platform_temperature_mocked
Remove tests/platform_tests/mellanox/test_thermal_control.py due to the case is not suitable for the new thermal control mechanism

- What is the motivation for this PR?
Update tests related to thermal control due to the thermal control mechanism being changed.

- How did you do it?
Update tests related to thermal control

- How did you verify/test it?
run tests related to thermal control on the image supporting new thermal control mechanism
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202305: #9667

mssonicbld pushed a commit that referenced this pull request Aug 28, 2023
Update tests related to thermal control due to the thermal control mechanism being changed for Nvidia device.

Update test_get_fans_target_speed, test_show_platform_fanstatus_mocked and test_show_platform_temperature_mocked
Remove tests/platform_tests/mellanox/test_thermal_control.py due to the case is not suitable for the new thermal control mechanism

- What is the motivation for this PR?
Update tests related to thermal control due to the thermal control mechanism being changed.

- How did you do it?
Update tests related to thermal control

- How did you verify/test it?
run tests related to thermal control on the image supporting new thermal control mechanism
@bingwang-ms
Copy link
Collaborator

@JibinBao I saw below error on my testbed. Looks like the function is_host_service_running is not defined. Can you please check?

>       if is_mellanox_device(duthost) and duthost.is_host_service_running("hw-management-tc"):

platform_tests/conftest.py:725: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
common/devices/multi_asic.py:289: in __getattr__
    return getattr(self.sonichost, attr)  # For backward compatibility
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <SonicHost str2-msn2700-spy-2>, module_name = 'is_host_service_running'

    def __getattr__(self, module_name):
        if self.host.has_module(module_name):
            self.module_name = module_name
            self.module = getattr(self.host, module_name)
    
            return self._run
>       raise AttributeError(
            "'%s' object has no attribute '%s'" % (self.__class__, module_name)
            )
E       AttributeError: '<class 'tests.common.devices.sonic.SonicHost'>' object has no attribute 'is_host_service_running'

@JibinBao
Copy link
Contributor Author

@bingwang-ms Because this function is in the PR https://github.com/sonic-net/sonic-mgmt/pull/8712,but it is still under review.
To fix this issue, I have created one PR only for this function, can you help review it?
#9942

@bingwang-ms
Copy link
Collaborator

@bingwang-ms Because this function is in the PR https://github.com/sonic-net/sonic-mgmt/pull/8712,but it is still under review. To fix this issue, I have created one PR only for this function, can you help review it? #9942

Thanks! The PR #9942 is merged

AharonMalkin pushed a commit to AharonMalkin/sonic-mgmt that referenced this pull request Jan 25, 2024
Update tests related to thermal control due to the thermal control mechanism being changed for Nvidia device.

Update test_get_fans_target_speed, test_show_platform_fanstatus_mocked and test_show_platform_temperature_mocked
Remove tests/platform_tests/mellanox/test_thermal_control.py due to the case is not suitable for the new thermal control mechanism

- What is the motivation for this PR?
Update tests related to thermal control due to the thermal control mechanism being changed.

- How did you do it?
Update tests related to thermal control

- How did you verify/test it?
run tests related to thermal control on the image supporting new thermal control mechanism
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.

7 participants