dcnm_vrf: UT - fix null fabric name in vrf attachments #365
Conversation
This file contains 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. module_utils/network/dcnm.py
Add two new methods which are duplicate functionality to get_ip_sn_fabric_dict()
get_ip_fabric_dict()
get_sn_fabric_dict()
get_ip_sn_fabric_dict() was not touched.
The reason for the new defs is because patch (in test_dcnm_vrf.py setUp() did not like patching to a def that returns more than one value. But another reason is that it's cleaner for these utility defs to be as simple as possible.
2. test_dcnm_vrf.py
- Add the mock for sn_fab
- Modify load_fixtures() for all test cases that required having sn_fab dict mocked.
- Define two new vars for fabric details (for a future commit)
- fabric_details_mfd
- fabric_details_vxlan
3. dcnm_vrf.json
Add the following objects (fabric_details_* for a future commit)
- mock_sn_fab
- fabric_details_mfd
- fabric_details_vxlan
1. test_dcnm_vrf.py Fix black whitespace on blank line. 2. dcnm_vrf.py Fix pylint unused-import 3. module_utils/network/dcnm/dcnm.py 1. Run through black 2. Fix pep8 expected 2 blank lines, found 1
Update fabric_details_* 1. tests/unit/modules/dcnm/fixtures/dcnm_vrf.json - Remove dcnm_fabric_orig - Update fabric_details_mfd with default MSD fabric - Update fabric_details_vxlan with default VXLAN/EVPN fabric
allenrobel
added
ready for review
Adding for future use.
module_utils/network/dcnm/dcnm.py Clean up the two recently-added method's docstrings. Fix typos and descriptions of return values.
Maybe overly paranoid, but better to fail at earliest possible point, IMHO.
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
In dcnm_vrf unit tests, multiple tests show a null fabric name in vrf attachments.
This PR provides a fix for this issue.
No changes in this PR impact
dcnm_vrffunctionality.RCA
Serial number to fabric name translation was not happening because there was not a mock defined for `DcnmVrf().sn_fab.
Changes
1.
module_utils/network/dcnm.pyAdd two new methods which duplicate the functionality of
get_ip_sn_fabric_dict()get_ip_fabric_dict()get_sn_fabric_dict()get_ip_sn_fabric_dict()was not touched.The reason for the new methods is because patch (test_dcnm_vrf.py, setUp()) doesn't seem to support patching a method that returns more than one value (at least I couldn't make that work).
Another reason is that it's cleaner for these utility methods to be as simple as possible. For example, dcnm_vrf.py does not need or use the ip_fab dict that is returned by
get_ip_sn_fabric_dict(), so it's better if we don't have to defineip_fabinDcnmVrf(). Separatingget_ip_sn_fabric_dict()into two methods allowsDcnmVrf()to define and use onlysn_fab.2.
modules/dcnm_vrf.pyIn init(), use the replacement method
get_sn_fabric_dict()rather thanget_ip_sn_fabric_dict()3.
test_dcnm_vrf.pyAdd the mock for
sn_fabModify load_fixtures() for all test cases that required a
sn_fabmock.Define two new vars for fabric details (for a future commit)
4.
dcnm_vrf.jsonAdd the following objects
Of the above, only
mock_sn_fabis currently used. The others will be used in future PRs as we work through cleaning up these unit tests.