ansible-collections / cisco.ios

Ansible Network Collection for Cisco IOS
GNU General Public License v3.0
261 stars 162 forks source link

Fix ospfv2 module improperly rendering distance.admin_distance. #1029

Closed round3d closed 3 months ago

round3d commented 3 months ago
SUMMARY

Fixes the ospfv2 distance.admin_distance template. Minor changes to qualify the properties in the setval of the template.

Fixes #1028

ISSUE TYPE
COMPONENT NAME

ospfv2

ADDITIONAL INFORMATION

before:


    "rendered": [
        "router ospf 1"
    ]

after:

    "rendered": [
        "router ospf 1",
        "distance 1 "
    ]
softwarefactory-project-zuul[bot] commented 3 months ago

Build succeeded. https://ansible.softwarefactory-project.io/zuul/buildset/f340e467e8dd422a86c877dcf43d7826

:heavy_check_mark: ansible-galaxy-importer SUCCESS in 4m 30s :heavy_check_mark: build-ansible-collection SUCCESS in 11m 06s :x: ansible-ee-integration-ios-latest RETRY_LIMIT in 9m 58s (non-voting) :x: ansible-ee-integration-ios-stable-2.9 FAILURE in 14m 37s (non-voting) :x: ansible-ee-integration-ios-stable-2.11 RETRY_LIMIT in 3m 17s (non-voting) :x: ansible-ee-integration-ios-stable-2.12 RETRY_LIMIT in 3m 10s (non-voting) :x: ansible-ee-integration-ios-libssh-latest RETRY_LIMIT in 6m 06s (non-voting) :x: ansible-ee-integration-ios-libssh-stable-2.9 FAILURE in 12m 41s (non-voting) :x: ansible-ee-integration-ios-libssh-stable-2.11 RETRY_LIMIT in 12m 32s (non-voting) :x: ansible-ee-integration-ios-libssh-stable-2.12 RETRY_LIMIT in 6m 27s (non-voting) :heavy_check_mark: ansible-tox-linters SUCCESS in 13m 19s

KB-perByte commented 3 months ago

Hey @round3d Thank you for your contribution, can you please add a changelog and a unit test case to confirm your changes? Let us know if you need any help. Regards

round3d commented 3 months ago

Hi @KB-perByte , I can do that with the changelog.

The existing test case test_ios_ospfv2_rendered (and some others that include distance.admin_distance) should already cover it, but the commands list for the assert doesn't include the admin lines that should be rendered. It includes the distance ospf config but not the distance for admin_distance. That's the cause of some of those checks failing.

https://github.com/ansible-collections/cisco.ios/blob/cefdf849fb395ebca2b49b342a524fa9e4430680/tests/unit/modules/network/ios/test_ios_ospfv2.py#L753-L765

https://github.com/ansible-collections/cisco.ios/blob/cefdf849fb395ebca2b49b342a524fa9e4430680/tests/unit/modules/network/ios/test_ios_ospfv2.py#L915-L950

Sorry if that's not clear.

softwarefactory-project-zuul[bot] commented 3 months ago

Build succeeded. https://ansible.softwarefactory-project.io/zuul/buildset/c2d89214c672447aa328df03f346c86c

:heavy_check_mark: ansible-galaxy-importer SUCCESS in 4m 17s :heavy_check_mark: build-ansible-collection SUCCESS in 9m 46s :x: ansible-ee-integration-ios-latest RETRY_LIMIT in 3m 08s (non-voting) :x: ansible-ee-integration-ios-stable-2.9 FAILURE in 14m 04s (non-voting) :x: ansible-ee-integration-ios-stable-2.11 RETRY_LIMIT in 3m 05s (non-voting) :x: ansible-ee-integration-ios-stable-2.12 RETRY_LIMIT in 3m 13s (non-voting) :x: ansible-ee-integration-ios-libssh-latest RETRY_LIMIT in 2m 55s (non-voting) :x: ansible-ee-integration-ios-libssh-stable-2.9 FAILURE in 13m 22s (non-voting) :x: ansible-ee-integration-ios-libssh-stable-2.11 RETRY_LIMIT in 3m 14s (non-voting) :x: ansible-ee-integration-ios-libssh-stable-2.12 RETRY_LIMIT in 3m 07s (non-voting) :heavy_check_mark: ansible-tox-linters SUCCESS in 11m 32s

roverflow commented 3 months ago

Hey @round3d , Thanks for the contribution, as you said the reason its failing is because the the assert list does not have commands for the admin_distance, can you add these lines to the list in the test file which is located in tests/unit/modules/network/ios/test_ios_ospfv2.py .

As for the changelog, you will have to add a changelog yml file in changelogs/fragments folder, you can look as ones that already exists or a previous PRs that have been merged for reference.

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (cefdf84) 86.74% compared to head (18a57a6) 86.74%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1029 +/- ## ======================================= Coverage 86.74% 86.74% ======================================= Files 197 197 Lines 12067 12067 ======================================= Hits 10468 10468 Misses 1599 1599 ``` [see 1 file with indirect coverage changes](https://app.codecov.io/gh/ansible-collections/cisco.ios/pull/1029/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ansible-collections)
softwarefactory-project-zuul[bot] commented 3 months ago

Build succeeded. https://ansible.softwarefactory-project.io/zuul/buildset/fd93ffda11c14c07bff8cdd7672e7362

:heavy_check_mark: ansible-galaxy-importer SUCCESS in 3m 49s :heavy_check_mark: build-ansible-collection SUCCESS in 9m 46s :x: ansible-ee-integration-ios-latest RETRY_LIMIT in 3m 09s (non-voting) :x: ansible-ee-integration-ios-stable-2.9 FAILURE in 12m 58s (non-voting) :x: ansible-ee-integration-ios-stable-2.11 RETRY_LIMIT in 3m 31s (non-voting) :x: ansible-ee-integration-ios-stable-2.12 RETRY_LIMIT in 4m 54s (non-voting) :x: ansible-ee-integration-ios-libssh-latest RETRY_LIMIT in 4m 56s (non-voting) :x: ansible-ee-integration-ios-libssh-stable-2.9 FAILURE in 14m 30s (non-voting) :x: ansible-ee-integration-ios-libssh-stable-2.11 RETRY_LIMIT in 3m 17s (non-voting) :x: ansible-ee-integration-ios-libssh-stable-2.12 RETRY_LIMIT in 5m 02s (non-voting) :heavy_check_mark: ansible-tox-linters SUCCESS in 11m 52s

round3d commented 3 months ago

Please fix the tests and add a changelog as mentioned above

Hi @roverflow , I've done that. Thanks.