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.

Sign up for GitHub

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

Jump to bottom

add unit tests for CMIS host management feature #13

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

add unit tests for CMIS host management feature #13

wants to merge 4 commits into from

Conversation

dbarashinvd
Copy link
Owner

@dbarashinvd dbarashinvd commented Feb 1, 2024 •
edited
Loading

Why I did it

add unit tests to test CMIS host management feature

Work item tracking
  • Microsoft ADO (number only):

How I did it

add new file for testing the feature using python unit tests

How to verify it

run all tests from mlnx-platform-api python wheel and make sure all of them passes

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

noaOrMlnx
noaOrMlnx suggested changes Feb 19, 2024
View reviewed changes
platform/mellanox/mlnx-platform-api/tests/test_modules_mgmt.py Outdated Show resolved Hide resolved
platform/mellanox/mlnx-platform-api/tests/test_modules_mgmt.py Outdated Show resolved Hide resolved
platform/mellanox/mlnx-platform-api/tests/test_modules_mgmt.py Show resolved Hide resolved
platform/mellanox/mlnx-platform-api/tests/test_modules_mgmt.py Outdated Show resolved Hide resolved
platform/mellanox/mlnx-platform-api/tests/test_modules_mgmt.py Show resolved Hide resolved
platform/mellanox/mlnx-platform-api/tests/test_modules_mgmt.py
return self.is_flat_memory

def read(self, field):
if consts.FLAT_MEM_FIELD == field:
Copy link

Choose a reason for hiding this comment

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

why FLAT_MEM_FIELD needed here? please check

Copy link
Owner Author

Choose a reason for hiding this comment

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

needed to mock the eeprom and flat memory vs non-flat memory (i.e. active optic cmis)
added prints inside this read and noticed it's done when creating an SFP and xcvr_eeprom:

collecting ... MockXcvrEeprom read field Flat_mem
MockXcvrEeprom read field Flat_mem is consts.FLAT_MEM_FIELD Flat_mem
MockXcvrEeprom read returning 1
MockXcvrEeprom read field Flat_mem
MockXcvrEeprom read field Flat_mem is consts.FLAT_MEM_FIELD Flat_mem
MockXcvrEeprom read returning 1
MockXcvrEeprom read field Flat_mem
MockXcvrEeprom read field Flat_mem is consts.FLAT_MEM_FIELD Flat_mem
MockXcvrEeprom read returning 1
MockXcvrEeprom read field Flat_mem
MockXcvrEeprom read field Flat_mem is consts.FLAT_MEM_FIELD Flat_mem
MockXcvrEeprom read returning 1
MockXcvrEeprom read field Flat_mem
MockXcvrEeprom read field Flat_mem is consts.FLAT_MEM_FIELD Flat_mem
MockXcvrEeprom read returning 1
MockXcvrEeprom read field Flat_mem
MockXcvrEeprom read field Flat_mem is consts.FLAT_MEM_FIELD Flat_mem
MockXcvrEeprom read returning 1
MockXcvrEeprom read field Flat_mem
MockXcvrEeprom read field Flat_mem is consts.FLAT_MEM_FIELD Flat_mem
MockXcvrEeprom read returning 1
MockXcvrEeprom read field Flat_mem
MockXcvrEeprom read field Flat_mem is consts.FLAT_MEM_FIELD Flat_mem
MockXcvrEeprom read returning 1
MockXcvrEeprom read field Flat_mem
MockXcvrEeprom read field Flat_mem is consts.FLAT_MEM_FIELD Flat_mem
MockXcvrEeprom read returning 1
MockXcvrEeprom read field Flat_mem
MockXcvrEeprom read field Flat_mem is consts.FLAT_MEM_FIELD Flat_mem
MockXcvrEeprom read returning 1
MockXcvrEeprom read field Flat_mem
MockXcvrEeprom read field Flat_mem is consts.FLAT_MEM_FIELD Flat_mem
MockXcvrEeprom read returning 1
MockXcvrEeprom read field Flat_mem
MockXcvrEeprom read field Flat_mem is consts.FLAT_MEM_FIELD Flat_mem
MockXcvrEeprom read returning 1
MockXcvrEeprom read field Flat_mem
MockXcvrEeprom read field Flat_mem is consts.FLAT_MEM_FIELD Flat_mem
MockXcvrEeprom read returning 1
MockXcvrEeprom read field Flat_mem
MockXcvrEeprom read field Flat_mem is consts.FLAT_MEM_FIELD Flat_mem
MockXcvrEeprom read returning 1
MockXcvrEeprom read field Flat_mem
MockXcvrEeprom read field Flat_mem is consts.FLAT_MEM_FIELD Flat_mem
MockXcvrEeprom read returning 1
MockXcvrEeprom read field Flat_mem
MockXcvrEeprom read field Flat_mem is consts.FLAT_MEM_FIELD Flat_mem
MockXcvrEeprom read returning 1
MockXcvrEeprom read field Flat_mem
MockXcvrEeprom read field Flat_mem is consts.FLAT_MEM_FIELD Flat_mem
MockXcvrEeprom read returning 1
MockXcvrEeprom read field Flat_mem
MockXcvrEeprom read field Flat_mem is consts.FLAT_MEM_FIELD Flat_mem
MockXcvrEeprom read returning 1
MockXcvrEeprom read field Flat_mem
MockXcvrEeprom read field Flat_mem is consts.FLAT_MEM_FIELD Flat_mem
MockXcvrEeprom read returning 1
MockXcvrEeprom read field Flat_mem
MockXcvrEeprom read field Flat_mem is consts.FLAT_MEM_FIELD Flat_mem
MockXcvrEeprom read returning 1
MockXcvrEeprom read field Flat_mem
MockXcvrEeprom read field Flat_mem is consts.FLAT_MEM_FIELD Flat_mem
MockXcvrEeprom read returning 1
MockXcvrEeprom read field Flat_mem
MockXcvrEeprom read field Flat_mem is consts.FLAT_MEM_FIELD Flat_mem
MockXcvrEeprom read returning 1
MockXcvrEeprom read field Flat_mem
MockXcvrEeprom read field Flat_mem is consts.FLAT_MEM_FIELD Flat_mem
MockXcvrEeprom read returning 1
MockXcvrEeprom read field Flat_mem
MockXcvrEeprom read field Flat_mem is consts.FLAT_MEM_FIELD Flat_mem
MockXcvrEeprom read returning 1
collected 13 items

tests/test_modules_mgmt.py::TestModulesMgmt::test_mdf_all_ports_feature_disabled PASSED

if this read function is not mocked, then getting this error:
collected 0 items / 1 error

================================================================================================================== ERRORS ===================================================================================================================
________________________________________________________________________________________________ ERROR collecting tests/test_modules_mgmt.py ________________________________________________________________________________________________
tests/test_modules_mgmt.py:385: in
class TestModulesMgmt(unittest.TestCase):
tests/test_modules_mgmt.py:541: in TestModulesMgmt
???
tests/test_modules_mgmt.py:367: in init
self.xcvr_api = Mock(spec=CmisApi(MockXcvrEeprom(False, CmisMemMap(CmisCodes))), return_value=MockXcvrapi(xcvr_api_is_cmis_api, xcvr_eeprom_is_flat_memory))
/sonic/src/sonic-platform-common/sonic_platform_base/sonic_xcvr/api/public/cmis.py:31: in init
self.vdm = CmisVdmApi(xcvr_eeprom) if not self.is_flat_memory() else None
/sonic/src/sonic-platform-common/sonic_platform_base/sonic_xcvr/api/public/cmis.py:343: in is_flat_memory
return self.xcvr_eeprom.read(consts.FLAT_MEM_FIELD)
E AttributeError: 'MockXcvrEeprom' object has no attribute 'read'
============================================================================================================= warnings summary ============================================================================

platform/mellanox/mlnx-platform-api/tests/test_modules_mgmt.py Outdated Show resolved Hide resolved
platform/mellanox/mlnx-platform-api/tests/test_modules_mgmt.py Outdated Show resolved Hide resolved
platform/mellanox/mlnx-platform-api/tests/test_modules_mgmt.py Show resolved Hide resolved
platform/mellanox/mlnx-platform-api/tests/test_modules_mgmt.py
else:
return mock_context.return_value

def mock_open_new_side_effect_warm_reboot(self, filename, *args, **kwargs):
Copy link

Choose a reason for hiding this comment

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

why do we need a different one for warm boot?

Copy link
Owner Author

Choose a reason for hiding this comment

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

needed for the warm reboot plugout - plugin test. not sure why though

@dbarashinvd
fix internal code review comments
6ed36bf 
shrink the code using setup and teardown functions
noaOrMlnx
noaOrMlnx suggested changes Feb 22, 2024
View reviewed changes
platform/mellanox/mlnx-platform-api/tests/test_modules_mgmt.py Outdated
# start modules_mgmt thread and the test in poller part
with patch('select.poll', MagicMock(return_value=MockPollerStopEvent(self.modules_mgmt_task_stopping_event
, self.modules_mgmt_thrd))):
with patch('sonic_platform.sfp.SFP', MagicMock(return_value=MockSFPxcvrapi())):
Copy link

Choose a reason for hiding this comment

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

please add this patch line to the patches above the test name

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@noaOrMlnx noaOrMlnx noaOrMlnx requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dbarashinvd @noaOrMlnx