ConversionUtils() unit tests and hardening #472
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1. tests/unit/module_utils/common/test_conversion_utils.py Unit tests for ConversioniUtils() 2. plugins/module_utils/common/conversion.py 2a. translate_mac_address - Hardening - raise ValueError if anything but a string
1. plugins/module_utils/common/conversion.py 1a. translate_mac_address was failing for non-string values. Fix by convering mac_addr to str before re.sub() Also, use the original mac_addr rather than the re.sub() version in the ValueError() message. 2. Fix two fabric unit tests Two unit tests were failing after the change in 1 above. These were in files: - tests/unit/modules/dcnm/dcnm_fabric/test_fabric_create.py - tests/unit/modules/dcnm/dcnm_fabric/test_fabric_create_bulk.py Fixed these to expect the original mac_addr rather than the re.sub() version.
Adding final unit tests for ConversionUtils().validate_fabric_name
mikewiebe
reviewed
Jul 16, 2025
mikewiebe
approved these changes
Jul 18, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
The
ConversionUtilsclass lacked associated unit tests. Adding them now.Hardening
translate_mac_addresscould throw an unintended TypeError with integer input since the re.sub() was trying to alter the passedmac_addr, which could potentially be anint. Fixed by castingmac_addrtostrwithin there.sub().Usability
translate_mac_addresswas returning the altered MAC address in its ValueError which could be confusing to the end user. Fixed by copying the originalmac_addrand using the copy in the ValueError.Updates to other unit tests
Two fabric unit tests started failing after the above changes now that we are using the original MAC address in the ValueError. Fixed the asserts in these to expect the original MAC address.