Skip to content

Commit

Permalink
Fix issue: should check remote file existence in ThermalPolicyFileCon…
Browse files Browse the repository at this point in the history
…text (#4582)

Fixes issue in ThermalPolicyFileContext, it should always check the file existence in remote DUT, not local machine. Without the fix, the thermal policy file will not be recovered and it could cause following test case fail.
This issue is introduced by PR #4262

-How did you do it?
Check remote file existence via dut.stat method

- How did you verify/test it?
Manual run the test and check thermal_policy.json is recovered

- Any platform specific information?
N/A
  • Loading branch information
Junchao-Mellanox authored Oct 31, 2021
1 parent 53d6574 commit 4f52b35
Showing 1 changed file with 6 additions and 5 deletions.
11 changes: 6 additions & 5 deletions tests/platform_tests/thermal_control_test_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ def restart_thermal_control_daemon(dut):
assert output["rc"] == 0, "Run command '%s' failed" % find_thermalctld_pid_cmd
# Usually there should be 2 thermalctld processes, but there is chance that
# sonic platform API might use subprocess which creates extra thermalctld process.
# For example, chassis.get_all_sfps will call sfp constructor, and sfp constructor may
# For example, chassis.get_all_sfps will call sfp constructor, and sfp constructor may
# use subprocess to call ethtool to do initialization.
# So we check here thermalcltd must have at least 2 processes.
assert len(output["stdout_lines"]) >= 2, "There should be at least 2 thermalctld process"
Expand Down Expand Up @@ -303,7 +303,8 @@ def __enter__(self):
thermal control daemon to make it effect.
:return:
"""
if os.path.exists(self.thermal_policy_file_path):
out = self.dut.stat(path=self.thermal_policy_file_path)
if out['stat']['exists']:
self.dut.command('mv -f {} {}'.format(self.thermal_policy_file_path, self.thermal_policy_file_backup_path))
else:
logging.warning("Thermal Policy file {} not found".format(self.thermal_policy_file_path))
Expand All @@ -318,8 +319,8 @@ def __exit__(self, exc_type, exc_val, exc_tb):
:param exc_tb: Not used.
:return:
"""

if os.path.exists(self.thermal_policy_file_backup_path):
out = self.dut.stat(path=self.thermal_policy_file_backup_path)
if out['stat']['exists']:
self.dut.command('mv -f {} {}'.format(self.thermal_policy_file_backup_path, self.thermal_policy_file_path))
restart_thermal_control_daemon(self.dut)

Expand All @@ -328,7 +329,7 @@ def __exit__(self, exc_type, exc_val, exc_tb):
def disable_thermal_policy(duthosts, enum_rand_one_per_hwsku_hostname):
"""Fixture to help disable thermal policy during the test. After test, it will
automatically re-enable thermal policy. The idea here is to make thermalctld
load a invalid policy file. To use this fixture, the test case will probably
load a invalid policy file. To use this fixture, the test case will probably
marked as @pytest.mark.disable_loganalyzer.
Args:
Expand Down

0 comments on commit 4f52b35

Please sign in to comment.