Skip to content

Fix test failures in test_thermalctld.py by passing arguments to ThermalControlDaemon()#700

Merged
judyjoseph merged 1 commit intosonic-net:masterfrom
nexthop-ai:fix-test-thermalctld
Oct 31, 2025
Merged

Fix test failures in test_thermalctld.py by passing arguments to ThermalControlDaemon()#700
judyjoseph merged 1 commit intosonic-net:masterfrom
nexthop-ai:fix-test-thermalctld

Conversation

@louis-nexthop
Copy link
Copy Markdown
Contributor

@louis-nexthop louis-nexthop commented Oct 30, 2025

Description

Fix test failures in test_thermalctld.py, that was introduced in #652. ThermalControlDaemon.__init__() now requires 3 arguments, so we have to pass them.

Motivation and Context

Fixes #699, which will allow Azure Pipelines to pass.

test_get_chassis_exception and test_get_chassis_success were failing with the following error:

TypeError: ThermalControlDaemon.__init__() missing 3 required positional arguments: 'thermal_monitor_initial_interval', 'thermal_monitor_update_interval', and 'thermal_monitor_update_elapsed_threshold' 

because they didn't pass thermal-monitor-related args to ThermalControlDaemon(), which were introduced in #635.

How Has This Been Tested?

Ran unit tests locally and passed.

Additional Information (Optional)

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Copy Markdown

@lotus-nexthop lotus-nexthop left a comment

Choose a reason for hiding this comment

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

@judyjoseph please take a look, thanks!

Copy link
Copy Markdown
Contributor

@vvolam vvolam left a comment

Choose a reason for hiding this comment

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

why the #635 got merged with these test failures, was the code not compiled before raising the PR?

@louis-nexthop
Copy link
Copy Markdown
Contributor Author

louis-nexthop commented Oct 31, 2025

why the #635 got merged with these test failures, was the code not compiled before raising the PR?

The Azure pipelines on #635 ran and passed on Oct 1, which was before #652 got merged on Oct 28.

The PR (#635) didn't raise any merge conflict, and we didn't sync master into it. So the failure was not caught.

Thanks for the quick review of this PR!

@judyjoseph judyjoseph merged commit ef9eb35 into sonic-net:master Oct 31, 2025
5 checks passed
vvolam pushed a commit to vvolam/sonic-platform-daemons that referenced this pull request Nov 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test failures in test_thermalctld.py: ThermalControlDaemon.__init__() missing 3 required positional arguments

5 participants