thermalctld: fix test warnings #694
Conversation
Add a missing method, remove duplicate code, remove a deprecated import. Signed-off-by: Fraser Gordon <[email protected]>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull Request Overview
This PR modernizes the test code by removing Python 2 compatibility support and consolidating duplicate mock implementations.
- Removes deprecated
imp.load_sourcein favor ofimportlib(Python 3.12+ compatible) - Eliminates Python 2/3 conditional imports for the
mockmodule - Consolidates duplicate mock code by moving it to a shared location
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| sonic-thermalctld/tests/test_thermalctld.py | Removes Python 2 compatibility code and replaces deprecated imp.load_source with modern importlib implementation |
| sonic-thermalctld/tests/mocked_libs/swsscommon/swsscommon.py | Adds missing constants and getKeys() method to the shared mock implementation |
| sonic-thermalctld/tests/mock_swsscommon.py | Consolidates duplicate code by importing from the shared mock library instead of duplicating class definitions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| spec = importlib.util.spec_from_file_location(modname, filename, loader=loader) | ||
| module = importlib.util.module_from_spec(spec) | ||
| sys.modules[module.__name__] = module | ||
| loader.exec_module(module) |
There was a problem hiding this comment.
The load_source function does not return the loaded module. Add return module at the end of the function, as the original imp.load_source returned the loaded module object.
| loader.exec_module(module) | |
| loader.exec_module(module) | |
| return module |
|
|
||
| def get_size(self): | ||
| return (len(self.mock_dict)) | ||
|
|
There was a problem hiding this comment.
Trailing whitespace on line 31 should be removed to maintain code cleanliness.
|
|
||
| STATE_DB = '' | ||
| CHASSIS_STATE_DB = '' | ||
| from .mocked_libs.swsscommon import swsscommon |
There was a problem hiding this comment.
Move these 3 lines of code directly to test_thermalctld.py as the file size is reduced now
3 participants
Description
Motivation and Context
Fixes exceptions which produce warnings (and harm test coverage) when running thermalctld tests.
How Has This Been Tested?
Ran thermalctld tests, observed no more warnings.